Skip to content

perf(mobile): share connectivity observation across environments - #25492

Open
saphid wants to merge 2 commits into
pingdotgg:mainfrom
saphid:perf/mobile-shared-connectivity-20261027
Open

saphid wants to merge 2 commits into
pingdotgg:mainfrom
saphid:perf/mobile-shared-connectivity-20261027

Conversation

@saphid

@saphid saphid commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Each mobile environment independently acquires connectivity and network-path observation. With several environments, the same native event fans out through duplicate registrations and repeats network-state reads on resume.

Change

Share live connectivity and account/path changes within the mobile runtime, with reference-counted acquisition, no replay and no idle grace. Keep status reads fresh and application resume timing local to each subscriber, so a newly opened connection retains its own probe/reconnect decision. Final release removes observation; later readers acquire it again.

Scope and approval

This is a focused reduction of duplicate work in the existing mobile connection adapter. It adds no setting, protocol or retry policy. Audit tracker: #189.

Verification

  • CI=true vp run check:changed f0dd86f16482595c6cfffe0ac64dc12bffb634f9: passed, including affected typecheck and all ten focused tests.
  • A separate GPT-6.1 Sol/high review session inspected the full contribution, found no actionable issues and independently passed all ten tests. Coverage includes late readers, short/long resumes, credential fan-out, delayed seeds, final release/reacquisition, old callbacks, native read failures, synchronous native registration events and slow consumers.
  • A count harness executes the exact baseline/candidate stream and layer bodies with real Effect service layers and mocked Expo/AppState boundaries. For ten supervisor-shaped readers plus one registry-shaped connectivity reader:
JavaScript operation Baseline Candidate
Network listener registrations 21 2
AppState listener registrations 21 11
Initial path-state reads 10 1
Network-state reads per active resume 11 1

The same harness also checks one and three supervisor-shaped readers. Every current reader receives the expected short-resume and path-change events, and final scope release leaves zero registrations. Fresh startup status reads are excluded from this count. These are JS API calls and registrations, not independent OS monitors or physical battery measurements.

A separate probe runs the actual EnvironmentSupervisor.make with a healthy mocked transport. A connection opened one second before resume stays open and receives a health probe on both baseline and candidate:

baseline: 1 connection, 0 releases, 1 probe
candidate: 1 connection, 0 releases, 1 probe

Native APIs and transport are mocked; device, full-registry and physical-energy behavior were not measured. No visible UI change is intended.

Current main's Android lifecycle reconciliation is preserved. Fresh suspension reads and per-subscriber initial reconciliation remain local, alongside application resume classification, outside the no-replay share. Native iOS/Android multi-environment path changes, offline/online transitions and short/long resume behavior still need integrated proof. Macroscope approved the repaired current head bac9318; its prior human-approval requirement is no longer current.

Implemented and independently reviewed in separate GPT-6.1 Sol/high sessions through Codex in T3 Code.

Current-main verification (2026-10-29)

Integrated upstream dac8b11. Fresh independent verification passed 10 adapter tests and 15 supervisor cases (34 unrelated cases filtered). The final scoped CI=true vp run check:changed dac8b11ddc25bd5da62a367fdb6c3de777ae2986 passed after the merge commit. Independent Codex/GPT-6.1 Sol high review found no actionable source findings in this frozen contribution. Earlier runtime captures above are historical; native proof was not repeated for this integration. Published CI and maintainer approval must be assessed on the new head.

Implementation and independent review used separate GPT-6.1 Sol/high sessions through Codex in T3 Code.

Latest readiness check (2026-10-29)

All eight required checks passed on bac9318. GitHub reports a clean, mergeable branch. Macroscope approved that exact head, with its correctness/approvability checks passing; there are no inline review comments. This is bot approval, not a maintainer merge decision.

A fresh integrated native verification attempt stopped before app launch: the T3 Device agent failed ownership verification because daemon.lock is not a directory. Android discovery also reports its emulator unavailable. The supported device workflow requires stopping when device tools are unavailable; no replacement automation or shared-daemon repair was attempted. iOS/Android multi-environment path changes, offline/online and short/long resume behavior remain unverified on a native client. No native pass or physical-energy result is claimed.

The source, deterministic fixtures and current-head CI are green; native compatibility evidence remains blocked by the device tool host.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 26, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at bac9318

Macroscope's review found this PR approvable — The change reduces duplicate mobile network observation while preserving fresh status reads and connection-specific resume timing. Its scope and lifecycle safeguards show no obvious material risk requiring human judgment.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between b618499 and bac9318.

Files selected for processing: 2
  • apps/mobile/src/connection/platform.test.ts
  • apps/mobile/src/connection/platform.ts

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

Walkthrough

The mobile platform now shares connectivity and general wakeup observations across readers. It acquires application-active wakeups per connection and retains per-connection resume-age classification. New tests cover acquisition, event delivery, ordering, cleanup, read failures, and resume behavior.

Changes: Mobile connection observations

Layer / File(s) Summary
Shared connectivity observations
apps/mobile/src/connection/platform.ts, apps/mobile/src/connection/platform.test.ts
The platform provides connectivity changes through a shared stream. Tests cover shared acquisition, late readers, event ordering, cleanup and reacquisition, read failures, and reader delivery.
Shared wakeups and per-connection resume classification
apps/mobile/src/connection/platform.ts, apps/mobile/src/connection/platform.test.ts
The platform shares general wakeup changes and acquires application-active wakeups per connection. Tests cover short and long resumes, readers joining while the app is backgrounded or inactive, and ordered delivery to slow readers.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Possibly related PRs

Suggested reviewers: macroscopeapp

Merge Risk: Minimal · up to bac93

No actionable merge-blocking issue is established; the change is ready for normal merge checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title "perf(mobile): share connectivity observation across environments" matches the main change: the connection adapter in platform.ts now shares live connectivity and network-path observation ac…
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.
Description check Passed The description includes all required sections. It clearly states the problem, change, scope, audit issue, verification results, limitations, and agent usage. It also records blocked native verificati…
Full details: Title check

Explanation

The title "perf(mobile): share connectivity observation across environments" matches the main change: the connection adapter in platform.ts now shares live connectivity and network-path observation across mobile environments with reference-counted acquisition. It follows the conventional commit form and avoids vague wording. The title does not mention the test file or the wakeup changes, which is acceptable because the title only needs the primary change.

Full details: Description check

Explanation

The description includes all required sections. It clearly states the problem, change, scope, audit issue, verification results, limitations, and agent usage. It also records blocked native verification and separates historical evidence from current-head evidence.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

GigaZoro pushed a commit to GigaZoro/CodeShip that referenced this pull request Oct 30, 2026
…tream pingdotgg#25492)

Cherry-picked from pingdotgg#25492

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant