Skip to content

Directed presence channel: server can wake the Mac (nudge push) - #9012

Merged
azooz2003-bit merged 9 commits into
mainfrom
feat-mac-presence-subscribe
Jul 29, 2026
Merged

azooz2003-bit merged 9 commits into
mainfrom
feat-mac-presence-subscribe

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 27, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The Mac publishes to presence (15s heartbeats) and the iroh trust broker (signed registration) but receives nothing. A server-side change to its binding — revocation, or the newest-wins replacement coming in #8883 — only reaches it on the next scheduled broker round trip, up to ~45 minutes later. The App Store review incident sat in exactly that blind spot. Phones already hold a presence WebSocket; this gives the Mac the equivalent, as a quiet directed channel.

Mechanism

Presence worker. GET /v1/presence/subscribe?deviceScope=<deviceId> turns the stream into a directed nudge channel: WebSocket-only, no snapshot, no team presence chatter, no sync. Authorization reuses the first-heartbeat device-owner pin (an unpinned device may subscribe, since the Mac connects at startup possibly before its first heartbeat, but subscribing never writes the pin). A new POST /v1/presence/nudge {deviceId, tag?, kind} (owner-only, kind from a server-side allowlist, currently iroh-binding-changed) delivers a {type: "nudge"} frame to that device's scoped sockets and reports delivered (0 is success: an offline Mac catches up via its normal round trips). Nudge frames go only to device-scoped sockets, mirroring the sync.hello gating, so legacy presence decoders that throw on unknown event types never see one. The frame carries no route or binding data.

Mac app. PresenceNudgeSubscriber (mirrors PresenceHeartbeatClient gating: same enable flag, service URL resolution, auth) holds the directed stream with 1s→60s reconnect backoff, reset when a stream delivers; the service bounds streams to token expiry, so clean close plus resubscribe is steady state. A nudge for this device and build tag calls the new CmxIrohHostRuntime.requestRegistrationRefresh(), one immediate registration/policy round through the existing coalesced refresh path, plus retryIfNeeded() so an absent runtime reactivates. Composes with #8930: once both merge, a nudge also instantly rebuilds a terminally failed runtime.

Compatibility. Against a pre-nudge worker the same endpoint serves snapshot/presence frames instead. The subscriber bounds messages at the transport (maximumMessageSize 2 KiB, so a team snapshot fails the receive before it is buffered) and treats the first non-nudge frame as proof of a legacy endpoint: it closes and re-probes every 15 minutes, costing one small handshake per probe instead of pumping team traffic through the main actor. Old clients are untouched (no new frame ever reaches a non-scoped socket).

Principled or hacky: principled — it reuses the existing presence transport, DO, auth, and ownership pin rather than adding a service, and the channel is a latency optimization layered over pull-is-truth, so its failure modes cost freshness, never correctness. Residual risk: directed sockets hold their own admission pool (256 per team, 32 per user, pure-tested checkSubscriberAdmission), so a Mac fleet can never 429 the phones' presence streams and one member cannot park sockets on arbitrary UUIDs until co-members' channels starve. The first-writer owner pin's known residual extends to nudges deliberately (documented in do.ts/README): a squatted pin suppresses only the acceleration, and the Mac falls back to its pre-nudge renewal cadence.

Deliberately deferred (PR 2)

The broker hook that fires the nudge on revocation/replacement lands after #8883 merges, because those trustBroker.ts/repository.ts mutation paths are being rewritten there and the replacement event only exists with its newest-wins semantics. Until then the endpoint is independently drivable (authenticated curl or a future phone-side "reconnect my Mac" action). That hook also owns the displaced-instance disposition: a replacement nudge should carry authoritative supersession so an old process can stop instead of re-registering through the mutating renewal path and reclaiming the newest-wins slot. Until emission exists a nudge only accelerates the renewal round that main already runs on its 45-minute timer, so this PR changes no displacement semantics.

Verification

  • Worker: bun test 195 pass (21 new in test/nudge.test.ts: parsing, owner-scoped delivery, split admission pools), tsgo typecheck clean.
  • Package: 41 CmxIrohHostRuntime* tests pass, 3 new in CmxIrohHostRuntimeRequestedRefreshTests (immediate broker round when active; no-op when inactive; a replaced binding settles .failed by the time the await returns).
  • Tagged fleet build macsub.
  • The worker deploy (dev, then prod) happens at merge time; until the dev worker is deployed, the tagged Mac build connects to the old worker and idles harmlessly, which is the compatibility path above.

No user-facing strings were added (log lines only), so no localization changes are needed.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Adds a device-scoped presence channel so the server can nudge a Mac and trigger an immediate iroh registration refresh after binding changes. Nudges only change when the renewal runs, not what it does.

  • New Features

    • Worker: GET /v1/presence/subscribe?deviceScope=<deviceId> is a WebSocket-only nudge channel; POST /v1/presence/nudge {deviceId, tag?, kind} (owner-only; allowlisted kind: iroh-binding-changed) sends {type:"nudge"} only to device‑scoped sockets and returns a delivered count. Separate caps: 256 directed sockets, 64 presence subscribers, and 32 directed per user.
    • Mac: PresenceNudgeSubscriber holds the directed stream with 1s→60s backoff, converts https/http→wss/ws, enforces a 2 KiB transport bound, treats quiet deadline closes as healthy, restarts on auth/team/service‑URL changes, and shuts down on disable. On a matching nudge it calls CmxIrohHostRuntime.requestRegistrationRefresh() (single‑flight with replay; accelerates the existing renewal round only) and rebuilds the runtime on a replaced binding.
  • Bug Fixes

    • Await across the coalesced refresh replay; burst nudges coalesce into one follow‑up round; absent runtimes trigger retryIfNeeded().
    • Revalidate delivery ownership per frame and route only to device‑scoped sockets; exclude wrong‑device, legacy (no user), expired, or lost‑pin‑race sockets.
    • Detect legacy endpoints without parsing: 2 KiB maximumMessageSize causes EMSGSIZE on snapshots; first foreign/oversized frame marks legacy and re‑probes every 15 minutes. Clean, empty closes count as served only after ≥60 s to avoid accept‑then‑close loops.

Written for commit 0c093b6. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added directed, device-scoped presence nudges that trigger single-flight, coalesced registration refreshes for faster recovery after server binding changes.
    • Introduced an authenticated POST /v1/presence/nudge endpoint and push-only device-scoped WebSocket subscriptions.
    • Automatically manages the nudge subscriber during app startup and termination.
    • Added a public async method to request an immediate registration refresh when the runtime is active.
  • Bug Fixes
    • Improved resilience when server-side bindings change during a refresh cycle.
  • Documentation
    • Updated presence worker docs with directed nudge and validation semantics.
  • Tests
    • Added coverage for nudge parsing/delivery eligibility and requested-refresh behavior.

The Mac publishes to presence and the broker but receives nothing, so a
server-side change to its iroh binding (revocation, re-key replacement)
only reached it on the next scheduled broker round trip, up to ~45
minutes later. The phone already holds a presence WebSocket; this adds
the Mac-side equivalent as a quiet directed channel.

Presence worker: `?deviceScope=<deviceId>` on the subscribe route turns
the stream into a WebSocket-only nudge channel — no snapshot, no team
presence chatter, no sync — gated by the same first-heartbeat owner pin
as heartbeats (subscribing never writes the pin). A new owner-only
`POST /v1/presence/nudge {deviceId, tag?, kind}` delivers a
`{type: "nudge"}` frame to that device's scoped sockets. Nudges are
never sent to normal subscribers, mirroring how sync frames are gated
on `sync.hello`, so legacy presence decoders that throw on unknown
event types never see one. Kinds are a server-side allowlist
(`iroh-binding-changed`); the frame carries no route or binding data.

Mac app: `PresenceNudgeSubscriber` mirrors `PresenceHeartbeatClient`'s
gating and holds the directed stream with 1s→60s reconnect backoff. A
nudge for this device (and build tag, when given) calls the new
`CmxIrohHostRuntime.requestRegistrationRefresh()` — one immediate
registration/policy round through the existing coalesced refresh path —
plus `retryIfNeeded()` for absent runtimes. Against a pre-nudge worker
the same endpoint serves snapshot/presence frames, which the subscriber
ignores, so old servers degrade to a no-op.

The broker-side hook that fires the nudge on revocation/replacement is
deliberately a follow-up: those mutation paths are being rewritten by
the in-flight binding re-key work (PR 8883), and the endpoint is
independently drivable until then.

Worker: bun test 183 pass (9 new), typecheck clean. Package: 37
CmxIrohHostRuntime tests pass (2 new for requestRegistrationRefresh).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 27, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds authenticated, device-scoped presence nudges through the worker and mobile WebSocket subscriber. Accepted nudges trigger mobile Iroh registration refreshes, with lifecycle wiring, runtime APIs, validation, documentation, project integration, and tests.

Changes

Presence nudge registration refresh

Layer / File(s) Summary
Nudge contracts and API validation
workers/presence/src/core.ts, workers/presence/src/validate.ts, workers/presence/src/index.ts, workers/presence/test/nudge.test.ts, workers/presence/README.md
Defines nudge payloads, validates device scopes and allowed kinds, exposes POST /v1/presence/nudge, documents the protocol, and tests validator behavior.
Device-scoped nudge channels
workers/presence/src/do.ts, workers/presence/src/core.ts, workers/presence/test/nudge.test.ts
Adds owner-authorized WebSocket channels, targeted nudge delivery, expiry handling, push-only behavior, delivery filtering, and exclusion from regular presence broadcasts.
Runtime refresh API and coverage
Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohHostRuntime+PublicAPI.swift, Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohHostRuntimeRequestedRefreshTests.swift
Adds an active-lifecycle registration refresh API and tests immediate, failed, and inactive behavior.
Mobile nudge subscription and refresh bridge
Sources/Cloud/PresenceNudgeSubscriber.swift, Sources/Mobile/MobileHostIrohRuntime.swift, Sources/AppDelegate.swift, cmux.xcodeproj/project.pbxproj
Adds the authenticated subscription loop, nudge filtering, lifecycle wiring, project compilation entry, and mobile registration refresh bridge.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant PresenceWorker
  participant PresenceNudgeSubscriber
  participant MobileHostIrohRuntime
  participant CmxIrohHostRuntime
  Caller->>PresenceWorker: POST /v1/presence/nudge
  PresenceWorker-->>PresenceNudgeSubscriber: deliver device-scoped nudge
  PresenceNudgeSubscriber->>MobileHostIrohRuntime: refreshRegistrationFromServerSignal()
  MobileHostIrohRuntime->>CmxIrohHostRuntime: requestRegistrationRefresh()
  CmxIrohHostRuntime-->>MobileHostIrohRuntime: settle registration refresh
Loading

Possibly related PRs

  • manaflow-ai/cmux#8930: Changes the mobile Iroh retry and failure-recovery flow used by this refresh integration.

Suggested reviewers: lawrencecchen


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors)

