[brennanb2025]

	Files	Added	Deleted	Net
Test	6	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​286	$\color{#cf222e}{\Huge{\mathbf{−}}}$​35	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​251
Prod	23	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​353	$\color{#cf222e}{\Huge{\mathbf{−}}}$​1546	$\color{#cf222e}{\Huge{\mathbf{−}}}$​1193

What this fixes

The problem. When the phone is connected to a desktop, it opens two background feeds from that desktop: desktop notifications and the account list. Each feed is a registration on the desktop, and the phone has to tell the desktop to stop it when the phone no longer needs it. For these two feeds, the phone often never sent that stop, or sent it with an out-of-date id, so the desktop kept a dead registration and kept pushing events to a phone that was no longer listening, until that phone connection closed.

The cases on main:

• Account list, direct connection: the stop was never sent. Every visit to the phone's Accounts screen left one more account feed running on the desktop. Reproduced live: after three visits, the desktop held 4 account listeners for one phone (1 → 2 → 3 → 4).
• Notifications and account list, closed quickly: if the phone closed the feed before the desktop's first reply arrived, no stop was ever sent.
• Notifications, after the phone reconnects (for example switching between direct and relay): a feed closed in the short window before the desktop answered on the new connection was stopped with the old connection's id, which no longer names anything, so the new feed kept running.
• Notifications, relay connection: the stop was sent twice.

Scope. Phone app only, direct and relay connections. No desktop change and no wire change. Not affected: terminal, chat, tab-list and browser-preview feeds; SSH hosts; and everything the phone shows.

Where these came from. None of these is a recent regression. Each gap has been there since the code that opened or closed the feed first landed:

• Account list never stopped on a direct connection: feat(mobile): account switcher and rate-limit usage on mobile #1467 (the phone's account switcher). It added the desktop's accounts.unsubscribe, but the phone never sent it.
• Notifications closed before the first reply: feat(mobile): Expo companion app [beta] #1245 (the first phone app). The notification code had a branch meant to send the stop when a late ready arrived, but the phone client stopped delivering to a closed feed's listener, so that branch never ran. refactor(mobile): drop the unreachable dispose-before-ready notifications arm #21293 later removed it as dead code, which changed nothing.
• Notifications stopped with an old id after a reconnect: feat(mobile): Expo companion app [beta] #1245. The notification code kept its own copy of the id, while the phone client re-opened the feed on each reconnect.
• The duplicate notification stop on relay, and relay account feeds closed before the first reply never stopped: Add Orca Relay desktop and mobile transport #8536 (the relay connection). It added the stop derived by rewriting x.subscribe into x.unsubscribe, which is sent only after ready and fires alongside the notification code's own stop. fix(mobile): stop host streams after relay subscription cancellation #18926 moved that code without changing this.

Before and after for users

• Before: nothing looked wrong on screen. The cost was hidden: the desktop kept one extra live registration per leaked feed and kept sending its events over the phone connection (wasted desktop work, phone bandwidth and battery), and this grew with every visit to the Accounts screen until the phone disconnected.
• After: the phone looks and behaves the same (notifications arrive, dismissals sync, the account list loads), and every notification or account feed is stopped exactly once, with the id the desktop issued on the current connection. Leaving the Accounts screen takes the desktop back to one listener.

How it works

Background: for a few feeds, the desktop only says which registration it created in its first reply, a ready message carrying a subscriptionId. To stop the feed the phone must send that id back (notifications.unsubscribe {subscriptionId}, accounts.unsubscribe {subscriptionId}). The only part of the phone that sees every ready, including the new one after a reconnect, and that knows whether a feed was closed before its ready arrived, is the connection layer (the direct stream registry and the relay stream manager).

The mechanism:

1. One table of these feeds. READY_STREAM_RELEASE_METHODS in mobile/src/transport/rpc-client-server-subscription.ts maps each such feed to its stop method: browser preview, workspace events, and now notifications and accounts.
2. Both connection types send the stop from that table. The direct registry (rpc-client-stream-registry.ts) and the relay manager (mobile-relay-rpc-streams.ts) check the table (isReadyIdStream) and use their existing "stop by the desktop's id" path. Closed after ready: stop with the current id. Closed before ready: keep a cancelled placeholder, and send the stop when the ready arrives. That path already handled browser preview and workspace events; this PR routes the two missing feeds through it.
3. The notification code no longer stops its own feed. mobile-notifications.ts stopped keeping its own copy of the id (which went stale on reconnect and did not exist before ready) and just calls the disposer the connection layer returns. Its now-unused stop request module is deleted.
4. The relay's name guess is gone. The relay used to derive a stop for any other feed by rewriting x.subscribe into x.unsubscribe. On main only notifications and accounts reached that guess. It was the second notification stop, and the only thing stopping relay account feeds. Both feeds are now in the table, so the guess is removed.
5. A test keeps the table complete. The phone's subscription inventory (rpc-subscription-inventory.ts) now records how each feed is stopped (ready-id or params), and its boundary test fails unless the ready-id feeds are exactly the table's. A new feed of this kind can't be added without a stop.

Before and after in the codebase

	Before (main)	After
Who stops a feed whose id arrives in ready	Three places, each different: direct registry (hardcoded browser preview + workspace events), relay manager (same two hardcoded, plus a string-rewrite guess for everything else), and notification feature code (its own id copy and its own stop request)	The connection layer only, from one table both connection types read
Account list stop	None on direct; relay only by the name guess, and only after ready	From the table, on both, before or after ready
Notification stop	Feature code after ready (stale after reconnect, missing before ready), plus the relay guess (duplicate)	Once, from the table, with the current id
desktop-notification-stream-operations.ts	Stop request definition used by the feature code	Deleted
Adding a new feed of this kind	Had to be remembered in both connection files separately, or it leaked silently	One table entry; the inventory test fails if it's missing

The plan this belongs to

#22928 moves every phone feed to one rule: a feed is stopped exactly once, by the part of the phone that knows which desktop registration it is, so a stop can never miss its feed or hit a newer one. The earlier steps (#22948, #22939, #22945, #23006, #22943, #23045) did this for terminal, chat and tab-list feeds and for feeds reopened after a reconnect. This PR is the last step: it applies the same rule to the notification and account-list feeds.

Why this approach

Only the connection layer sees every ready, so it is the only place that can hold a current id. Moving the stop there removes the stale-id and close-before-ready bugs by construction; patching the feature code's copy would have kept the id in two places. The connection layer already stopped browser preview and workspace events this way, so this extends that path rather than adding another.

Alternatives considered:

• Refresh the notification code's id on reconnect: the id still lives in two places, and the notification code would still need its own close-before-ready tracking.
• Keep the relay's name guess: one class of feeds would be stopped by an explicit table and another by a string rewrite no test pins.
• Keep separate per-method checks in the direct and relay code: a feed added to one but not the other leaks silently, which is exactly how the direct account leak happened.

Differences from comparable designs:

• Intended: stops are looked up in a per-method table rather than sent through one generic "release this registration" call. Every shipped desktop already accepts these per-feed stops, while a generic release would be a new wire method that old desktops reject and that would need capability negotiation. The inventory test catches a missing entry.
• Intended: a feed closed before its ready is kept as a cancelled placeholder and stopped when the ready arrives, instead of being stopped by the id of the request that opened it (as terminal feeds are since fix(runtime): let the phone end a terminal stream by the request that opened it #23006). The desktop registers these feeds synchronously just before sending ready, so the placeholder lives for one round trip, and it is dropped on ready, error, end or connection close.
• Intended: the desktop's own client for remote Orca servers keeps a similar method-to-stop table (getCleanupRequest in src/shared/remote-runtime-shared-control-protocol.ts). The phone keeps its own, because that module imports desktop-only connection code and only three rows overlap.

Feeds deliberately left out of the table: files.watch and browser.clientHost.attach (the phone may not call them); terminal, tab-list, native chat and agent session feeds (stopped from their subscribe params and request id, params in the inventory); browser network tunnels and the terminal multiplex (their ready has no id).

Linked Issue

Part of #22928

Visual Proof

No screen changes. Live phone QA ran on an iOS simulator paired to an isolated desktop built from main, first with main's phone app, then with this branch's phone app, watching the desktop's feed registrations.

Check	main's phone app	this PR
Account listeners on the desktop after 3 visits to the Accounts screen	1 → 2 → 3 → 4 (leak)	back to 1 after each visit
Phone receives a desktop notification	yes	yes
Notifications after a hard desktop restart and phone reconnect	yes	yes
Leftover notification listeners on the desktop	none	none
Terminal open / send / leave / return, 3 cycles	not run	every send executed

Accounts screen on this build:

Reconnected after a hard desktop restart:

Terminal sends after leaving and returning:

Testing

• I manually tested these changes locally
• Automated tests added/updated, or explained why not below

New and updated tests, using the real direct registry and the real relay manager (each fails without the fix):

• Notifications, both connections: close before ready, then ready arrives → exactly one notifications.unsubscribe with that id (main: none).
• Notifications, direct: reopen after a socket close, close before the new ready → the only stop names the new id (main: the old id).
• Notifications, relay: close after ready → exactly one stop (main: two).
• Accounts, direct: close after ready → accounts.unsubscribe (main: none); close before ready → stopped once ready arrives, on both connections.
• Every feed in the table, both connections: one stop after ready, one when the close came first. Reads the table, so future entries are covered.
• Inventory boundary test fails if a ready-id feed is missing from the table (checked by removing accounts.subscribe).
• Unchanged behaviour: dismissals and the catch-up read still run, and a reopened feed re-runs the catch-up on its new ready.

On the current head (87e46dd): the transport and notification test files, rpc-client-ready-stream-release.test.ts, the inventory boundary test, and every file under mobile/src/test-support/rpc-recording and bridged-parity pass with --config vitest.config.ts; host and mobile typecheck pass; oxlint and the changed-code quality gate are clean; CI is green.

Recorded RPC goldens (rpc:diff against main): of 794, 787 are unchanged, 6 desktop-notification goldens change, and 1 is deleted. The 6 change because the stop is now sent by the connection layer instead of the notification code: same wire frame, minus the feature code's request entries; after a reconnect the old connection's sub-1 is also stopped, as workspace-event feeds already are; and where the desktop's reply ends the feed with an error, no stop is sent. The deleted matrix-notifications.desktop-stream-notifications.unsubscribe-1 varied the reply to the notification code's own stop request, which no longer exists, so its scripted replies are dropped from pilot-scenarios.json too. The corpus is now 793, so the bridged-parity tally and the comments quoting the corpus size drop by one. Main's recorded-request check parses every recorded stop with the desktop's own params parser and passes; the four stops the table can build were also checked against that parser.

AI Disclosure

Review

• Security: no new methods or parameters. notifications.unsubscribe and accounts.unsubscribe are already on the mobile allowlist, and the phone only sends ids the desktop issued to it.
• Cross-platform: phone connection logic only.
• Remote SSH: unaffected; this is the phone-to-desktop connection, not how the desktop reaches its hosts.
• Mobile: direct and relay both change, each covered by tests and by live QA on direct.
• Mixed phone/desktop versions: no wire change. Every desktop already answers these feeds with {type:'ready', subscriptionId} and accepts the matching stop (src/main/runtime/rpc/methods/notifications.ts, accounts.ts). An older desktop simply starts receiving the stop it was missing.
• Performance: the desktop stops pushing account and notification events to phone connections that stopped listening, and the relay sends one fewer stop.

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

This branch has merged main several times (latest f37a849). It composes with the earlier steps in the same files: the stream record is removed before its listener hears end or an error (#22939); the relay's same-slot sibling skip still runs before every stop and never skips a ready-id stop, because those ids are unique; terminal stops carry the opening request id (#23006); a direct tab-list stop waits for the first snapshot (#22943); and reopening after a socket close forgets the old desktop id (#22945). Merge conflicts were only in recorded goldens and in comments that quote the corpus size, resolved by re-recording from the merged tree.

Checklist

• This PR is small and focused
• I explained what changed and why (the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[pullfrog]

✅ No new issues found.

Reviewed changes

• Transport owns ready-id stream release — READY_STREAM_RELEASE_METHODS in rpc-client-server-subscription.ts is now the single table behind buildReadyStreamUnsubscribe and the new isReadyIdStream, adding notifications.subscribe and accounts.subscribe to the two methods that already worked this way.
• Direct registry replays without the dead socket's id — markForReplay clears stream.subscriptionId, so a reopened feed discards the previous connection's registration and releases only the id from its new ready.
• Relay drops the name-guess fallback — the .subscribe→.unsubscribe string rewrite is gone; a feed is released via the table or a cancelled placeholder, which removes the duplicate notifications.unsubscribe.
• Notification module sheds its local id and stop — mobile-notifications.ts keeps only its listener/catch-up work; desktop-notification-stream-operations.ts is deleted.
• Golden repertoire re-recorded — 787→786 goldens, six desktop-notification body moves, matrix-notifications.desktop-stream-notifications.unsubscribe-1 deleted, and the bridged-parity pins/tally renumbered to match.

The behavioural claims hold up on a read of the transports: every subscribe method the phone actually opens is either in the table (browser.screencast, runtime.clientEvents.subscribe, notifications, accounts), handled by its own param-derived path (terminal.subscribe, session.tabs.subscribe, nativeChat.subscribe), or id-less (agentSession.subscribe), so removing the relay name guess leaks nothing. The sub-1→sub-2 replay test genuinely fails against the unpatched markForReplay, so it is coverage rather than a tautology.

ℹ️ Nitpicks

• mobile/src/test-support/rpc-recording/README.md:410-420 still explains the old blindness with "Neither unsubscribe builder in rpc-client-stream-registry.ts knows notifications.subscribe". That paragraph is history, but the mechanism clause is now false — this PR put notifications.subscribe in that builder — and the sibling paragraph at :656 was updated while this one was not.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Incremental delta since the prior pullfrog review (e7e3a57): one commit, ecc04dc.

• Subscription inventory gains a release classification — every RPC_SUBSCRIPTION_SITES entry now declares how the phone ends its host stream, through the new RpcSubscriptionRelease type: ready-id, params, or connection-close.
• Boundary test binds the ready-id entries to the transport release table — a new test asserts set equality between the inventory's release: 'ready-id' methods and READY_STREAM_RELEASE_METHODS keys, so a feed marked ready-id without a table entry (or a table entry with no ready-id site) fails. The classifications hold up against both transports: ready-id is exactly the four table methods, the three params entries have matching builders (terminal, nativeChat, session.tabs), and agentSession.subscribe is genuinely connection-close (neither builder handles it).
• Doc headers updated — the boundary test and inventory headers now describe the fifth failure mode.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Refactors notification stream lifecycle and transport cleanup.

The PR appears safe to merge; no new actionable issue was identified in changes since the previous review.

Summary

The PR moves notification and account-stream cleanup into the mobile transport, which uses the desktop-issued ready ID to release registrations on direct and relay connections.

• Adds shared release-method mapping and boundary tests for ready-ID streams.
• Removes notification-owned unsubscribe requests and updates recordings to reflect transport-owned cleanup.

Diagram

sequenceDiagram
  participant Phone as Mobile consumer
  participant Transport as Direct or relay transport
  participant Desktop
  Phone->>Transport: subscribe to notifications or accounts
  Transport->>Desktop: subscribe
  Desktop-->>Transport: ready(subscriptionId)
  Phone->>Transport: dispose
  Transport->>Desktop: unsubscribe(subscriptionId)

Loading

_{Reviews (6) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..."}

[pullfrog]

✅ No new issues found.

Reviewed changes

Incremental delta since the prior pullfrog review (f503170): the branch merged origin/main twice and re-recorded the golden corpus against the new baseline.

• Merged origin/main — brings in main's transport refactor (finish, buildRequestStreamUnsubscribe, the relay sendOrder/slot-eviction path, session-tabs snapshot hold) alongside unrelated app-update, keyboard, and session work. The resolution keeps this PR's release behavior intact: isReadyIdStream still routes every ready-id feed through disposeServerSubscription (direct) and the cancelled-placeholder release (relay), and markForReplay still clears stream.subscriptionId.
• Re-recorded the corpus after the merge — 789 goldens repinned to the merge baseline; the six desktop-notification recordings change only in baseline/lockfileSha256/recorderSha256, leaving their bodies byte-identical to the pre-merge branch, and the deleted notifications.unsubscribe matrix stays deleted.
• Renumbered the bridged-parity pins — corpus 789, identical 397, result-absent-settlement 342, result-absent-stream-release 6, params-undefined 33, write-ordinal 8; the recorder README, page-closure headers, and boundary-test comments follow.

The author's own delta is test-support metadata with no product-code behavior change. I checked the counts against the tree (789 golden files, 398 manifest scenarios; 397 + 342 + 3 + 6 + 33 + 8 = 789) and ran the PR's transport and notification suites against the merged code — rpc-client-ready-stream-release.test.ts, mobile-relay-rpc-streams.test.ts, mobile-notifications.test.ts, and rpc-subscription-boundary.test.ts all pass (39 tests).

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Incremental delta since the prior pullfrog review (a09d1e0, review 5359972404): the branch merged origin/main at 8b01590847 and re-recorded the desktop-notification goldens on the header-free format.

• Merged origin/main (8b01590847) — brings #23732, which dropped the golden header and pin, plus unrelated main churn. The resolution keeps this PR's release paths intact: isReadyIdStream still routes every ready-id feed through disposeServerSubscription (direct) and the cancelled-placeholder release (relay), and markForReplay still clears stream.subscriptionId.
• Re-recorded the six desktop-notification goldens — the recordings now show only the transport's notifications.unsubscribe wire frame, with the former feature-code sender calls (args + settlement) removed, and the obsolete matrix-notifications.desktop-stream-notifications.unsubscribe-1 golden pruned. The replayed golden records the old connection's sub-1 stop (frame-4) alongside the new sub-2 stop (frame-6).
• Doc and count touch-ups — the recorder README, page-closure headers, and parity test comments follow the header-free corpus (789 goldens, 398 scenarios).

I replayed the corpus on this head — the full mobile rpc-recording suite (1752 passed, 3 skipped, including rpc-recording-through-bridge at 795 passed) and the bridged-parity suite — and re-ran the PR's transport/notification suites (39 passed). The re-recorded bodies match the already-reviewed fix; nothing the merge brought altered this PR's behavior.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 7047976f-e9b2-46d4-a013-2f3738c40d30

📥 Commits

Reviewing files that changed from the base of the PR and between 8b01590 and 05e9977.

📒 Files selected for processing (29)

• mobile/rpc-foundation/goldens/matrix-notifications.desktop-stream-notifications.getmissedsince-1.json
• mobile/rpc-foundation/goldens/matrix-notifications.desktop-stream-notifications.subscribe-1-1.json
• mobile/rpc-foundation/goldens/matrix-notifications.desktop-stream-notifications.subscribe-1-2.json
• mobile/rpc-foundation/goldens/matrix-notifications.desktop-stream-notifications.unsubscribe-1.json
• mobile/rpc-foundation/goldens/notifications-desktop-stream-closed.json
• mobile/rpc-foundation/goldens/notifications-desktop-stream-replayed.json
• mobile/rpc-foundation/goldens/notifications-desktop-stream.json
• mobile/rpc-foundation/pilot-scenarios.json
• mobile/src/notifications/desktop-notification-stream-operations.ts
• mobile/src/notifications/mobile-notifications.test.ts
• mobile/src/notifications/mobile-notifications.ts
• mobile/src/test-support/bridged-parity/c1-page-closure.ts
• mobile/src/test-support/bridged-parity/c2-page-closure.ts
• mobile/src/test-support/bridged-parity/c3-page-closure.ts
• mobile/src/test-support/bridged-parity/c5-page-closure.ts
• mobile/src/test-support/bridged-parity/divergence-classes.test.ts
• mobile/src/test-support/bridged-parity/divergence-classes.ts
• mobile/src/test-support/rpc-recording/README.md
• mobile/src/test-support/rpc-recording/adapters/desktop-notification-stream-mount-adapters.ts
• mobile/src/test-support/rpc-recording/rpc-recording-through-bridge.test.ts
• mobile/src/test-support/rpc-recording/run-recording.ts
• mobile/src/transport/mobile-relay-rpc-streams.test.ts
• mobile/src/transport/mobile-relay-rpc-streams.ts
• mobile/src/transport/rpc-client-ready-stream-release.test.ts
• mobile/src/transport/rpc-client-server-subscription.ts
• mobile/src/transport/rpc-client-stream-registry.ts
• mobile/src/transport/rpc-subscription-boundary.test.ts
• mobile/src/transport/rpc-subscription-inventory.ts
• mobile/src/transport/unvalidated-rpc-request-port-inventory.ts

💤 Files with no reviewable changes (3)

• mobile/rpc-foundation/goldens/matrix-notifications.desktop-stream-notifications.unsubscribe-1.json
• mobile/src/notifications/desktop-notification-stream-operations.ts
• mobile/rpc-foundation/pilot-scenarios.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

The RPC transport now maps ready-ID stream methods to their unsubscribe methods and handles release when a stream is disposed before or after readiness. Notification code no longer sends its own unsubscribe RPC. Tests and recording fixtures now cover direct and relay transports, stream teardown, and updated notification traces. The bridged-parity corpus counts also reflect the revised recordings.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 05e99

The change centralizes subscription cleanup while retaining notification handling and catch-up behavior. No actionable merge-blocking issue is identified; merge after normal checks. Live phone validation remains unperformed.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 05e99

The change improves background-feed cleanup without an identified new attack path. Confidence remains limited because cleanup after a lost connection has not been fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The new automatic release behavior is bounded to four fixed stream-to-release mappings. The mapping does not accept a caller-selected release method; it extends cleanup behavior rather than granting a new general-purpose RPC capability.

Trust Boundaries and Controls

• observed — Release identity comes from server response metadata correlated to the locally tracked opener request. Transport ownership removes the notification feature's independently retained ticket, and direct replay explicitly forgets the old connection's ID. Desktop enforcement of subscription ownership was not verified.

Resilience and Maintainability Implications

• observed — Relay clear discards active streams and pending cleanup entries. Both release paths remove local bookkeeping without awaiting server confirmation, so cleanup after failed dispatch or connection replacement depends on desktop lifecycle behavior that the reviewed mobile source does not establish.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 19 files. (7 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly and concisely describes the main change: moving notification and account stream release into the mobile transport layer.
Description check	✅ Passed	The description is complete and well structured. It includes the problem, scope, implementation details, rationale, linked issue, visual proof, testing evidence, review considerations, and completed c…

Full details: Docstring Coverage

Explanation

Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 19 files. (7 skipped: 7 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[brennanb2025]

Reviewed and tested. Status: ready for review. This is the last step of the terminal subscription identity series (#22928).

What it fixes, as a user would see it. The phone never stopped some background feeds it opened on the desktop. Every visit to the phone's Accounts screen left one more account-list feed running on the desktop. A notifications feed closed before its first reply could also stay open, and one could be left stopped with an out-of-date id after the phone switched between a direct and a relay connection. With this change, the phone's connection layer ends every feed the desktop gave an id to, including one closed before the desktop's first reply.

Review. A fresh review ran after merging current main, because the same phone connection files changed several times in between. It covered readiness at the start and the end, an up-front architecture challenge, a precedent check, and two review rounds. The final round was clean or nits-only. Every stream the phone opens now has an explicit way to end. Only notifications and accounts used the removed guess (unsubscribe method name derived from the subscribe name), and both are in the release table. Mixed versions are safe: every desktop that serves these feeds already accepts notifications.unsubscribe and accounts.unsubscribe with the id it issued. There is no desktop change. The recorded goldens differ from main by exactly this PR's six notification recordings plus one deleted recording.

Testing. Live QA used an iOS simulator paired to an isolated desktop host built from main, running main's phone app first and then this branch's phone app. The desktop's feed registrations and listener counts were watched throughout.

Check	main's phone app	this PR
Account feeds still on the desktop after 3 visits to the Accounts screen	leaked: listeners 1 → 2 → 3 → 4	released each time: back to 1
Phone receives a desktop notification	yes	yes
Notifications after a hard desktop restart, and after the phone reconnects	yes	yes
Extra notification listeners left on the desktop	none	none
Terminal open / send / leave / return, 3 cycles	not run	every send executed

Accounts screen on this build:

Reconnected after a hard desktop restart:

Terminal sends after leaving and returning:

Typecheck (host and mobile) passes, and all CI checks pass on 87e46dd.

[brennanb2025]

Second round of live phone QA on 87e46dd: regression checks for the other feeds this PR's stop path touches. Each was run first with main's phone app, then with this PR's phone app, against the same isolated desktop built from main. Every check passed, and nothing behaved unexpectedly.

Check	main's phone app	this PR
Browser preview: open, leave and reopen, 3 times, plus leaving by switching tabs	renders every time; desktop drops the preview after each leave; desktop "mobile is driving" overlay clears	same; one stop per leave, each with that preview's own id
Live worktree name and worktree list: rename and create on the desktop; leave and return 3 times	phone updates live; desktop feeds go 0 → 1 → 0 each cycle	same
Notification, dismiss on desktop, graceful desktop restart, then notify again	delivered once, dismissal reaches the phone, one listener per phone	same
Accounts screen, 3 visits	desktop registrations 2 → 3 → 4, still 4 after leaving (leak)	2 on every visit, back to 1 after leaving
Home screen: disconnect, connect, remove and re-pair a desktop	counts return to baseline each time	same; each stop names the current connection's id
Relay connection	blocked	blocked

Limits of this round:

• Relay: not tested live. A development build cannot create a relay pairing ("Orca Relay is not available on this desktop"). The relay changes are covered by unit tests only.
• Notifications: the simulator receives no Apple push notifications, so delivery and dismissal were confirmed on the phone's connection to the desktop, not as system banners.
• Accounts: the test desktop has no logged-in accounts, so the only switch available was to "System default". Selecting it stuck on both builds.

Browser preview on the phone, second open:

Desktop after the phone left the preview, with no phone-control overlay:

The phone header after the worktree was renamed on the desktop: