Mobile Push Relay Platform#705
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR adds push-relay and tunnel-relay workers, expands ADE CLI sync/push/DPoP/cloud-relay support, and updates desktop and iOS clients for pairing, notifications, Live Activities, and offline chat state. ChangesCI and repository plumbing
Push Relay Cloudflare Worker
Tunnel Relay Cloudflare Worker
ADE CLI push, DPoP, and cloud relay
Desktop cloud relay UI, IPC, and pairing QR
iOS push, Live Activities, QR pairing, and offline chat
Estimated code review effort: 5 (Critical) | ~150 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
@copilot review but do not make fixes |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
arul28
force-pushed
the
ade/mobile-push-relay-platform-c1ebe009
branch
from
ddf9e3d to
d0e1451
Compare
arul28
force-pushed
the
ade/mobile-push-relay-platform-c1ebe009
branch
from
d0e1451 to
430acac
Compare
|
@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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ios/ADE/Views/Work/WorkRootComponents.swift (1)
1066-1085: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winVoiceOver announces "running" for pending-sync rows.
accessibilityLabelstill derives its status text fromsessionStatusLabel(for: status), but pending-sync optimistic sessions are built withstatus: "running"(seeWorkStatusAndFormattingHelpers.swiftline 37). VoiceOver users will hear an active/running status while sighted users see "Pending sync" — a real mismatch introduced by this change.♿️ Proposed fix
var accessibilityLabel: String { - var parts = [chatSummary?.title ?? session.title, session.laneName, sessionStatusLabel(for: status)] + var parts = [ + chatSummary?.title ?? session.title, + session.laneName, + isPendingSyncCreation ? "pending sync" : sessionStatusLabel(for: status) + ] if session.pinned { parts.append("pinned") } if isArchived { parts.append("archived") } return parts.joined(separator: ", ") }🤖 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/Work/WorkRootComponents.swift` around lines 1066 - 1085, The accessibility label for pending-sync rows is still using the normal status text, so VoiceOver announces “running” instead of the visible pending-sync state. Update `accessibilityLabel` in `WorkRootComponents` to special-case `isPendingSyncCreation` and use a pending-sync-friendly label rather than `sessionStatusLabel(for: status)`, while leaving the existing pinned/archived parts unchanged.apps/ios/ADE/Services/SyncService.swift (1)
4089-4099: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAwait the unregister before disconnecting
apps/ios/ADE/Services/SyncService.swift:4089-4093
handleUnpair()launchespush.unregisterDevicein aTask, thendisconnect(clearCredentials:)tears down the socket immediately.sendPushCommandonly sends whilecanSendLiveRequests()is true, so this unregister can be dropped before it reaches the relay.🤖 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 4089 - 4099, `forgetHost()` is disconnecting before the unpair/unregister request has a chance to be sent, so the relay cleanup can be dropped. Update the flow so `PushNotificationService.shared.handleUnpair()` is awaited or otherwise completed before calling `disconnect(clearCredentials:)`, and keep the rest of the teardown in `forgetHost()` afterward. Use the `handleUnpair()` path in `PushNotificationService` and the `disconnect(clearCredentials:)` call in `SyncService` as the key points to adjust.
🧹 Nitpick comments (10)
apps/tunnel-relay/README.md (1)
118-122: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valuePersonal account name embedded in public docs.
The deploy example hardcodes
ade-tunnel-relay.arulsharma1028.workers.dev, exposing a personal identifier in a checked-in README rather than a generic placeholder.✏️ Suggested fix
-curl https://ade-tunnel-relay.arulsharma1028.workers.dev/health +curl https://ade-tunnel-relay.<your-workers-subdomain>.workers.dev/health🤖 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/tunnel-relay/README.md` around lines 118 - 122, The deploy example in the README hardcodes a personal worker subdomain in the curl URL, which should be replaced with a generic placeholder or example domain. Update the example near the deploy/health-check snippet to use a neutral identifier derived from the app name instead of the embedded personal account name, keeping the rest of the workflow unchanged.apps/tunnel-relay/package.json (1)
16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftVitest is still several majors behind.
apps/tunnel-relay/package.jsonpinsvitest@^0.34.6; the relay packages are already aligned on that version, but it’s far behind current Vitest 4.x and will need a migration plan to upgrade cleanly.🤖 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/tunnel-relay/package.json` at line 16, The tunnel-relay package is still pinned to an old Vitest version, so update the vitest dependency in the package.json for the relay app to match the newer workspace standard and plan the migration to a current 4.x release. Check any related test config or scripts that reference Vitest directly so they remain compatible with the newer version.apps/ade-cli/src/multiProjectRpcServer.ts (1)
770-776: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winInconsistent response shape for
sync.getRequireDpop/sync.setRequireDpop.Every other simple-scalar getter/setter in this handler wraps its primitive in a named field — e.g.
sync.getPinreturns{ pin: ... }(Line 733) andsync.getRuntimeNamereturns{ runtimeName: ... }(Line 750) — butsync.getRequireDpop/sync.setRequireDpopreturn a bareboolean. This diverges from the established RPC response convention and may not render correctly wherever the CLI/desktop generically expects an object with named fields (e.g.,--text/--jsonformatting, or future desktop consumers keyed on a field name).♻️ Proposed fix: wrap for consistency
if (method === "sync.getRequireDpop") { - return (await getSyncService()).getRequireDpop(); + return { requireDpop: (await getSyncService()).getRequireDpop() }; } if (method === "sync.setRequireDpop") { - return (await getSyncService()).setRequireDpop(params.requireDpop === true); + return { + requireDpop: (await getSyncService()).setRequireDpop(params.requireDpop === true), + }; }Note: this also requires updating
syncService.test.ts/cli.tsconsumers if the shape changes (cli.test.tscurrently expects the plansteps, not the RPC result shape, so should be unaffected).Please verify how
cli.tsrenders--textoutput for this step's result to confirm the raw boolean isn't silently mis-rendered.🤖 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/ade-cli/src/multiProjectRpcServer.ts` around lines 770 - 776, The sync.getRequireDpop and sync.setRequireDpop branches in multiProjectRpcServer.ts return a bare boolean, unlike the other getter/setter handlers such as sync.getPin and sync.getRuntimeName that wrap primitives in named fields. Update the getSyncService() call sites so both methods return an object with a stable field name (for example matching the existing RPC convention), and then adjust any tests or consumers that assert the raw boolean shape, including syncService.test.ts if needed. Also verify cli.ts text/json rendering still reads the new wrapped response correctly so the output remains consistent with the other sync RPCs.apps/ios/ADE/Views/Settings/SettingsPairingScannerSheet.swift (1)
270-273: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: SwiftLint flags
override class var→override static varin this final class.Since
PreviewViewisfinal, SwiftLint'sstatic_over_final_classrule suggests usingstaticinstead ofclassfor thelayerClassoverride. This is a style-only change with no behavior impact; there are also known false-positive reports for this rule on overrides of inherited class properties, so it's worth a quick compile check before applying.💡 Proposed fix
- override class var layerClass: AnyClass { AVCaptureVideoPreviewLayer.self } + override static var layerClass: AnyClass { AVCaptureVideoPreviewLayer.self }🤖 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/Settings/SettingsPairingScannerSheet.swift` around lines 270 - 273, SwiftLint is flagging the `PreviewView` override because the class is `final`, so update the `layerClass` declaration in `PreviewView` from a class override to the `static` form SwiftLint expects, while keeping the `previewLayer` accessor unchanged. Since this is a style-only `static_over_final_class` change, do a quick compile check after adjusting the `layerClass` symbol to confirm the inherited `UIView` override still behaves correctly.Source: Linters/SAST tools
apps/ios/ADE/Views/Settings/SettingsPushDeliverySection.swift (1)
30-32: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win"Enable notifications" prompt shows before pairing, but tapping it silently no-ops.
needsPermissionPromptonly checkspermissionStatus, so on an unpaired device the panel shows both "Pair a machine first" (Status row) and an actionable "Enable notifications" button. PerPushNotificationService.enableIfPaired()(provided snippet), tapping it whenhasPairedHost == falsejust sets state to.unsupportedwithout ever prompting for system permission — a dead-end tap.🛠️ Proposed fix
var needsPermissionPrompt: Bool { - permissionStatus == .notDetermined || permissionStatus == .denied + registrationState != .unsupported + && (permissionStatus == .notDetermined || permissionStatus == .denied) }Also applies to: 114-116
🤖 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/Settings/SettingsPushDeliverySection.swift` around lines 30 - 32, The notification prompt logic in SettingsPushDeliverySection.swift is too permissive because needsPermissionPrompt only checks permissionStatus, so the UI can show “Enable notifications” even when no machine is paired. Update the conditions that drive the prompt/button so they also require a paired host state, and make sure the action path through PushNotificationService.enableIfPaired() is only exposed when hasPairedHost is true. Also review the related display logic near the other referenced section to keep the status row and the actionable prompt consistent.apps/ios/ADE/Views/Work/WorkStatusAndFormattingHelpers.swift (1)
5-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLGTM overall; consider deduplicating optimistic-session construction.
Correct and matches the offline-create contract in
SyncService.swift. Minor: this function andmakeOptimisticSession(for:)inWorkRootScreen+Actions.swiftboth hand-build aTerminalSessionSummarywith ~10 identical placeholder fields (ptyId: nil, tracked: true, pinned: false, manuallyNamed: nil, exitCode: nil, transcriptPath: "", headShaStart: nil, headShaEnd: nil, resumeCommand: nil, resumeMetadata: nil). Factoring these shared defaults into a small factory/extension would reduce future drift risk whenTerminalSessionSummarygains fields.♻️ Sketch of a shared default factory
extension TerminalSessionSummary { static func optimisticPlaceholder( id: String, laneId: String, laneName: String, toolType: String, title: String, status: String, startedAt: String? ) -> TerminalSessionSummary { TerminalSessionSummary( id: id, laneId: laneId, laneName: laneName, ptyId: nil, tracked: true, pinned: false, manuallyNamed: nil, goal: nil, toolType: toolType, title: title, status: status, startedAt: startedAt, endedAt: nil, exitCode: nil, transcriptPath: "", headShaStart: nil, headShaEnd: nil, lastOutputPreview: nil, summary: nil, runtimeState: status, resumeCommand: nil, resumeMetadata: nil, chatIdleSinceAt: nil ) } }🤖 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/Work/WorkStatusAndFormattingHelpers.swift` around lines 5 - 51, The optimistic-session construction is duplicated between workPendingChatCreationOptimisticSession and makeOptimisticSession(for:), so factor the shared placeholder TerminalSessionSummary fields into a common factory or extension. Create a reusable helper on TerminalSessionSummary (or a small builder) that centralizes the repeated defaults like ptyId, tracked, pinned, manuallyNamed, exitCode, transcriptPath, headShaStart, headShaEnd, resumeCommand, and resumeMetadata, then have both functions call it and only supply the per-row values such as id, laneId, laneName, toolType, title, status, and startedAt.apps/push-relay/migrations/0001_push_registrations.sql (1)
44-53: 🧹 Nitpick | 🔵 TrivialNo retention/cleanup policy for
publish_suppressionrows.The suppression table grows unbounded with one row per
(machine_key, suppression_key)and only an index onpublished_at; there's no scheduled job referenced in this migration/cohort to prune stale rows. Consider a Cron Trigger or TTL-style cleanup to bound table growth over time.🤖 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/push-relay/migrations/0001_push_registrations.sql` around lines 44 - 53, The publish_suppression table is created without any retention mechanism, so rows can accumulate indefinitely. Update the push-relay migration cohort around publish_suppression to add a cleanup strategy, such as a Cron Trigger or TTL-style pruning job, and reference the relevant table/index definitions so stale rows are periodically removed and table growth stays bounded.apps/push-relay/src/relay.ts (2)
431-444: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider deferring
pruneRelayStateoff the response path.Both
handleDeviceUpsert(764) andhandlePublish(869)awaitpruneRelayState, adding two blockingDELETEround-trips to every write request. Since pruning is best-effort maintenance and not something the caller needs to wait on, threadingExecutionContextthrough fromindex.tsand usingctx.waitUntil(pruneRelayState(env))would cut this latency from the response path without changing behavior.Also applies to: 841-878
🤖 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/push-relay/src/relay.ts` around lines 431 - 444, pruneRelayState is currently awaited on the critical path, blocking write requests with maintenance DELETEs. Update handleDeviceUpsert and handlePublish to stop awaiting pruneRelayState and instead trigger it via ExecutionContext using ctx.waitUntil(pruneRelayState(env)) after threading ctx through from index.ts. Keep pruneRelayState unchanged, but make sure the call sites use the existing handleDeviceUpsert/handlePublish flow to defer pruning off the response path.
536-563: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSequential per-device APNs delivery.
Devices are delivered to one at a time via a
forloop withawaitinside. Parallelizing withPromise.all(mindful of the per-deviceclearInvalidToken/deleteActivityTokenside effects) would reduce publish latency, especially sinceMAX_DEVICES_PER_MACHINEis configurable above the default of 16.Also applies to: 595-668
🤖 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/push-relay/src/relay.ts` around lines 536 - 563, The APNs delivery logic in relay.ts is sequential because the per-device send happens inside a for loop with await, which increases publish latency. Refactor the device processing in the APNs send path (the alert flow and the other matching device-delivery block(s) in relay.ts) to run requests in parallel with Promise.all or a similar concurrency pattern, while still preserving the per-device side effects like clearInvalidToken and deleteActivityToken for each result. Keep the existing outcome shaping and use the same sendApnsPush, clearInvalidToken, and outcomes.push logic per device.apps/ios/ADE/Services/PushNotificationService.swift (1)
133-164: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConcurrent registration triggers can silently drop updates.
registerInFlightcausesregisterDevice()to return immediately if a call is already in progress, with no mechanism to re-run once it completes. IfupdateLiveActivityPushToStartToken(Line 127) ordidRegisterForRemoteNotificationsfire while a priorregisterDevice()call is still awaiting the network round-trip, that update is dropped — the brain never learns the latest token/prefs until some unrelated trigger fires again.♻️ Proposed fix: re-run registration if a change arrived mid-flight
private var registerInFlight = false + private var registerDirty = false private func registerDevice() async { guard let sync = SyncService.shared, sync.hasPairedHost else { setRegistrationState(.unsupported) return } - guard !registerInFlight else { return } + guard !registerInFlight else { + registerDirty = true + return + } registerInFlight = true - defer { registerInFlight = false } + defer { + registerInFlight = false + if registerDirty { + registerDirty = false + Task { await registerDevice() } + } + }🤖 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/PushNotificationService.swift` around lines 133 - 164, `registerDevice()` can drop newer token/prefs changes when `registerInFlight` is already true, so add a follow-up registration pass after the in-flight request completes if a change arrived during the await. Use the existing `registerDevice()` and `registerInFlight` flow in `PushNotificationService` to track that a retry is needed, then re-invoke registration once the current call finishes so updates from `updateLiveActivityPushToStartToken` and `didRegisterForRemoteNotifications` are not lost.
🤖 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 @.github/workflows/ci.yml:
- Around line 146-164: The new checkout steps in the push-relay CI jobs are
missing the credential hardening used elsewhere. Update the actions/checkout@v4
step in typecheck-push-relay and the matching checkout in test-push-relay to set
persist-credentials: false, consistent with validate-docs, so the GITHUB_TOKEN
is not left in git config during npm execution.
- Around line 266-285: The checkout step in the tunnel-relay GitHub Actions jobs
is missing the credential hardening setting, so update both
`typecheck-tunnel-relay` and `test-tunnel-relay` to use `actions/checkout@v4`
with `persist-credentials: false`, matching the secure checkout configuration
already expected for the `npm ci` steps in these jobs.
In `@apps/ade-cli/src/bootstrap.ts`:
- Line 1390: The shutdown cleanup for syncTunnelClientService.dispose() is
letting an async rejection escape because swallow only handles synchronous
errors. Update the disposal call in bootstrap.ts to follow the same pattern as
configReloadService.dispose(), using the dispose() promise’s .catch() handling
instead of void-ing it so failures are swallowed during shutdown. Use the
syncTunnelClientService.dispose() and configReloadService.dispose() calls as the
nearby reference points.
In `@apps/ade-cli/src/services/push/pushPublisherService.ts`:
- Around line 679-694: The relay APNS health check in relayApnsConfigured is
caching failures for the full APNS_HEALTH_CACHE_MS window because the catch
block writes { value: null, at: nowMs } and the freshness guard treats it as
valid; change this so a transient deps.relayClient.health() error does not
suppress re-checks for 24 hours. Update the caching logic in relayApnsConfigured
to either avoid storing failed results at all or use a much shorter negative
TTL, while keeping the successful health path and deps.store.recordRelayContact
behavior unchanged.
In `@apps/ade-cli/src/services/sync/syncTunnelClientService.ts`:
- Around line 191-224: The tunnel forwarding in syncTunnelClientService
currently drops frames in forward() whenever the peer socket is still
CONNECTING, which can lose the first bytes during startup. Update the message
handling around the pipe/local WebSocket pair to queue outgoing data per
direction until the opposite socket emits open, then flush the buffered frames
before resuming live forwarding. Use the existing forward, pipe, local, and
closeBoth logic to locate the change and make sure the buffering is cleared on
close/error.
In `@apps/ios/ADE/App/ADEAppDelegate.swift`:
- Around line 37-43: The didReceiveRemoteNotification callback in ADEAppDelegate
only records push diagnostics via
PushNotificationService.shared.notePushReceived(), so it should not claim that
new content was fetched. Update application(_:didReceiveRemoteNotification:) to
return .noData instead of .newData, keeping the MainActor update unchanged.
In `@apps/ios/ADE/Services/DpopKeyService.swift`:
- Around line 134-146: Avoid force-casting the keychain result in
loadPrivateKey(); the `item as! SecKey` in DpopKeyService can crash if Keychain
returns an unexpected type. Update the `SecItemCopyMatching` handling to safely
cast the returned `item` to `SecKey` with a conditional cast and return nil on
failure, keeping the method’s optional-safe behavior consistent with the rest of
DpopKeyService.
In `@apps/ios/ADE/Services/LiveActivityService.swift`:
- Around line 87-100: The observePushToken(for:) task in LiveActivityService can
still run its post-loop cleanup after being cancelled, which may let an old
observer call report(token: "") and clear perActivityTokenTasks[activity.id]
after a replacement task is already active. Update the Task block to guard the
cleanup with Task.isCancelled or a task-identity check before sending the empty
token and niling the dictionary entry, so only the currently active observer
performs that final cleanup.
In `@apps/ios/ADE/Services/SyncService.swift`:
- Line 4080: `clearPendingChatCreations()` currently removes only the in-memory
pending chat state, but leaves matching queued `chat.create` operations behind.
Update `clearPendingChatCreations()` (and the `disconnect(clearCredentials:)`
forget-machine path that calls it) to also purge any persisted queued create
commands from the pending operations store, similar to how
`cancelPendingChatCreation(id:)` clears both the operation entry and UI
snapshot. Make sure the cleanup covers the
`loadPendingOperations()`/`savePendingOperations()` queue so
`flushPendingOperations()` cannot replay stale chat creates after re-pairing.
In `@apps/push-relay/src/apns.ts`:
- Around line 119-134: The APNs request in sendApnsPush is unbounded and can
stall the sequential delivery path in relay.ts. Update the fetch call in
sendApnsPush to use a timeout-backed AbortSignal (for example via
AbortSignal.timeout or equivalent) so hanging APNs connections are canceled
after a reasonable limit, and make sure the timeout error is handled or
propagated cleanly through ApnsSendResult.
In `@apps/push-relay/src/relay.ts`:
- Around line 146-149: Update readNumber in relay.ts so it distinguishes
explicit JSON null from a missing value instead of coercing null through
Number(). Treat null as null and only convert actual numeric-looking inputs,
then keep liveActivityApnsPayload using that helper so stale-date,
dismissalDate, and relevanceScore preserve “absent” semantics; mirror the
explicit-null handling already used by the sound logic.
- Around line 285-293: The readDeviceIds helper is collapsing two different
cases: missing deviceIds and provided-but-empty/invalid deviceIds, which causes
selectTargets to treat invalid input as “all devices.” Update readDeviceIds in
relay.ts to preserve the absent-vs-empty distinction by returning null only when
deviceIds is not present, and returning an empty array when the property exists
but contains no valid IDs; then ensure selectTargets handles an empty array as
“target none” rather than falling back to the unrestricted path.
- Around line 671-696: `handleMachineClaim` currently does a read-then-insert on
`machines.machine_key`, which can race under concurrent first claims and surface
a unique-constraint failure instead of the intended 201/409 response. Update the
claim flow in `handleMachineClaim` to use a single atomic `INSERT ... ON
CONFLICT(machine_key)` path, and preserve the existing semantics by
distinguishing same-secret idempotent claims from conflicting secrets via the
conflict handling result rather than a separate `loadMachine()` lookup.
- Around line 893-895: The body size check in relay.ts only relies on
contentLengthExceeds(), which can be bypassed when the content-length header is
missing or invalid. Update the request handling flow around the body read and
arrayBuffer() usage to validate the actual body.byteLength after reading, or
reject requests without a trustworthy length header before buffering the
payload. Use the existing contentLengthExceeds helper and the surrounding
request handler logic to keep the 256 KB limit enforced.
In `@apps/tunnel-relay/src/relay.ts`:
- Around line 129-146: routeTunnelPath currently decodes each path segment with
decodeURIComponent without handling malformed escape sequences, so bad input can
throw and escape into handleRequest. Update routeTunnelPath to fail closed by
catching decodeURIComponent errors (or otherwise validating before decoding) and
returning null for malformed paths, while keeping the existing route matching
for valid cases in routeTunnelPath, validKey, and CONNECTION_ID_PATTERN.
---
Outside diff comments:
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 4089-4099: `forgetHost()` is disconnecting before the
unpair/unregister request has a chance to be sent, so the relay cleanup can be
dropped. Update the flow so `PushNotificationService.shared.handleUnpair()` is
awaited or otherwise completed before calling `disconnect(clearCredentials:)`,
and keep the rest of the teardown in `forgetHost()` afterward. Use the
`handleUnpair()` path in `PushNotificationService` and the
`disconnect(clearCredentials:)` call in `SyncService` as the key points to
adjust.
In `@apps/ios/ADE/Views/Work/WorkRootComponents.swift`:
- Around line 1066-1085: The accessibility label for pending-sync rows is still
using the normal status text, so VoiceOver announces “running” instead of the
visible pending-sync state. Update `accessibilityLabel` in `WorkRootComponents`
to special-case `isPendingSyncCreation` and use a pending-sync-friendly label
rather than `sessionStatusLabel(for: status)`, while leaving the existing
pinned/archived parts unchanged.
---
Nitpick comments:
In `@apps/ade-cli/src/multiProjectRpcServer.ts`:
- Around line 770-776: The sync.getRequireDpop and sync.setRequireDpop branches
in multiProjectRpcServer.ts return a bare boolean, unlike the other
getter/setter handlers such as sync.getPin and sync.getRuntimeName that wrap
primitives in named fields. Update the getSyncService() call sites so both
methods return an object with a stable field name (for example matching the
existing RPC convention), and then adjust any tests or consumers that assert the
raw boolean shape, including syncService.test.ts if needed. Also verify cli.ts
text/json rendering still reads the new wrapped response correctly so the output
remains consistent with the other sync RPCs.
In `@apps/ios/ADE/Services/PushNotificationService.swift`:
- Around line 133-164: `registerDevice()` can drop newer token/prefs changes
when `registerInFlight` is already true, so add a follow-up registration pass
after the in-flight request completes if a change arrived during the await. Use
the existing `registerDevice()` and `registerInFlight` flow in
`PushNotificationService` to track that a retry is needed, then re-invoke
registration once the current call finishes so updates from
`updateLiveActivityPushToStartToken` and `didRegisterForRemoteNotifications` are
not lost.
In `@apps/ios/ADE/Views/Settings/SettingsPairingScannerSheet.swift`:
- Around line 270-273: SwiftLint is flagging the `PreviewView` override because
the class is `final`, so update the `layerClass` declaration in `PreviewView`
from a class override to the `static` form SwiftLint expects, while keeping the
`previewLayer` accessor unchanged. Since this is a style-only
`static_over_final_class` change, do a quick compile check after adjusting the
`layerClass` symbol to confirm the inherited `UIView` override still behaves
correctly.
In `@apps/ios/ADE/Views/Settings/SettingsPushDeliverySection.swift`:
- Around line 30-32: The notification prompt logic in
SettingsPushDeliverySection.swift is too permissive because
needsPermissionPrompt only checks permissionStatus, so the UI can show “Enable
notifications” even when no machine is paired. Update the conditions that drive
the prompt/button so they also require a paired host state, and make sure the
action path through PushNotificationService.enableIfPaired() is only exposed
when hasPairedHost is true. Also review the related display logic near the other
referenced section to keep the status row and the actionable prompt consistent.
In `@apps/ios/ADE/Views/Work/WorkStatusAndFormattingHelpers.swift`:
- Around line 5-51: The optimistic-session construction is duplicated between
workPendingChatCreationOptimisticSession and makeOptimisticSession(for:), so
factor the shared placeholder TerminalSessionSummary fields into a common
factory or extension. Create a reusable helper on TerminalSessionSummary (or a
small builder) that centralizes the repeated defaults like ptyId, tracked,
pinned, manuallyNamed, exitCode, transcriptPath, headShaStart, headShaEnd,
resumeCommand, and resumeMetadata, then have both functions call it and only
supply the per-row values such as id, laneId, laneName, toolType, title, status,
and startedAt.
In `@apps/push-relay/migrations/0001_push_registrations.sql`:
- Around line 44-53: The publish_suppression table is created without any
retention mechanism, so rows can accumulate indefinitely. Update the push-relay
migration cohort around publish_suppression to add a cleanup strategy, such as a
Cron Trigger or TTL-style pruning job, and reference the relevant table/index
definitions so stale rows are periodically removed and table growth stays
bounded.
In `@apps/push-relay/src/relay.ts`:
- Around line 431-444: pruneRelayState is currently awaited on the critical
path, blocking write requests with maintenance DELETEs. Update
handleDeviceUpsert and handlePublish to stop awaiting pruneRelayState and
instead trigger it via ExecutionContext using
ctx.waitUntil(pruneRelayState(env)) after threading ctx through from index.ts.
Keep pruneRelayState unchanged, but make sure the call sites use the existing
handleDeviceUpsert/handlePublish flow to defer pruning off the response path.
- Around line 536-563: The APNs delivery logic in relay.ts is sequential because
the per-device send happens inside a for loop with await, which increases
publish latency. Refactor the device processing in the APNs send path (the alert
flow and the other matching device-delivery block(s) in relay.ts) to run
requests in parallel with Promise.all or a similar concurrency pattern, while
still preserving the per-device side effects like clearInvalidToken and
deleteActivityToken for each result. Keep the existing outcome shaping and use
the same sendApnsPush, clearInvalidToken, and outcomes.push logic per device.
In `@apps/tunnel-relay/package.json`:
- Line 16: The tunnel-relay package is still pinned to an old Vitest version, so
update the vitest dependency in the package.json for the relay app to match the
newer workspace standard and plan the migration to a current 4.x release. Check
any related test config or scripts that reference Vitest directly so they remain
compatible with the newer version.
In `@apps/tunnel-relay/README.md`:
- Around line 118-122: The deploy example in the README hardcodes a personal
worker subdomain in the curl URL, which should be replaced with a generic
placeholder or example domain. Update the example near the deploy/health-check
snippet to use a neutral identifier derived from the app name instead of the
embedded personal account name, keeping the rest of the workflow unchanged.
🪄 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: 3715f065-0d4c-4a0a-bd1d-92ededec24b6
⛔ Files ignored due to path filters (7)
apps/ios/ADE.xcodeproj/project.pbxprojis excluded by!**/*.xcodeproj/project.pbxprojapps/push-relay/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsonapps/tunnel-relay/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsondocs/ARCHITECTURE.mdis excluded by!docs/**docs/features/sync-and-multi-device/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**docs/features/sync-and-multi-device/push-notifications.mdis excluded by!docs/**
📒 Files selected for processing (79)
.github/workflows/ci.yml.gitignoreapps/ade-cli/README.mdapps/ade-cli/src/bootstrap.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/multiProjectRpcServer.tsapps/ade-cli/src/services/push/pushPublisherService.test.tsapps/ade-cli/src/services/push/pushPublisherService.tsapps/ade-cli/src/services/push/pushRegistrationStore.tsapps/ade-cli/src/services/push/pushRelayClient.tsapps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.tsapps/ade-cli/src/services/sync/syncCloudRelayStore.tsapps/ade-cli/src/services/sync/syncDpop.test.tsapps/ade-cli/src/services/sync/syncDpop.tsapps/ade-cli/src/services/sync/syncHostService.tsapps/ade-cli/src/services/sync/syncPairingStore.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.tsapps/ade-cli/src/services/sync/syncSecurityStore.tsapps/ade-cli/src/services/sync/syncService.tsapps/ade-cli/src/services/sync/syncTunnelClientService.test.tsapps/ade-cli/src/services/sync/syncTunnelClientService.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/components/settings/SyncDevicesSection.tsxapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/pairingQr.test.tsapps/desktop/src/shared/pairingQr.tsapps/desktop/src/shared/types/push.tsapps/desktop/src/shared/types/sync.tsapps/ios/ADE/ADE.entitlementsapps/ios/ADE/App/ADEApp.swiftapps/ios/ADE/App/ADEAppDelegate.swiftapps/ios/ADE/App/DeepLinkRouter.swiftapps/ios/ADE/Info.plistapps/ios/ADE/Models/RemoteModels.swiftapps/ios/ADE/Services/DpopKeyService.swiftapps/ios/ADE/Services/LiveActivityService.swiftapps/ios/ADE/Services/PairingQrPayload.swiftapps/ios/ADE/Services/PushNotificationService.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Shared/ADEAgentActivityAttributes.swiftapps/ios/ADE/Views/Settings/ConnectionSettingsView.swiftapps/ios/ADE/Views/Settings/SettingsPairingScannerSheet.swiftapps/ios/ADE/Views/Settings/SettingsPairingSection.swiftapps/ios/ADE/Views/Settings/SettingsPinSheet.swiftapps/ios/ADE/Views/Settings/SettingsPushDeliverySection.swiftapps/ios/ADE/Views/Settings/SettingsSupportTypes.swiftapps/ios/ADE/Views/Work/WorkNewChatScreen.swiftapps/ios/ADE/Views/Work/WorkRootComponents.swiftapps/ios/ADE/Views/Work/WorkRootScreen+Actions.swiftapps/ios/ADE/Views/Work/WorkRootScreen.swiftapps/ios/ADE/Views/Work/WorkStatusAndFormattingHelpers.swiftapps/ios/ADETests/ADETests.swiftapps/ios/ADETests/PairingAndDpopTests.swiftapps/ios/ADEWidgets/ADEAgentActivityWidget.swiftapps/ios/ADEWidgets/ADEWidgetBundle.swiftapps/push-relay/README.mdapps/push-relay/migrations/0001_push_registrations.sqlapps/push-relay/package.jsonapps/push-relay/src/apns.tsapps/push-relay/src/index.tsapps/push-relay/src/relay.tsapps/push-relay/test/relay.test.tsapps/push-relay/tsconfig.jsonapps/push-relay/wrangler.jsoncapps/tunnel-relay/.gitignoreapps/tunnel-relay/README.mdapps/tunnel-relay/package.jsonapps/tunnel-relay/src/index.tsapps/tunnel-relay/src/relay.tsapps/tunnel-relay/src/tunnelDo.tsapps/tunnel-relay/test/relay.test.tsapps/tunnel-relay/test/smoke.mjsapps/tunnel-relay/tsconfig.jsonapps/tunnel-relay/wrangler.jsoncpackage.json
![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: 430acac652
ℹ️ 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".
| let event: "start" | "update" | "end"; | ||
| if (activeCount > 0 && !liveActivityStarted) event = "start"; | ||
| else if (activeCount === 0 && liveActivityStarted) event = "end"; | ||
| else if (activeCount === 0 && !liveActivityStarted) return null; // nothing live to report | ||
| else event = "update"; |
There was a problem hiding this comment.
When a second phone registers while an agent run is already active, handleDeviceRegistered() calls poke(), but this machine-global liveActivityStarted flag makes the next payload an update instead of a start. The relay's update/end path targets per-activity tokens, and the new phone only has a push-to-start token at that point, so it never gets a Live Activity for the current run until the aggregate ends and a later run starts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid but deliberately deferred: starting the Live Activity per newly-registered device needs per-device start tracking and a start/update split in the aggregate planner — a design change, not a review fix. Tracked as a follow-up.
| try { | ||
| await deps.relayClient.publish(payload); | ||
| for (const [key, fingerprint] of alertCommits) lastAlertFingerprintByKey.set(key, fingerprint); | ||
| laPlan?.commit(); | ||
| deps.store.recordPublishResult({ at: new Date().toISOString() }); |
There was a problem hiding this comment.
If APNs rejects a publish for a transient per-device reason, the relay still returns HTTP 200 with a failed count in apps/push-relay/src/relay.ts::handlePublish; this call ignores that body and immediately commits alert dedupe/live-activity state. In that scenario the alert is dropped and future identical content is suppressed locally instead of being retried, even though no device received it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c50d7a2 (same change as the Greptile thread): delivery failures no longer count as published.
| private func registerDevice() async { | ||
| guard let sync = SyncService.shared, sync.hasPairedHost else { | ||
| setRegistrationState(.unsupported) | ||
| return | ||
| } | ||
| guard !registerInFlight else { return } | ||
| registerInFlight = true |
There was a problem hiding this comment.
During first pairing/foreground, APNs registration and LiveActivityService push-to-start token reporting can overlap; if the token update calls registerDevice() while the APNs registration is still in flight, this guard drops the second registration. When the first request was built before liveActivityPushToStartTokenHex was set, the relay never receives the push-to-start token until another foreground/token change, so remote Live Activity starts silently do nothing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c50d7a2: registerDevice() coalesces overlapping registrations (pending flag + re-run loop) so a mid-flight push-to-start token still reaches the relay.
arul28
force-pushed
the
ade/mobile-push-relay-platform-c1ebe009
branch
from
430acac to
14e5c0d
Compare
arul28
force-pushed
the
ade/mobile-push-relay-platform-c1ebe009
branch
from
14e5c0d to
c50d7a2
Compare
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c50d7a2558
ℹ️ 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".
| const pushPublisherService = createPushPublisherService({ | ||
| logger, | ||
| store: pushRegistrationStore, | ||
| relayClient: pushRelayClient, | ||
| machineName: os.hostname(), |
There was a problem hiding this comment.
In the multi-project daemon, ProjectScopeRegistry.get() calls createAdeRuntime() for every opened project, so this instantiates a separate publisher per project while all of them share ~/.ade/secrets/push-relay.json and publish the fixed agent-runs Live Activity. With two booted projects, each publisher has its own runs/liveActivityStarted state; a run event from project B overwrites or ends the phone's single machine activity with only B's state, hiding active runs from project A. Gate this to the active sync host or make the publisher a singleton per machine.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 36737c2: the publisher is now a machine-level singleton (memoized by identity file) with per-project attachSources/detach — runs merge across scopes into the one agent-runs aggregate, and a detached project's runs drop out.
| return; | ||
| } | ||
| for (const [key, fingerprint] of alertCommits) lastAlertFingerprintByKey.set(key, fingerprint); | ||
| laPlan?.commit(); |
There was a problem hiding this comment.
When a publish contains both an alert and a Live Activity item (for example the first event is an approval request), the relay can return delivered > 0 because the alert succeeded while the Live Activity push-to-start failed. This line still runs laPlan.commit(), setting liveActivityStarted/dedupe state, so subsequent flushes send updates instead of retrying the start and the phone never shows the activity. Inspect the relay outcomes for the liveactivity item before committing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 36737c2: the Live Activity plan commits only off its own outcomes (delivered/suppressed liveactivity entries); an alert-only success no longer commits LA state, so the start retries.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/ade-cli/src/services/push/pushRegistrationStore.ts`:
- Around line 107-115: The write path in pushRegistrationStore’s write function
currently relies on a post-write chmod after writeTextAtomic, which can leave
machineKey and machineSecret briefly exposed with broader permissions. Update
the file creation flow so the secrets file is created with 0600 immediately,
either by passing the mode into writeTextAtomic or by setting that default
inside the helper, and keep the existing cache update in write.
In `@apps/ade-cli/src/services/sync/syncCloudRelayStore.ts`:
- Around line 84-100: The load() path in syncCloudRelayStore is racing on first
run because it reads, generates, and writes machineKey/secret without any
exclusive first-write guard. Update load() (and the read/write helpers it uses)
so the initial config creation is atomic across processes, using an exclusive
file creation or equivalent lock before persisting the identity values. Keep the
existing validation and normalization logic, but ensure only one caller can mint
and write the first SyncCloudRelayConfig so every accessor returns the same
machineKey/secret pair.
In `@apps/ios/ADE/Views/Settings/SettingsPairingScannerSheet.swift`:
- Around line 241-248: The setTorch(_:) method currently ignores failures from
lockForConfiguration() and still changes torchMode and calls
unlockForConfiguration(), which can run without a valid lock. Update the
AVCaptureDevice handling in SettingsPairingScannerSheet.setTorch(_:) to attempt
the lock first, bail out immediately if locking fails, and only then set
torchMode and unlock the device.
🪄 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: 61f98e73-9073-44b7-b619-d1642a21ff78
⛔ Files ignored due to path filters (7)
apps/ios/ADE.xcodeproj/project.pbxprojis excluded by!**/*.xcodeproj/project.pbxprojapps/push-relay/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsonapps/tunnel-relay/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsondocs/ARCHITECTURE.mdis excluded by!docs/**docs/features/sync-and-multi-device/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**docs/features/sync-and-multi-device/push-notifications.mdis excluded by!docs/**
📒 Files selected for processing (79)
.github/workflows/ci.yml.gitignoreapps/ade-cli/README.mdapps/ade-cli/src/bootstrap.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/multiProjectRpcServer.tsapps/ade-cli/src/services/push/pushPublisherService.test.tsapps/ade-cli/src/services/push/pushPublisherService.tsapps/ade-cli/src/services/push/pushRegistrationStore.tsapps/ade-cli/src/services/push/pushRelayClient.tsapps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.tsapps/ade-cli/src/services/sync/syncCloudRelayStore.tsapps/ade-cli/src/services/sync/syncDpop.test.tsapps/ade-cli/src/services/sync/syncDpop.tsapps/ade-cli/src/services/sync/syncHostService.tsapps/ade-cli/src/services/sync/syncPairingStore.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.tsapps/ade-cli/src/services/sync/syncSecurityStore.tsapps/ade-cli/src/services/sync/syncService.tsapps/ade-cli/src/services/sync/syncTunnelClientService.test.tsapps/ade-cli/src/services/sync/syncTunnelClientService.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/components/settings/SyncDevicesSection.tsxapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/pairingQr.test.tsapps/desktop/src/shared/pairingQr.tsapps/desktop/src/shared/types/push.tsapps/desktop/src/shared/types/sync.tsapps/ios/ADE/ADE.entitlementsapps/ios/ADE/App/ADEApp.swiftapps/ios/ADE/App/ADEAppDelegate.swiftapps/ios/ADE/App/DeepLinkRouter.swiftapps/ios/ADE/Info.plistapps/ios/ADE/Models/RemoteModels.swiftapps/ios/ADE/Services/DpopKeyService.swiftapps/ios/ADE/Services/LiveActivityService.swiftapps/ios/ADE/Services/PairingQrPayload.swiftapps/ios/ADE/Services/PushNotificationService.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Shared/ADEAgentActivityAttributes.swiftapps/ios/ADE/Views/Settings/ConnectionSettingsView.swiftapps/ios/ADE/Views/Settings/SettingsPairingScannerSheet.swiftapps/ios/ADE/Views/Settings/SettingsPairingSection.swiftapps/ios/ADE/Views/Settings/SettingsPinSheet.swiftapps/ios/ADE/Views/Settings/SettingsPushDeliverySection.swiftapps/ios/ADE/Views/Settings/SettingsSupportTypes.swiftapps/ios/ADE/Views/Work/WorkNewChatScreen.swiftapps/ios/ADE/Views/Work/WorkRootComponents.swiftapps/ios/ADE/Views/Work/WorkRootScreen+Actions.swiftapps/ios/ADE/Views/Work/WorkRootScreen.swiftapps/ios/ADE/Views/Work/WorkStatusAndFormattingHelpers.swiftapps/ios/ADETests/ADETests.swiftapps/ios/ADETests/PairingAndDpopTests.swiftapps/ios/ADEWidgets/ADEAgentActivityWidget.swiftapps/ios/ADEWidgets/ADEWidgetBundle.swiftapps/push-relay/README.mdapps/push-relay/migrations/0001_push_registrations.sqlapps/push-relay/package.jsonapps/push-relay/src/apns.tsapps/push-relay/src/index.tsapps/push-relay/src/relay.tsapps/push-relay/test/relay.test.tsapps/push-relay/tsconfig.jsonapps/push-relay/wrangler.jsoncapps/tunnel-relay/.gitignoreapps/tunnel-relay/README.mdapps/tunnel-relay/package.jsonapps/tunnel-relay/src/index.tsapps/tunnel-relay/src/relay.tsapps/tunnel-relay/src/tunnelDo.tsapps/tunnel-relay/test/relay.test.tsapps/tunnel-relay/test/smoke.mjsapps/tunnel-relay/tsconfig.jsonapps/tunnel-relay/wrangler.jsoncpackage.json
✅ Files skipped from review due to trivial changes (8)
- apps/push-relay/package.json
- apps/tunnel-relay/.gitignore
- apps/push-relay/wrangler.jsonc
- apps/tunnel-relay/tsconfig.json
- apps/push-relay/tsconfig.json
- apps/ade-cli/README.md
- apps/tunnel-relay/README.md
- apps/push-relay/README.md
🚧 Files skipped from review as they are similar to previous changes (59)
- apps/tunnel-relay/package.json
- apps/tunnel-relay/wrangler.jsonc
- package.json
- apps/tunnel-relay/src/index.ts
- apps/push-relay/src/index.ts
- apps/desktop/src/preload/preload.ts
- apps/ios/ADE/Views/Work/WorkRootComponents.swift
- apps/ios/ADE/App/DeepLinkRouter.swift
- apps/desktop/src/shared/ipc.ts
- apps/ios/ADE/ADE.entitlements
- apps/desktop/src/preload/global.d.ts
- apps/ios/ADE/Views/Work/WorkStatusAndFormattingHelpers.swift
- apps/tunnel-relay/test/relay.test.ts
- apps/ade-cli/src/multiProjectRpcServer.ts
- apps/ios/ADE/Views/Work/WorkRootScreen+Actions.swift
- apps/ios/ADE/Views/Settings/SettingsPinSheet.swift
- apps/ios/ADEWidgets/ADEWidgetBundle.swift
- apps/ios/ADE/Views/Work/WorkNewChatScreen.swift
- apps/ade-cli/src/services/sync/syncTunnelClientService.test.ts
- apps/ios/ADE/Models/RemoteModels.swift
- apps/desktop/src/main/services/ipc/registerIpc.ts
- apps/ios/ADE/Info.plist
- apps/ios/ADE/App/ADEApp.swift
- apps/tunnel-relay/test/smoke.mjs
- apps/ade-cli/src/cli.ts
- apps/desktop/src/shared/pairingQr.test.ts
- apps/ade-cli/src/services/sync/syncPairingStore.ts
- apps/ios/ADE/Views/Settings/SettingsPairingSection.swift
- apps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.ts
- apps/ios/ADE/Views/Settings/SettingsSupportTypes.swift
- apps/ios/ADE/Shared/ADEAgentActivityAttributes.swift
- apps/desktop/src/shared/types/sync.ts
- apps/ios/ADE/Services/PairingQrPayload.swift
- apps/desktop/src/shared/types/push.ts
- apps/ios/ADE/App/ADEAppDelegate.swift
- apps/ios/ADEWidgets/ADEAgentActivityWidget.swift
- apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
- apps/ios/ADE/Services/DpopKeyService.swift
- apps/push-relay/migrations/0001_push_registrations.sql
- apps/ios/ADETests/ADETests.swift
- apps/ios/ADE/Views/Work/WorkRootScreen.swift
- apps/push-relay/src/apns.ts
- apps/ios/ADE/Views/Settings/ConnectionSettingsView.swift
- apps/tunnel-relay/src/relay.ts
- apps/ade-cli/src/services/sync/syncDpop.ts
- apps/tunnel-relay/src/tunnelDo.ts
- apps/push-relay/test/relay.test.ts
- apps/desktop/src/renderer/components/settings/SyncDevicesSection.tsx
- apps/desktop/src/shared/pairingQr.ts
- apps/ade-cli/src/services/sync/syncHostService.ts
- apps/ios/ADE/Services/LiveActivityService.swift
- apps/ios/ADE/Views/Settings/SettingsPushDeliverySection.swift
- apps/ade-cli/src/services/push/pushRelayClient.ts
- apps/ade-cli/src/services/push/pushPublisherService.test.ts
- .github/workflows/ci.yml
- apps/ios/ADE/Services/PushNotificationService.swift
- apps/push-relay/src/relay.ts
- apps/ios/ADE/Services/SyncService.swift
- apps/ade-cli/src/services/sync/syncService.ts
|
@codex review |
arul28
force-pushed
the
ade/mobile-push-relay-platform-c1ebe009
branch
from
c50d7a2 to
36737c2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36737c2f17
ℹ️ 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".
| const landed = laOutcomes.length === 0 || laOutcomes.some((o) => o.delivered || o.suppressed); | ||
| if (landed) { | ||
| laPlan.commit(); |
There was a problem hiding this comment.
When the relay returns no liveactivity outcomes, this treats the publish as landed and commits the Live Activity fingerprint. That happens for update/end publishes before the phone has reported the per-activity token (or after the relay has no matching device row), so no APNs update is sent, the new state is locally deduped, and handleLiveActivityToken does not trigger a resend when the token arrives; the Lock Screen activity can stay stuck on the old content until another distinct agent event occurs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8870601: a real outcomes array with zero landed liveactivity entries no longer commits (legacy/mock responses without an outcomes field keep the old behavior), and handleLiveActivityToken pokes a flush when a token arrives so the activity catches up immediately.
| // be paired; unknown devices must pair via the PIN flow. Existing | ||
| // paired phones from older releases may not have a host PIN configured | ||
| // yet, and should still be able to reconnect with their stored token. | ||
| return !alreadyPaired; | ||
| return bootstrapPairingRecord == null; |
There was a problem hiding this comment.
When the require-DPoP setting/env is enabled, a legacy paired device with no stored DPoP key can still authenticate through this bootstrap-token branch because only records that already have dpopPublicKey are rejected. That bypasses the documented force-repair/force-re-pair posture for previously paired devices on the LAN (the brain ingress mirrors the same check), leaving the shared bootstrap token sufficient for those device IDs instead of requiring a DPoP-bound pairing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8870601: with require-DPoP on, the shared bootstrap token no longer satisfies a LAN hello for ANY device (keyless legacy pairings included) — enforced on both the project host and the brain ingress; loopback-only mode retains the historical local-process behavior.
arul28
force-pushed
the
ade/mobile-push-relay-platform-c1ebe009
branch
from
36737c2 to
8870601
Compare
|
@codex review |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8870601211
ℹ️ 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".
| logger, | ||
| store, | ||
| relayClient: createPushRelayClient({ store, logger }), | ||
| machineName: os.hostname(), |
There was a problem hiding this comment.
On macOS this uses the network hostname, but the phone stores the paired machine as the sync host identity name (the device display name/ComputerName) and LiveActivityService.endOrphanedActivities compares activity.attributes.machineName against SyncService.hostName. When those names differ (common on macOS), foregrounding the app treats the current machine's valid activity as orphaned and ends it immediately, so remote Live Activities disappear after the next foreground transition.
Useful? React with 👍 / 👎.
| if apnsTokenHex != nil { | ||
| Task { await registerDevice() } |
There was a problem hiding this comment.
When ActivityKit yields a push-to-start token while apnsTokenHex is still nil (for example the user denied alert notification permission, or APNs has not called back yet), this guard skips registerDevice() and the token is never sent to the brain. The new push DTO/store explicitly support push-to-start-only registrations, so these devices cannot receive remote Live Activity starts until an alert APNs token happens to arrive.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/ade-cli/src/services/push/pushPublisherService.ts`:
- Around line 529-545: The publish flow in pushPublisherService currently only
retries when relayClient.publish returns zero delivered and some failed, but
partial failures still commit alert fingerprints and block retries. Update the
publish handling around readOutcomeCount, alertCommits, and scheduleRetry so
that any failed target with delivered > 0 also re-queues the affected alerts
instead of committing dedupe/Live-Activity state immediately. Keep the existing
all-failed path, but expand the logic to detect partial relay failures, preserve
pendingAlerts for retry, and only update lastAlertFingerprintByKey after a fully
successful publish.
In `@apps/ade-cli/src/services/push/pushRegistrationStore.ts`:
- Around line 129-133: The device normalization in pushRegistrationStore’s load
path can crash when a persisted entry is malformed because `isValid()` only
validates the `devices` container, not each entry. Update the
`Object.entries(parsed.devices).map(...)` logic to guard each `device` before
dereferencing `device.prefs`, and either skip invalid entries or filter them out
before building `devices` so `normalizePrefs` only runs on valid objects.
- Line 93: The in-memory cache in PushRegistrationStore can become stale across
multiple ADE runtimes and overwrite newer data in push-relay.json. Update the
mutation path in PushRegistrationStore so each write re-reads and merges the
latest file state before saving, or add a file lock around the write flow to
serialize updates. Use the existing PushRegistrationStore cache and the methods
that persist the registration data to ensure device/prefs changes from other
processes are not clobbered.
In `@apps/ade-cli/src/services/sync/syncPairingStore.ts`:
- Around line 79-89: The pairing record write in pairPeer currently stores any
non-empty dpopPublicKey value, which can leave invalid key material in
syncPairingStore. Validate the incoming key before assigning it to
records[peer.deviceId], using the same Base64 X9.63 P-256 expectations already
implied by adoptDpopPublicKey and the rest of the sync flow, and reject/throw on
malformed input so only usable DPoP keys are persisted.
In `@apps/ade-cli/src/services/sync/syncTunnelClientService.ts`:
- Around line 126-157: `claimOnce()` and the tunnel connect flow in
`connectControl()` can hang indefinitely because the relay POST and subsequent
WebSocket opens have no deadline. Add abort/open timeouts to the claim request
and to each relay/local WebSocket connection path, then treat timeout as a
failure that updates `lastError`, logs via
`log.warn?.("sync_tunnel.claim_failed", ...)` or the equivalent connect error
handler, and triggers `scheduleReconnect()`. Make the same timeout handling
consistent across the control, pipe, and local connect helpers so `start()`
never stalls on a dead relay or sync port.
In `@apps/desktop/src/renderer/components/settings/SyncDevicesSection.tsx`:
- Around line 677-678: The pairing QR payload is missing the enabled cloud relay
route because PairQrPanel only gets connectInfo while buildPairingQrPayload can
also take relayUrl. Update the PairQrPanel usage and its props so it receives
the active relay URL when cloud relay fallback is enabled, and pass that value
through to buildPairingQrPayload alongside connectInfo. Check the PairQrPanel
component and any callers in SyncDevicesSection so the scanned QR matches the
relay the UI advertises.
In `@apps/tunnel-relay/test/smoke.mjs`:
- Around line 49-52: The smoke test WebSocket bridge in the local WebSocket
setup can drop the first frame if either side receives a message before the
other socket reaches OPEN. Update the bridging logic around pipe and local so
messages are buffered until both connections are open, then flushed in order;
use the existing closeBoth, pipe.on("message"), and local.on("message") wiring
in smoke.mjs as the place to add the buffering state.
🪄 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: e19dfb08-9765-4e4c-a69b-2c809a731982
⛔ Files ignored due to path filters (7)
apps/ios/ADE.xcodeproj/project.pbxprojis excluded by!**/*.xcodeproj/project.pbxprojapps/push-relay/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsonapps/tunnel-relay/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsondocs/ARCHITECTURE.mdis excluded by!docs/**docs/features/sync-and-multi-device/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**docs/features/sync-and-multi-device/push-notifications.mdis excluded by!docs/**
📒 Files selected for processing (80)
.github/workflows/ci.yml.gitignoreapps/ade-cli/README.mdapps/ade-cli/src/bootstrap.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/multiProjectRpcServer.tsapps/ade-cli/src/services/push/pushPublisherService.test.tsapps/ade-cli/src/services/push/pushPublisherService.tsapps/ade-cli/src/services/push/pushRegistrationStore.tsapps/ade-cli/src/services/push/pushRelayClient.tsapps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.tsapps/ade-cli/src/services/sync/syncCloudRelayStore.tsapps/ade-cli/src/services/sync/syncDpop.test.tsapps/ade-cli/src/services/sync/syncDpop.tsapps/ade-cli/src/services/sync/syncHostService.tsapps/ade-cli/src/services/sync/syncPairingStore.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.tsapps/ade-cli/src/services/sync/syncSecurityStore.tsapps/ade-cli/src/services/sync/syncService.tsapps/ade-cli/src/services/sync/syncTunnelClientService.test.tsapps/ade-cli/src/services/sync/syncTunnelClientService.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.tsapps/desktop/src/main/services/shared/utils.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/components/settings/SyncDevicesSection.tsxapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/pairingQr.test.tsapps/desktop/src/shared/pairingQr.tsapps/desktop/src/shared/types/push.tsapps/desktop/src/shared/types/sync.tsapps/ios/ADE/ADE.entitlementsapps/ios/ADE/App/ADEApp.swiftapps/ios/ADE/App/ADEAppDelegate.swiftapps/ios/ADE/App/DeepLinkRouter.swiftapps/ios/ADE/Info.plistapps/ios/ADE/Models/RemoteModels.swiftapps/ios/ADE/Services/DpopKeyService.swiftapps/ios/ADE/Services/LiveActivityService.swiftapps/ios/ADE/Services/PairingQrPayload.swiftapps/ios/ADE/Services/PushNotificationService.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Shared/ADEAgentActivityAttributes.swiftapps/ios/ADE/Views/Settings/ConnectionSettingsView.swiftapps/ios/ADE/Views/Settings/SettingsPairingScannerSheet.swiftapps/ios/ADE/Views/Settings/SettingsPairingSection.swiftapps/ios/ADE/Views/Settings/SettingsPinSheet.swiftapps/ios/ADE/Views/Settings/SettingsPushDeliverySection.swiftapps/ios/ADE/Views/Settings/SettingsSupportTypes.swiftapps/ios/ADE/Views/Work/WorkNewChatScreen.swiftapps/ios/ADE/Views/Work/WorkRootComponents.swiftapps/ios/ADE/Views/Work/WorkRootScreen+Actions.swiftapps/ios/ADE/Views/Work/WorkRootScreen.swiftapps/ios/ADE/Views/Work/WorkStatusAndFormattingHelpers.swiftapps/ios/ADETests/ADETests.swiftapps/ios/ADETests/PairingAndDpopTests.swiftapps/ios/ADEWidgets/ADEAgentActivityWidget.swiftapps/ios/ADEWidgets/ADEWidgetBundle.swiftapps/push-relay/README.mdapps/push-relay/migrations/0001_push_registrations.sqlapps/push-relay/package.jsonapps/push-relay/src/apns.tsapps/push-relay/src/index.tsapps/push-relay/src/relay.tsapps/push-relay/test/relay.test.tsapps/push-relay/tsconfig.jsonapps/push-relay/wrangler.jsoncapps/tunnel-relay/.gitignoreapps/tunnel-relay/README.mdapps/tunnel-relay/package.jsonapps/tunnel-relay/src/index.tsapps/tunnel-relay/src/relay.tsapps/tunnel-relay/src/tunnelDo.tsapps/tunnel-relay/test/relay.test.tsapps/tunnel-relay/test/smoke.mjsapps/tunnel-relay/tsconfig.jsonapps/tunnel-relay/wrangler.jsoncpackage.json
✅ Files skipped from review due to trivial changes (5)
- apps/tunnel-relay/.gitignore
- apps/push-relay/README.md
- apps/ios/ADE/ADE.entitlements
- .gitignore
- apps/tunnel-relay/README.md
🚧 Files skipped from review as they are similar to previous changes (61)
- apps/tunnel-relay/package.json
- apps/push-relay/src/index.ts
- apps/ios/ADEWidgets/ADEWidgetBundle.swift
- apps/desktop/src/preload/global.d.ts
- apps/tunnel-relay/tsconfig.json
- apps/ios/ADE/App/ADEApp.swift
- apps/tunnel-relay/src/index.ts
- apps/tunnel-relay/wrangler.jsonc
- apps/push-relay/wrangler.jsonc
- apps/ios/ADE/Models/RemoteModels.swift
- apps/ios/ADE/Info.plist
- apps/ios/ADE/Views/Work/WorkRootScreen+Actions.swift
- apps/desktop/src/main/services/ipc/registerIpc.ts
- apps/ios/ADE/Views/Work/WorkRootScreen.swift
- apps/tunnel-relay/test/relay.test.ts
- apps/push-relay/tsconfig.json
- apps/ade-cli/src/multiProjectRpcServer.ts
- apps/desktop/src/shared/types/push.ts
- apps/ade-cli/README.md
- apps/ade-cli/src/services/sync/syncTunnelClientService.test.ts
- apps/desktop/src/preload/preload.ts
- apps/ios/ADE/App/DeepLinkRouter.swift
- apps/ios/ADE/Views/Settings/SettingsPairingSection.swift
- apps/ios/ADE/App/ADEAppDelegate.swift
- apps/ios/ADE/Views/Work/WorkRootComponents.swift
- apps/push-relay/package.json
- apps/ios/ADE/Views/Work/WorkStatusAndFormattingHelpers.swift
- apps/ios/ADE/Views/Work/WorkNewChatScreen.swift
- apps/desktop/src/shared/types/sync.ts
- apps/ade-cli/src/cli.ts
- apps/ios/ADETests/ADETests.swift
- apps/ios/ADEWidgets/ADEAgentActivityWidget.swift
- apps/ios/ADE/Views/Settings/SettingsPinSheet.swift
- apps/desktop/src/shared/ipc.ts
- apps/ade-cli/src/services/sync/syncHostService.ts
- apps/ios/ADE/Services/DpopKeyService.swift
- package.json
- apps/ios/ADE/Views/Settings/SettingsSupportTypes.swift
- apps/push-relay/migrations/0001_push_registrations.sql
- apps/ade-cli/src/services/push/pushPublisherService.test.ts
- apps/desktop/src/shared/pairingQr.ts
- apps/ade-cli/src/bootstrap.ts
- apps/push-relay/test/relay.test.ts
- apps/ios/ADE/Views/Settings/ConnectionSettingsView.swift
- apps/desktop/src/shared/pairingQr.test.ts
- apps/ade-cli/src/services/push/pushRelayClient.ts
- apps/ios/ADE/Views/Settings/SettingsPushDeliverySection.swift
- apps/push-relay/src/apns.ts
- apps/ios/ADE/Shared/ADEAgentActivityAttributes.swift
- apps/ade-cli/src/services/sync/syncDpop.ts
- apps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.ts
- apps/ios/ADE/Services/PairingQrPayload.swift
- apps/tunnel-relay/src/tunnelDo.ts
- .github/workflows/ci.yml
- apps/tunnel-relay/src/relay.ts
- apps/ade-cli/src/services/sync/syncService.ts
- apps/ios/ADE/Services/PushNotificationService.swift
- apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
- apps/push-relay/src/relay.ts
- apps/ios/ADE/Services/SyncService.swift
- apps/ios/ADE/Services/LiveActivityService.swift
… DPoP + smart QR - apps/push-relay (new CF Worker, deployed): D1 device registrations, machine-claim + HMAC-signed publish API, APNs HTTP/2 (ES256 .p8 JWT), phase TTLs (2h running / 24h waiting), delivery-gated content-hash suppression, dead-token cleanup, Live Activity start/update/end routing. - Brain publisher (ade-cli services/push): subscribes agentChatService / ptyService.onExit / prPollingService pr-notifications; debounced + deduped; per-device prefs enforced brain-side (mute, quiet hours in device timezone); push.* runtime sync commands for phone registration. - iOS: aps-environment + remote-notification; registration after pairing; alert pushes deep-link via ade://; ADEAgentRunsAttributes Live Activity (lock screen + Dynamic Island, up to 3 runs) with push-to-start + foreground re-registration + orphan reaping; Push delivery settings panel (status, toggles, quiet hours). - apps/tunnel-relay (new DO Worker, deployed): phone->relay->brain WS tunnel with per-connection pipe sockets and pre-open frame buffering on both hops; brain tunnel client with backoff; "Cloud relay fallback" toggle (desktop + iOS, default off); relay advertised as the lowest-priority address candidate. - DPoP pairing hardening: Secure Enclave P-256 key registered at pairing; every paired hello signs a fresh secret-bound challenge; TOFU upgrade for legacy devices; fail-closed once a key is on record; bootstrap downgrade path blocked; requireDpop machine setting enforced on both the project host and brain ingress (+ ade sync security/relay CLI). - One-QR smart pairing: desktop QR encodes https://ade-app.dev/pair payload (identity + port + candidates + relay URL); iOS scanner with already-paired instant reconnect or PIN-only pair; universal-link route. - Offline outbox: queued chat.create persists a creation snapshot and renders a pending Work-list row that reconciles on drain. - Tests: syncDpop contract suite (replay/skew/TOFU/fail-closed + requireDpop regression), cross-app HMAC golden vectors, consolidated push/cloud-relay suites; docs updated (sync README, ios-companion, ARCHITECTURE relay-workers section). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
arul28
force-pushed
the
ade/mobile-push-relay-platform-c1ebe009
branch
from
8870601 to
1e8c9b6
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e8c9b66d8
ℹ️ 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".
| PushNotificationService.shared.handleUnpair() | ||
| disconnect(clearCredentials: true) |
There was a problem hiding this comment.
When a user forgets a paired host, this schedules push.unregisterDevice inside handleUnpair() but immediately calls disconnect(clearCredentials: true). Because push.unregisterDevice is a live, non-queued sync command (sendPushCommand throws once offline), the task can run after the socket has been torn down and the desktop keeps this device's APNs registration, so an unpaired phone can continue receiving pushes from that machine. Await the unregister (or otherwise make it complete before closing the sync connection) on the connected path.
Useful? React with 👍 / 👎.
1 participant
Mobile Push & Relay Platform
Cloud connectivity for ADE mobile: APNs push + Live Activities, a Cloudflare push relay, a Durable Objects WebSocket tunnel relay, DPoP pairing hardening, one-QR smart pairing, and an offline pending-chat row.
What's here
apps/push-relay/(new CF Worker, deployed) — D1 device registrations, machine-claim + HMAC-signed API, APNs HTTP/2 (ES256.p8JWT), phase TTLs (2h running / 24h waiting), content-hash suppression, dead-token cleanup, Live Activity start/update/end routing (input-push-tokenminting, 10-min stale default).apps/ade-cli/src/services/push/) — subscribesagentChatServiceevents,ptyService.onExit, andprPollingServicepr-notifications; debounced + double-deduped; per-device prefs (mute, quiet hours in device timezone) enforced brain-side;push.*sync commands for phone registration.ade://deep links,ADEAgentRunsAttributesLive Activity (lock screen + Dynamic Island, ≤3 runs) with push-to-start + foreground re-registration + orphan reaping, and a Settings "Push delivery" panel (status, toggles, quiet hours).apps/tunnel-relay/(new DO Worker, deployed) — phone→relay→brain WS tunnel (per-connection pipe sockets, early-frame buffering, idle sweep); brain tunnel client with backoff; "Cloud relay fallback" toggle (desktop + iOS, default off); relay advertised as lowest-priority address candidate.requireDpopmachine setting (+ade sync security require-dpop).https://ade-app.dev/pair#<payload>(identity + port + candidates + relay URL); iOS scanner: already-paired → instant reconnect, new machine → PIN-only.chat.createpersists a creation snapshot and renders a pending Work-list row that reconciles on drain.Verification
Needs a physical device / operator
wrangler secret put APNS_KEY / APNS_KEY_ID / APNS_TEAM_IDinapps/push-relay(relay/healthreportsapnsConfigured:falseuntil then).🤖 Generated with Claude Code
Summary by CodeRabbit
Greptile Summary
This PR adds mobile push delivery and cloud relay support for ADE. The main changes are:
requireDpopcontrols.Confidence Score: 5/5
This PR appears safe to merge from the reviewed changed paths.
No review findings were identified in the core push relay, tunnel relay, DPoP authentication, push publisher, QR, and iOS routing changes. Previously reported issues are addressed in the current diff, including relay buffer overflow closure, pre-open pipe buffering, delivery outcome checks, and post-delivery suppression recording.
No files require special attention.
What T-Rex did
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Phone as iOS phone participant Brain as ADE brain participant Push as Cloudflare push relay participant APNs as APNs participant Tunnel as Tunnel Durable Object Phone->>Brain: Pair via QR/PIN and register DPoP public key Phone->>Brain: push.registerDevice / prefs / activity token Brain->>Push: Claim machine and HMAC-signed device registration Brain->>Push: Publish alert / Live Activity updates Push->>APNs: APNs HTTP/2 alert or liveactivity push APNs-->>Phone: Notification / Live Activity update Brain->>Tunnel: Signed host control WebSocket Phone->>Tunnel: WebSocket connect using relay candidate Tunnel-->>Brain: open(id) Brain->>Tunnel: Signed pipe WebSocket Phone<<->>Brain: ADE sync protocol frames relayed byte-for-byte%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Phone as iOS phone participant Brain as ADE brain participant Push as Cloudflare push relay participant APNs as APNs participant Tunnel as Tunnel Durable Object Phone->>Brain: Pair via QR/PIN and register DPoP public key Phone->>Brain: push.registerDevice / prefs / activity token Brain->>Push: Claim machine and HMAC-signed device registration Brain->>Push: Publish alert / Live Activity updates Push->>APNs: APNs HTTP/2 alert or liveactivity push APNs-->>Phone: Notification / Live Activity update Brain->>Tunnel: Signed host control WebSocket Phone->>Tunnel: WebSocket connect using relay candidate Tunnel-->>Brain: open(id) Brain->>Tunnel: Signed pipe WebSocket Phone<<->>Brain: ADE sync protocol frames relayed byte-for-byteReviews (8): Last reviewed commit: "Mobile push & relay platform: APNs + Liv..." | Re-trigger Greptile