Check name Status Explanation Resolution
Cmux Swift Blocking Runtime ❌ Error FAIL: Sources/Cloud/PresenceNudgeSubscriber.swift adds a production reconnect loop that uses ContinuousClock.sleep for 1s→60s backoff and 15m reprobes. Replace the sleep loop with a cancellation-aware timer/async-sequence/callback or state-transition signal that resubscribes without sleeping in shipped code.
Cmux No Ambient Global State ❌ Error PresenceNudgeSubscriber is a new mutable static let shared singleton, so the diff introduces ambient runtime state forbidden by the rule. Construct PresenceNudgeSubscriber in the app composition root and inject it where needed; remove the global shared access and keep state scoped.
✅ Passed checks (23 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed New Swift code is MainActor-isolated or actor-isolated; added callers stay on MainActor, and AuthCoordinator is also @MainActor.
Cmux Browser Automation Off-Main ✅ Passed Diff only touches presence runtime/worker files; no browser-automation commands, routing, or WebKit/AppKit browser-state changes appear.
Cmux Expensive Synchronous Load ✅ Passed No diff hunk adds a main-actor agent-history load; the only new socket parse is a bounded ~2 KiB nudge frame, and the rest is async/coalesced refresh logic.
Cmux Cache Substitution Correctness ✅ Passed No fresh-authoritative-read swap found: presence snapshot/delivery paths still read storage, and runtime snapshot is updated in the same refresh flow before being checked.
Cmux No Hacky Sleeps ✅ Passed No non-Swift sleeps/delays were introduced; the TS changes are pure validation/routing plus deterministic tests, not lifecycle wait hacks.
Cmux Algorithmic Complexity ✅ Passed PASS: New scans are linear and bounded (≤64 tags, 2KB frames, tiny event arrays); no nested rescans or repeated hot-path sorting/filtering were introduced.
Cmux Swift Concurrency ✅ Passed New Tasks are lifecycle-tied or callback-bound; no new Combine, DispatchQueue background work, or completion-handler APIs were added in the diff.
Cmux Swift @Concurrent ✅ Passed PASS: requestRegistrationRefresh is actor-isolated on CmxIrohHostRuntime; no new nonisolated async needs @concurrent, and the nudge subscriber’s main-actor loop is intentional lifecycle/UI-bo...
Cmux Swift Package Boundaries ✅ Passed The reusable refresh logic was moved into CmuxIrohTransport; the new app-side nudge subscriber is lifecycle glue wiring auth, defaults, and the singleton runtime.
Cmux Swiftpm Lockfiles ✅ Passed Diff only changes source/docs files; no .gitignore, Package.resolved, Xcode project, or workflow dependency metadata changed.
Cmux Swift Logging ✅ Passed No new print/NSLog/debugPrint/dump or changed file-scoped Logger appears in the touched Swift diff; existing logs are preexisting.
Cmux User-Facing Error Privacy ✅ Passed New API errors are generic codes (invalid_*, device_scope_*, device_owner_mismatch); the only iroh text is in docs/logging, not user-facing errors.
Cmux Full Internationalization ✅ Passed No new user-facing Swift text; changes are control flow plus a debug log, and worker text is protocol/operational docs only.
Cmux Swiftui State Layout ✅ Passed Touched files are AppKit/runtime code; no new ObservableObject, @Published, GeometryReader, List/ForEach subtree, or render-time state mutation found.
Cmux Architecture Rethink ✅ Passed PASS: the new nudge bridge has explicit owners/invariants and mirrors existing presence-client lifecycle wiring; it doesn’t leave bad state representable.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR only adds PresenceNudgeSubscriber lifecycle wiring and iroh refresh logic; diff shows no new NSWindow/NSPanel/WindowGroup code or cmuxAuxiliaryWindowIdentifiers changes.
Cmux Source Artifacts ✅ Passed All changed paths are intentional source, test, docs, or Xcode config files; none match artifact/scratch/build-output patterns in the rule.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Touched production sources add only real product APIs/state; no new #if DEBUG test hooks or debug*/ForTesting members appear in the diff.
Title check ✅ Passed The title is concise and clearly describes the new directed presence nudge channel.
Description check ✅ Passed The description is detailed and covers summary and testing; the missing demo video, review trigger, and checklist sections are non-blocking.
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch feat-mac-presence-subscribe
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-mac-presence-subscribe

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@workers/presence/src/do.ts`:
- Around line 400-439: Update the delivery loop in nudge to require both the
matching device scope and wsUserId(ws) matching the current pinned owner before
sending the event. Preserve the existing expiry and delivery-count behavior,
ensuring sockets subscribed while the device was unpinned do not receive nudges
after another user claims it.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: ab9df5d7-85f5-416b-9aec-6dc6ff471e76

📥 Commits

Reviewing files that changed from the base of the PR and between e0cf205 and c6f0b6c.

📒 Files selected for processing (12)
  • Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohHostRuntime+PublicAPI.swift
  • Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohHostRuntimeRequestedRefreshTests.swift
  • Sources/AppDelegate.swift
  • Sources/Cloud/PresenceNudgeSubscriber.swift
  • Sources/Mobile/MobileHostIrohRuntime.swift
  • cmux.xcodeproj/project.pbxproj
  • workers/presence/README.md
  • workers/presence/src/core.ts
  • workers/presence/src/do.ts
  • workers/presence/src/index.ts
  • workers/presence/src/validate.ts
  • workers/presence/test/nudge.test.ts

Comment thread workers/presence/src/do.ts
azooz2003-bit and others added 2 commits July 27, 2026 22:12
…cycle, delivery ownership

Five review findings, all confirmed against the code:

- The subscribe URL kept the https scheme; URLSessionWebSocketTask needs
  wss, so the channel never connected. Convert https/http to wss/ws,
  same as the iOS PresenceClient.
- A nudge-triggered refresh that discovers the binding was replaced
  (different binding id) fails closed into the terminal .failed phase
  and nothing rebuilt it. requestRegistrationRefresh now awaits the
  refresh round settling, and the composition root reads the
  post-refresh snapshot and rebuilds through reconcile with
  restartActiveRuntime so a fresh activation re-registers under the new
  server state. New package test pins the fail-closed contract for a
  replaced binding id.
- evaluate() only toggled on/off, so a team or service-URL change rode
  the old socket to the 15-minute deadline. The loop is now keyed by a
  team+URL scope and restarts when the scope changes.
- URLSessionWebSocketTask.receive() ignores Swift task cancellation, so
  disabling presence left the socket suspended in receive until expiry.
  The receive loop runs under withTaskCancellationHandler that cancels
  the socket, and frames received after cancellation are dropped.
- The DO delivered nudges by deviceScope alone; a subscriber who lost
  the first-heartbeat pin race could still receive owner-only frames.
  Delivery now also requires the socket's verified user to equal the
  device's current pinned owner.

Worker: 183 bun tests pass, typecheck clean. Package: 38
CmxIrohHostRuntime tests pass (replaced-binding case new).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uiet-close backoff

Four fixes from the second structured review pass:

- requestRegistrationRefresh() now awaits across the coalesced replay
  round, not just the in-flight one, so a caller that rebuilds on
  `.failed` observes the state AFTER the replay the pending bit
  scheduled.
- PresenceNudgeSubscriber's scope key includes the authenticated user id
  and requires isAuthenticated, so two solo accounts (nil resolvedTeamID)
  can never share a directed stream scope, and auth identity changes
  restart the loop via an @observable tracking re-arm.
- MobileHostIrohRuntime.refreshRegistrationFromServerSignal() is
  single-flight with a pending bit: a burst of nudge frames coalesces
  into one follow-up refresh instead of fanning out one main-actor
  waiter per frame.
- subscribeOnce() treats a normal/going-away close as healthy service:
  a directed stream is silent between nudges, so the quiet 15-minute
  renewal close must reset backoff instead of doubling it toward 60s
  gaps that could swallow a one-shot nudge.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
Sources/Cloud/PresenceNudgeSubscriber.swift (3)

24-37: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Make the subscriber a composition-root-owned dependency.

shared adds ambient mutable runtime state under Sources/. Construct and retain this subscriber in the application lifecycle owner, then inject its AuthCoordinator; do not use a process-wide singleton for the socket loop.

🤖 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 `@Sources/Cloud/PresenceNudgeSubscriber.swift` around lines 24 - 37, Remove the
process-wide static shared instance from PresenceNudgeSubscriber and make the
class constructible by the application lifecycle owner. Have that owner retain a
single subscriber instance and inject its AuthCoordinator during initialization,
updating call sites to use the injected instance so the socket loop is
composition-root-owned rather than ambient global state.

Source: Coding guidelines


112-236: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Keep WebSocket receive and frame decoding off the main actor.

@MainActor class methods are isolated to the main actor, so task.receive() and handleFrame’s JSONSerialization run on the UI thread for every remote frame. Route the receive/decode work to a non-UI shared worker actor or explicit concurrent boundary, then forward only a validated nudge value snapshot back to @MainActor for the registration/logging action.

🤖 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 `@Sources/Cloud/PresenceNudgeSubscriber.swift` around lines 112 - 236, The
startLoop task and subscribeOnce currently perform WebSocket receiving and JSON
frame decoding on the main actor. Move receive and decoding out of `@MainActor`
into a non-UI worker actor or explicit concurrent boundary, and have that
boundary return only a validated nudge value snapshot; keep logging and
MobileHostIrohRuntime.shared.refreshRegistrationFromServerSignal() on the main
actor, replacing direct handleFrame calls accordingly.

Source: Coding guidelines


112-130: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Remove the sleeping reconnect retry loop.

startLoop() coordinates reconnect backoff with ContinuousClock().sleep(for:), creating a production timer-driven retry path. Move reconnect/refresh backoff to the transport/lifecycle owner or make this wait on an event/signaler rather than sleeping between WebSocket attempts.

🤖 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 `@Sources/Cloud/PresenceNudgeSubscriber.swift` around lines 112 - 130, Remove
the timer-driven reconnect loop from startLoop(), including ContinuousClock,
failureDelay, and clock.sleep(for:) backoff handling. Delegate reconnect/refresh
scheduling to the transport or lifecycle owner, or wait on its event/signaler
instead, while preserving cancellation and the existing subscribeOnce()
coordination.

Source: Coding guidelines

🤖 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.

Outside diff comments:
In `@Sources/Cloud/PresenceNudgeSubscriber.swift`:
- Around line 24-37: Remove the process-wide static shared instance from
PresenceNudgeSubscriber and make the class constructible by the application
lifecycle owner. Have that owner retain a single subscriber instance and inject
its AuthCoordinator during initialization, updating call sites to use the
injected instance so the socket loop is composition-root-owned rather than
ambient global state.
- Around line 112-236: The startLoop task and subscribeOnce currently perform
WebSocket receiving and JSON frame decoding on the main actor. Move receive and
decoding out of `@MainActor` into a non-UI worker actor or explicit concurrent
boundary, and have that boundary return only a validated nudge value snapshot;
keep logging and
MobileHostIrohRuntime.shared.refreshRegistrationFromServerSignal() on the main
actor, replacing direct handleFrame calls accordingly.
- Around line 112-130: Remove the timer-driven reconnect loop from startLoop(),
including ContinuousClock, failureDelay, and clock.sleep(for:) backoff handling.
Delegate reconnect/refresh scheduling to the transport or lifecycle owner, or
wait on its event/signaler instead, while preserving cancellation and the
existing subscribeOnce() coordination.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8d19c211-2203-4866-8364-c7d2d0ba4b7c

📥 Commits

Reviewing files that changed from the base of the PR and between 03a4346 and 05f63d2.

📒 Files selected for processing (3)
  • Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohHostRuntime+PublicAPI.swift
  • Sources/Cloud/PresenceNudgeSubscriber.swift
  • Sources/Mobile/MobileHostIrohRuntime.swift

azooz2003-bit and others added 2 commits July 28, 2026 19:43
Review round 3 flagged that the security-sensitive delivery filter (owner
re-check per frame, directed-socket routing) had no behavior coverage.
Following the suite's no-Workers-runtime pattern (checkDeviceOwner), the
per-socket decision moves into a pure shouldDeliverNudge in core.ts, the
DO delivery loop calls it, and tests cover: normal presence subscribers
never receive nudges, wrong-device scopes and expired sockets are
excluded, a subscriber who lost the first-heartbeat pin race is excluded
at delivery despite an accepted subscription, legacy sockets without a
verified user id never match, and a mixed subscriber set delivers to
exactly the pinned owner's directed socket.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… main

Review round 4 P1: against a pre-nudge worker, ?deviceScope= is ignored
and the directed socket degrades to a full presence subscription — a
team snapshot (megabytes at the service's caps) followed by seen events,
each JSON-parsed on the main actor before being discarded.

The receive loop now classifies each frame before parsing: anything over
a 2 KiB bound (a real nudge is ~200 bytes) is foreign in O(1), so a
snapshot is never parsed. The first foreign frame proves the endpoint is
legacy — a nudge-aware worker sends only nudge frames on a directed
stream — so the subscriber closes immediately and re-probes every 15
minutes instead of pumping team traffic. A legacy worker has no nudges
to deliver, so the slow probe loses nothing; once the worker upgrades,
the next probe holds a normal directed stream.

Also documents the deliberately accepted first-writer pin residual on
the nudge authorization path (do.ts, README): the presence worker keeps
no synchronous registry dependency by design, and a squatted pin only
suppresses the acceleration — the Mac falls back to its pre-nudge
renewal cadence, never to a correctness failure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Sources/Cloud/PresenceNudgeSubscriber.swift (1)

26-38: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Replace the ambient subscription singleton with an injected lifecycle owner.

This global owns mutable auth, observer, and socket-task state. Construct and retain it at the app composition root; have callbacks weakly capture that instance instead of routing through shared.

As per coding guidelines, “Avoid new ambient global runtime state … runtime singletons. Prefer constructable injectable owners.” Based on learnings, the singleton exception is for a sole app-wide persisted resource, which this auth-bound stream is not.

Also applies to: 51-54, 71-75

🤖 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 `@Sources/Cloud/PresenceNudgeSubscriber.swift` around lines 26 - 38, Replace
the PresenceNudgeSubscriber.shared singleton with an explicitly constructed,
retained instance owned by the app composition root. Remove the static shared
accessor, expose construction as needed, and update callbacks and references
around PresenceNudgeSubscriber to weakly capture and use the injected instance
while preserving its auth, observer, and loop lifecycle state.

Sources: Coding guidelines, Learnings

🤖 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 `@Sources/Cloud/PresenceNudgeSubscriber.swift`:
- Around line 221-234: Update the message switch in the nudge frame handling
path to return .legacyEndpoint immediately for .data messages, before
constructing or UTF-8 decoding text. Preserve string processing through
classifyAndHandleFrame, while keeping unknown message cases treated as legacy.

---

Outside diff comments:
In `@Sources/Cloud/PresenceNudgeSubscriber.swift`:
- Around line 26-38: Replace the PresenceNudgeSubscriber.shared singleton with
an explicitly constructed, retained instance owned by the app composition root.
Remove the static shared accessor, expose construction as needed, and update
callbacks and references around PresenceNudgeSubscriber to weakly capture and
use the injected instance while preserving its auth, observer, and loop
lifecycle 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: ASSERTIVE

Plan: Pro Plus

Run ID: 86ecb2bc-f9c3-4f48-a94d-91f79d2ca4f5

📥 Commits

Reviewing files that changed from the base of the PR and between 1d2d43b and 01e27c1.

📒 Files selected for processing (3)
  • Sources/Cloud/PresenceNudgeSubscriber.swift
  • workers/presence/README.md
  • workers/presence/src/do.ts

Comment on lines +221 to +234
let text: String?
switch message {
case let .string(string): text = string
case let .data(data): text = String(data: data, encoding: .utf8)
@unknown default: text = nil
}
// A nudge-aware worker sends nothing but small text nudge
// frames on a directed stream, so the first frame that is
// anything else (a snapshot, presence chatter, binary) proves
// the endpoint is legacy: stop pumping it instead of paying a
// main-actor decode for team-sized traffic.
guard let text, classifyAndHandleFrame(text) == .nudge else {
return .legacyEndpoint
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Reject binary frames before decoding them on MainActor.

Line 224 decodes the entire binary payload before the 2 KB guard at Line 259. A large binary legacy frame can still allocate and UTF-8 decode on the main actor. Binary is already treated as a foreign/legacy protocol frame, so return .legacyEndpoint directly.

Proposed fix
-                let text: String?
+                let text: String
                 switch message {
                 case let .string(string): text = string
-                case let .data(data): text = String(data: data, encoding: .utf8)
-                `@unknown` default: text = nil
+                case .data:
+                    return .legacyEndpoint
+                `@unknown` default:
+                    return .legacyEndpoint
                 }
-                guard let text, classifyAndHandleFrame(text) == .nudge else {
+                guard classifyAndHandleFrame(text) == .nudge else {
                     return .legacyEndpoint
                 }

As per coding guidelines, parsing-heavy work invoked from UI isolation needs an explicit off-main boundary; this avoids the unbounded decode entirely.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let text: String?
switch message {
case let .string(string): text = string
case let .data(data): text = String(data: data, encoding: .utf8)
@unknown default: text = nil
}
// A nudge-aware worker sends nothing but small text nudge
// frames on a directed stream, so the first frame that is
// anything else (a snapshot, presence chatter, binary) proves
// the endpoint is legacy: stop pumping it instead of paying a
// main-actor decode for team-sized traffic.
guard let text, classifyAndHandleFrame(text) == .nudge else {
return .legacyEndpoint
}
let text: String
switch message {
case let .string(string): text = string
case .data:
return .legacyEndpoint
`@unknown` default:
return .legacyEndpoint
}
// A nudge-aware worker sends nothing but small text nudge
// frames on a directed stream, so the first frame that is
// anything else (a snapshot, presence chatter, binary) proves
// the endpoint is legacy: stop pumping it instead of paying a
// main-actor decode for team-sized traffic.
guard classifyAndHandleFrame(text) == .nudge else {
return .legacyEndpoint
}
🤖 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 `@Sources/Cloud/PresenceNudgeSubscriber.swift` around lines 221 - 234, Update
the message switch in the nudge frame handling path to return .legacyEndpoint
immediately for .data messages, before constructing or UTF-8 decoding text.
Preserve string processing through classifyAndHandleFrame, while keeping unknown
message cases treated as legacy.

Source: Coding guidelines

…lose

Review round 5:

- Directed (device-scoped) sockets no longer draw from the shared
  64-subscriber presence pool. Every enabled Mac instance holds one, so
  a fleet of Macs or tagged dev builds could deterministically 429 the
  phones' presence streams. Admission is now a pure, tested decision
  (checkSubscriberAdmission): directed sockets get their own bounded
  pool of 256 and each pool only rejects its own kind.

- An EMPTY cleanly-closed stream counts as served only after living 60
  seconds. The close code alone let an accept-then-close loop
  (persistent drain, misbehaving proxy) pin every Mac at one WebSocket
  handshake per second forever; the healthy quiet close arrives at the
  service's 15-minute deadline, far above the threshold, so normal
  renewals still resubscribe promptly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

azooz2003-bit and others added 2 commits July 28, 2026 20:32
Review round 6:

- Directed subscribe admits unpinned devices by design (a Mac subscribes
  before its first heartbeat), which let one member park sockets on
  arbitrary fresh UUIDs until the 256-socket team pool 429'd legitimate
  owners. Admission now also enforces a per-user slice (32), so one
  member can never reach the team ceiling; the pure decision and its
  tests cover both pools and the slice.

- The Mac's 2 KiB nudge bound moved from post-receive classification to
  URLSessionWebSocketTask.maximumMessageSize, so a legacy worker's
  team snapshot fails the receive (EMSGSIZE) before it is buffered
  instead of after megabytes land in memory. That failure classifies as
  .legacyEndpoint, converging with the parsed-foreign-frame path on the
  15-minute reprobe. The in-classifier length check stays as a second
  layer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…scribe

# Conflicts:
#	Sources/Mobile/MobileHostIrohRuntime.swift
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

Comment-only. Review round 7 flagged that a superseded host answering a
replacement nudge re-registers (mutating the newest-wins slot) before it
detects the changed binding id. That ordering is the pre-existing
renewal path; the nudge deliberately reuses it unchanged, and the
displaced-instance disposition (stand down without re-taking the slot)
belongs to the nudge-emission hook that fires from the authoritative
broker mutation — deferred with it to the follow-up PR behind
#8883.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@azooz2003-bit
azooz2003-bit merged commit 8ba7781 into main Jul 29, 2026
6 checks passed
@azooz2003-bit
azooz2003-bit deleted the feat-mac-presence-subscribe branch July 29, 2026 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant