Skip to content

feat: real background push notifications for the mobile app (#8129) - #18554

Merged
Jinwoo-H merged 24 commits into
mainfrom
feat/mobile-push
Sep 7, 2026
Merged

Jinwoo-H merged 24 commits into
mainfrom
feat/mobile-push

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 76 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​8080 $\color{#cf222e}{\Huge{\mathbf{−}}}$​342 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​7738
Prod 134 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​8903 $\color{#cf222e}{\Huge{\mathbf{−}}}$​350 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​8553

ELI5

Today your phone only gets an Orca notification while the app is open and connected to your desktop. This adds real push notifications, so an agent finishing or asking for input reaches your phone even when Orca is closed. It is off by default; one switch on the phone turns it on and says exactly what leaves your machine.

What Changed

Real background push for the Orca mobile app (#8129). A small open-source push gateway in cloud/apps/push holds the APNs key and FCM credentials and sends to phones. The desktop registers each paired phone's native push token with the gateway over the existing encrypted socket and asks the gateway to push on every mobile notification it already fans out. The phone dedupes by notificationId#notificationSeq, so an open app shows the socket copy and drops the push, and a closed app shows the push. Off by default; one switch on the phone with a plain disclaimer of what leaves the machine.

Design and every wire shape are in docs/reference/mobile-push-contract.md. Decisions recorded there: no ack gate, no generic-alert mode, no staging gateway, one auth path for signed-in and accountless hosts (box challenge + HMAC proof on the pairing key, since X25519 cannot sign).

  • cloud/packages/push-contract, cloud/apps/push: zod contract, hono gateway, SQLite-in-tests / Postgres-in-prod store, challenge/session auth, device registry, quota, 3 s coalescer, APNs HTTP/2 client with JWT rotation, FCM V1 client via the runtime service account, aggregate-only logging. 70 tests including a cross-lane test that runs the real desktop answerer against a real gateway-issued challenge.
  • Desktop: notifications.remote-push.v1 capability, notifications.registerPush / unregisterPush RPCs on the mobile allowlist, push client with session cache and 401 re-auth, durable unregister outbox, dispatcher hooked after the socket fan-out (fire-and-forget, one retry, drops dead registrations), agentState on the mobile frame, and the [Bug]: Non turn-end notification spam #4375 wording fix so a working agent is never announced as finished. The relay host-proof code is refactored onto a shared envelope module with no behaviour change (relay tests unchanged and green).
  • Mobile: google-services.json and the expo-notifications plugin, native token fetch with sandbox/production detection, per-host registration gated on the capability, persisted unregister retry, foreground suppression inside handleNotification against the existing seen set, fingerprint tap routing, tray dismissal parity, settings section with the disclaimer and needs-input / finished sub-switches.
  • Infra: push-gateway.tf behind push_gateway_enabled (prod only), cloud-push-deploy.yml (gated, no-traffic deploy, /ready probe, validate-only FCM probe that must return INVALID_ARGUMENT, then traffic shift), contract tests, cloud/docs/push-gateway.md with import commands for the resources created by hand and the APNs key rotation procedure.
  • Docs: user docs for the switch and what it sends; headless-serve limitation noted.

Why

Apple and Google will only deliver a push through their own services, and those need credentials that cannot ship inside a desktop app. So a small Orca-hosted gateway holds them. The desktop keeps owning the phone relationship: it registers the phone's token and asks for the push, so accountless and signed-in users get the same path and nothing new is required from Orca Cloud accounts. The phone dedupes against the socket copy so no ack round-trip or generic-alert mode is needed. Rationale for each rejected alternative is in docs/reference/mobile-push-contract.md.

Linked Issue

Fixes #8129

Visual Proof

N/A for now: the only UI change is the new "Background notifications" section on the phone's Notifications screen, and no physical-device build has been produced yet. Screenshots will be attached from the first TestFlight build.

Testing

Root: pnpm tc:node, check:code-quality:changed, check:max-lines-ratchet, the seven existing suites the desktop lane touched (41 tests) and the new push suites (66 tests). Cloud: pnpm -r typecheck, push-contract 16 tests, push 70 tests, full cloud script list 508 + 148 pretest, terraform fmt and validate offline, Docker image built and smoke-booted (/health, /ready, /v1/host/challenge all 200 on SQLite with no Apple credentials). Mobile: typecheck, lint, format:check, full suite 4110 tests.

APNs key (sandbox + production, team scoped) in Secret Manager; Firebase attached to onorca-cloud with the Android app registered; runtime SA orca-cloud-push@onorca-cloud with FCM admin and secret accessor. APNs probe returned BadDeviceToken on both hosts and a validate-only FCM send returned INVALID_ARGUMENT, which proves both credentials.

  • I manually tested these changes locally (gateway booted in Docker; APNs and FCM credentials probed end to end; production Terraform applied from this branch)
  • Automated tests added/updated, or explained why not below

AI Disclosure

Internal contributor.

Review

  • cloud/infra/terraform/relay-github-actions.tf: cloud-push-deploy.yml is added to the production deploy identity's workflow allowlist. It runs as the shared relay deploy account because the Cloud SQL rollout lease grant is foundation-owned. Its extra authority is three bindings scoped to the push service.
  • DNS for push.onorca.dev was created by hand in Cloudflare on 2026-09-04, matching how relay.onorca.dev is managed; the production Terraform in this PR has been applied (see the infra comment below).
  • Nothing here has run on a physical device. The entitlement and Firebase config are config-only until the next iOS and Android builds.

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

Security: push is not end to end encrypted (the OS draws the banner from the payload); the disclaimer says so and E2EE via an iOS Notification Service Extension is a listed follow-up. Backwards compatibility: all wire changes are additive. Old phone + new desktop and new phone + old desktop behave exactly as today; the new phone shows "Update your desktop app" instead of the switch. Cross-platform: desktop changes are main-process only, no path or shortcut impact.

Not done: physical-device delivery (iPhone dev build on sandbox APNs, TestFlight on production, sideloaded APK), the first deploy via cloud-push-deploy.yml after merge, and the follow-ups listed under "Non-goals" in the contract doc (notification service extension for E2EE banner text, Live Activities).

Checklist

  • This PR is small and focused (it is not; it is one feature across four lanes, split into one commit per lane for review)
  • I explained what changed and why (including ELI5)
  • 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)

@Jinwoo-H

Jinwoo-H commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Two of the CI failures on the first run are pre-existing and not from this branch:

  • Cloud Verify / Secret scan: gitleaks runs with --all, and the one finding is a Sec-WebSocket-Key header in a test on the open branch fix/relay-upgrade-malformed-uri (fix(relay): reject malformed percent-escapes on upgrade instead of throwing #18547). That commit is not on this branch; the same scan is failing on every recent cloud PR.
  • PR Checks / static analysis: verify:localization-catalog reports twelve extra keys in the renderer i18n catalog. This branch touches no catalog file, and the check fails identically on a clean origin/main worktree.

The third, Cloud Verify build and test, was ours: the gateway suite imported the desktop's challenge-answerer across workspaces, which the cloud-only install cannot resolve. Replaced in 8a34257ed3 with a checked-in challenge vector that both suites replay.

@Jinwoo-H

Jinwoo-H commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Production infrastructure is provisioned (2026-09-04)

Applied from this branch with a saved, targeted plan (only push-gateway.tf resources plus the WIF provider allowlist); nothing else in the relay root was touched, and the standing cell-template drift is untouched.

Imported (nine pre-existing resources, per cloud/docs/push-gateway.md): runtime SA, its two project roles, the three Apple secrets, and their accessor bindings.

Created: orca_push database and user on the shared Cloud SQL instance, orca-cloud-push-database-url secret + version + accessor, roles/cloudsql.client on the runtime SA, Cloud Run service orca-cloud-push (bootstrap hello image, min 1 / max 4), domain mapping push.onorca.dev, and the three deploy-identity bindings. WIF provider now also trusts cloud-push-deploy.yml.

One correction (commit 8c133e2): the org's domain-restricted-sharing policy rejects an allUsers run.invoker member, so the service uses invoker_iam_disabled = true like the relay director. The run.app URL now answers /health 200.

DNS: push.onorca.dev CNAME ghs.googlehosted.com (DNS only) created in Cloudflare by hand, matching how relay.onorca.dev is managed. Google's certificate is provisioning; the hostname will start answering once CertificateProvisioned flips to True.

Post-apply targeted plan: clean except one cosmetic in-place diff on the Cloud Run service (provider reports scaling { min_instance_count = 0 } at the service level that the config does not declare). Harmless; will look at it separately.

Still to do after merge: dispatch Deploy Push Gateway Production (cloud-push-deploy.yml, confirmation DEPLOY_PUSH_GATEWAY) to replace the bootstrap image, then device tests (iPhone dev build on sandbox APNs, TestFlight on production, sideloaded Android APK).

@Jinwoo-H
Jinwoo-H marked this pull request as ready for review September 4, 2026 03:09
@greptile-apps

greptile-apps Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Too many files changed for review (210 files, 100 file limit).

Bypass the limit by tagging @greptile-apps to review.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

One operational risk to resolve before shipping to production: the unauthenticated challenge endpoint grows a database table without bound.

Reviewed changes

  • Push gateway (cloud/apps/push, cloud/packages/push-contract) — an hono service that mints box-challenge sessions from the host's X25519 key, registers paired phone tokens, enforces per-host/per-registration quotas, coalesces bursts over a 3 s window, and sends through an APNs HTTP/2 client (ES256 JWT, 50-minute rotation) and an FCM V1 client. Storage mirrors the relay (Postgres in prod, SQLite in tests) with aggregate-only logging.
  • Desktop (src/main) — notifications.remote-push.v1 capability, notifications.registerPush/unregisterPush RPCs gated to mobile callers, a session-caching gateway client with 401 re-auth, a fire-and-forget dispatcher hooked after the socket fan-out (filtered, one retry, drops dead registrations), a durable unregister outbox, agentState on the mobile dispatch event, and the #4375 wording fix so a working agent is no longer announced as finished. The relay host-proof code was refactored onto a shared host-challenge-envelope.ts with no behavior change.
  • Mobile (mobile/) — expo-notifications + google-services.json, native token fetch with sandbox/production detection, per-host registration gated on the capability, foreground dedup against the seen set, fingerprint tap routing, tray dismissal parity, and a settings section with the disclosure and two sub-switches.
  • Infra/docs — push-gateway.tf behind push_gateway_enabled (prod only), a no-traffic blue/green deploy workflow with a validate-only FCM probe, and the mobile-push-contract.md design doc plus runbooks.

⚠️ push_hosts grows without bound from the public challenge endpoint

POST /v1/host/challenge is open to the whole internet (the service sets INGRESS_TRAFFIC_ALL and invoker_iam_disabled = true), and every valid request — a caller just has to send any canonical 32-byte public key, no keypair ownership, no proof completion — runs rememberHost and upserts a permanent row into push_hosts. Every other table (push_challenges, push_sessions, push_send_log) is pruned on a timer, but push_hosts has no TTL and no pruning, and the route has no rate limit. An attacker can fill the shared Cloud SQL instance's push_hosts table indefinitely with one abandoned HTTP request per row, which also degrades the auth and relay workloads that share that instance.

Technical details
# Unbounded push_hosts growth from unauthenticated challenge issuance

## Affected sites
- cloud/apps/push/src/host-challenge-store.ts:73 — `rememberHost(...)` is called inside `issue()`, before any proof is verified, so a public-key-only request already persists a host row.
- cloud/apps/push/src/host-challenge-store.ts:133 — `pruneExpired()` deletes only `push_challenges`; nothing prunes `push_hosts`.
- cloud/apps/push/src/index.ts:35-39 — the timer list prunes challenges, sessions, and send_log; no host pruning.
- cloud/apps/push/src/push-server.ts:120 — the challenge route has no rate limiting, only a body-size cap.
- cloud/infra/terraform/push-gateway.tf — `INGRESS_TRAFFIC_ALL` + `invoker_iam_disabled = true` make the endpoint public by design.

## Required outcome
- A public, unauthenticated request must not create unbounded durable state on the shared database.

## Suggested approach (pick any)
- Rate-limit challenge issuance (global and/or per source IP) so an unauthenticated caller cannot mint rows at will.
- Prune `push_hosts` past a retention window keyed on `last_seen_at` (an active host re-issues challenges and refreshes `last_seen_at`, so only long-dead hosts are reclaimed).
- Defer the `push_hosts` upsert until `verify()` succeeds, so an abandoned challenge writes nothing — this raises the bar (the caller must complete the box open + HMAC) even if a determined attacker with a real keypair can still create rows.

## Open questions for the human
- Is `push_hosts` read for anything operationally load-bearing, or is it a pure "last seen" ledger that can be pruned aggressively?
</details>
<!--
Pullfrog review metadata. These findings were written against 334190c;
if commits have landed on feat/mobile-push since, treat every specific bug, file, or
line callout as POTENTIALLY STALE and re-diff before acting on it.

- Mode: Review (initial)
- Files reviewed: 133
- Commits reviewed: 9
- Base: main (34222e0)
- Head: feat/mobile-push (334190c)
- Reviewed commits:
  - 334190c — docs(cloud): the push.onorca.dev record exists and is hand-managed (#8129)
  - 8c133e2 — fix(cloud): open the push gateway with invoker_iam_disabled, not an allUsers binding (#8129)
  - a650941 — test(push): replay one checked-in host-proof vector on both sides (#8129)
  - cc4bd92 — docs: check in the mobile push contract (#8129)
  - 52c3b20 — docs: describe background notifications on the phone (#8129)
  - 0987c98 — chore(cloud): Terraform and deploy workflow for the push gateway (#8129)
  - 2dfabb3 — feat(mobile): background push registration, receive, and settings (#8129)
  - 4b7c9b0 — feat(push): register phones and send background push from the desktop (#8129)
  - 6e44c0f — feat(cloud): add the mobile push gateway and its contract package (#8129)
- Prior pullfrog review: none
-->

<!-- PULLFROG_DIVIDER_DO_NOT_REMOVE_PLZ -->
<sup><a href="https://pullfrog.com"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pullfrog.com/logos/frog-white-full-18px.png"><img src="https://pullfrog.com/logos/frog-green-full-18px.png" width="9px" height="9px" style="vertical-align: middle; " alt="Pullfrog"></picture></a>&nbsp;&nbsp;| [Fix it ➔](https://pullfrog.com/trigger/stablyai/orca/18554?action=fix&review_id=5108910494) | [View workflow run](https://github.com/stablyai/orca/actions/runs/33832156191/job/100897236013) | Using `DeepSeek Pro` (free via [Pullfrog for OSS](https://pullfrog.com/for-oss)) | [𝕏](https://x.com/pullfrogai)</sup>

@coderabbitai

coderabbitai Bot commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

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

This change adds end-to-end mobile push support. It defines shared contracts and host authentication, implements a Cloud Run gateway with APNs and FCM delivery, and adds desktop registration, dispatch, retry, and cleanup flows. The mobile app adds token registration, capability-gated settings, foreground deduplication, tap routing, and push dismissal. Terraform provisions production resources, and a gated workflow deploys verified revisions with serialized Cloud SQL rollout handling.

Merge Risk: 🟡 Moderate · up to 724e3

This adds opt-in background push delivery, but unresolved concurrency and lifecycle issues can cause authentication inconsistencies, blocked replacement-device registration, lost notifications, or unreliable iOS delivery. These issues should be resolved before merge.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Most changes support the push-notification feature, but cloud/package.json removes two relay staging test files from the test command without a stated push-related requirement. This is unrelated scope… Restore the two relay staging test files to the cloud test command, or move their removal to a separate pull request with a clear justification and replacement coverage.
Docstring Coverage ⚠️ Warning Docstring coverage is 18.87% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 106 functions across 69 files. (4 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: adding real background push notifications for the mobile app.
Description check ✅ Passed The description covers the required sections, explains the design and scope, documents testing, and identifies known pending physical-device and deployment validation.
Linked Issues check ✅ Passed The implementation satisfies issue #8129 by adding opt-in background delivery, preserving socket delivery, preventing duplicates, and supporting delivery while the app is suspended or disconnected. Th…
Full details: Out of Scope Changes check

Explanation

Most changes support the push-notification feature, but cloud/package.json removes two relay staging test files from the test command without a stated push-related requirement. This is unrelated scope expansion and weakens test coverage.

Full details: Docstring Coverage

Explanation

Docstring coverage is 18.87% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 106 functions across 69 files. (4 skipped: 4 unsupported.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 12


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 2f679f24-3b59-4b52-9909-5ca0da3a8bcd

📥 Commits

Reviewing files that changed from the base of the PR and between 7106101 and 334190c.

⛔ Files ignored due to path filters (1)
  • cloud/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (132)
  • .github/workflows/cloud-push-deploy.yml
  • .gitignore
  • cloud/README.md
  • cloud/apps/push/Dockerfile
  • cloud/apps/push/package.json
  • cloud/apps/push/src/apns-authentication-token.ts
  • cloud/apps/push/src/apns-client.test.ts
  • cloud/apps/push/src/apns-client.ts
  • cloud/apps/push/src/apns-http2-transport.ts
  • cloud/apps/push/src/canonical-base64.ts
  • cloud/apps/push/src/coalescer.test.ts
  • cloud/apps/push/src/coalescer.ts
  • cloud/apps/push/src/config.test.ts
  • cloud/apps/push/src/config.ts
  • cloud/apps/push/src/desktop-host-proof-interop.test.ts
  • cloud/apps/push/src/device-registry-store.test.ts
  • cloud/apps/push/src/device-registry-store.ts
  • cloud/apps/push/src/fcm-access-token.ts
  • cloud/apps/push/src/fcm-client.test.ts
  • cloud/apps/push/src/fcm-client.ts
  • cloud/apps/push/src/host-challenge-answering.test-fixture.ts
  • cloud/apps/push/src/host-challenge-store.test.ts
  • cloud/apps/push/src/host-challenge-store.ts
  • cloud/apps/push/src/host-fingerprint.ts
  • cloud/apps/push/src/host-session-store.test.ts
  • cloud/apps/push/src/host-session-store.ts
  • cloud/apps/push/src/index.ts
  • cloud/apps/push/src/push-database.ts
  • cloud/apps/push/src/push-delivery-message.ts
  • cloud/apps/push/src/push-dispatcher.ts
  • cloud/apps/push/src/push-observability.ts
  • cloud/apps/push/src/push-provider-outcome.ts
  • cloud/apps/push/src/push-readiness.ts
  • cloud/apps/push/src/push-schema.ts
  • cloud/apps/push/src/push-server-auth.test.ts
  • cloud/apps/push/src/push-server-harness.test-fixture.ts
  • cloud/apps/push/src/push-server-send.test.ts
  • cloud/apps/push/src/push-server.ts
  • cloud/apps/push/src/send-quota.test.ts
  • cloud/apps/push/src/send-quota.ts
  • cloud/apps/push/tsconfig.build.json
  • cloud/apps/push/tsconfig.json
  • cloud/apps/push/vitest.config.ts
  • cloud/dev/fixtures/terraform-root-partition/families.json
  • cloud/dev/scripts/cloud-sql-rollout-lock-census.mjs
  • cloud/dev/scripts/push-gateway-workflow.test.mjs
  • cloud/dev/scripts/relay-production-identity-boundaries.test.mjs
  • cloud/dev/scripts/relay-public-workflow-contract.test.mjs
  • cloud/dev/scripts/workload-identity-attribute-conditions.test.mjs
  • cloud/docs/push-gateway.md
  • cloud/docs/relay-workflows.md
  • cloud/infra/terraform/environments/production.tfvars
  • cloud/infra/terraform/environments/staging.tfvars
  • cloud/infra/terraform/outputs.tf
  • cloud/infra/terraform/push-gateway.tf
  • cloud/infra/terraform/relay-github-actions.tf
  • cloud/infra/terraform/variables.tf
  • cloud/package.json
  • cloud/packages/push-contract/package.json
  • cloud/packages/push-contract/src/contract.test.ts
  • cloud/packages/push-contract/src/device-registration-messages.ts
  • cloud/packages/push-contract/src/host-auth-messages.ts
  • cloud/packages/push-contract/src/index.ts
  • cloud/packages/push-contract/src/push-host-proof-transcript.test.ts
  • cloud/packages/push-contract/src/push-host-proof-transcript.ts
  • cloud/packages/push-contract/src/push-host-proof-vector.json
  • cloud/packages/push-contract/src/push-limits.ts
  • cloud/packages/push-contract/src/send-messages.ts
  • cloud/packages/push-contract/src/wire-scalars.ts
  • cloud/packages/push-contract/tsconfig.build.json
  • cloud/packages/push-contract/tsconfig.json
  • docs/reference/headless-linux-server.md
  • docs/reference/mobile-push-contract.md
  • docs/site/content/docs/mobile.mdx
  • docs/site/content/docs/notifications.mdx
  • mobile/app.json
  • mobile/app/_layout.tsx
  • mobile/app/notifications.tsx
  • mobile/google-services.json
  • mobile/src/home/use-mobile-home-host-connections.ts
  • mobile/src/notifications/BackgroundNotificationsSection.test.tsx
  • mobile/src/notifications/BackgroundNotificationsSection.tsx
  • mobile/src/notifications/local-notification-scheduling.ts
  • mobile/src/notifications/push-host-fingerprint.test.ts
  • mobile/src/notifications/push-host-fingerprint.ts
  • mobile/src/notifications/push-payload.ts
  • mobile/src/notifications/push-receive.test.ts
  • mobile/src/notifications/push-receive.ts
  • mobile/src/notifications/push-registration.test.ts
  • mobile/src/notifications/push-registration.ts
  • mobile/src/notifications/push-token.test.ts
  • mobile/src/notifications/push-token.ts
  • mobile/src/notifications/push-tray-dismissal.test.ts
  • mobile/src/notifications/push-tray-dismissal.ts
  • mobile/src/notifications/use-remote-push-capable-hosts.ts
  • mobile/src/storage/preferences.ts
  • mobile/src/transport/host-removal-lifecycle.test.ts
  • mobile/src/transport/host-removal-lifecycle.ts
  • src/main/global-fetch-call-site-audit.test.ts
  • src/main/ipc/notification-options.ts
  • src/main/ipc/notifications-message-formatting.test.ts
  • src/main/ipc/notifications-mobile-fanout.test.ts
  • src/main/ipc/notifications.ts
  • src/main/orca-profiles/profile-cloud-auth-config.ts
  • src/main/runtime/device-registry.ts
  • src/main/runtime/host-challenge-envelope.ts
  • src/main/runtime/push/desktop-push-service.test.ts
  • src/main/runtime/push/desktop-push-service.ts
  • src/main/runtime/push/push-device-registration-persistence.test.ts
  • src/main/runtime/push/push-dispatcher.test.ts
  • src/main/runtime/push/push-dispatcher.ts
  • src/main/runtime/push/push-gateway-client.test.ts
  • src/main/runtime/push/push-gateway-client.ts
  • src/main/runtime/push/push-host-challenge-fixtures.ts
  • src/main/runtime/push/push-host-proof-vector.test.ts
  • src/main/runtime/push/push-host-proof.test.ts
  • src/main/runtime/push/push-host-proof.ts
  • src/main/runtime/push/push-registration-rpc.test.ts
  • src/main/runtime/push/push-unregister-outbox.test.ts
  • src/main/runtime/push/push-unregister-outbox.ts
  • src/main/runtime/relay/relay-host-proof.ts
  • src/main/runtime/rpc/methods/notifications.ts
  • src/main/runtime/runtime-mobile-notification-controller.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-pairing.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-state.ts
  • src/main/runtime/runtime-service-command-surface.ts
  • src/main/startup/main-process-push-startup.ts
  • src/main/startup/main-process-runtime-launch.ts
  • src/main/startup/main-process-state.ts
  • src/shared/mobile-push-contract.ts
  • src/shared/protocol-version.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Comment thread .github/workflows/cloud-push-deploy.yml Resolved
Comment thread cloud/apps/push/src/push-database.ts Resolved
Comment thread cloud/apps/push/src/push-server.ts Resolved
@Jinwoo-H

Jinwoo-H commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Review round: bot findings triaged, four independent reviews, fixes landed

Every CodeRabbit and Pullfrog finding was checked against the code; each inline thread has its disposition. Ten were confirmed and fixed, two were refuted (one applied anyway as hardening). Four Opus reviewers then went over each lane independently and found more. Fixed in four commits, one per lane: 7bd9cdd270 (gateway), 5a6d1e8226 (desktop), 1600abe344 (mobile), 9fb9513309 (infra).

Blockers found by the review round, not by the bots

  • Gateway quota was not enforced at all under Postgres: 80 concurrent sends against a 60/hour cap were all admitted (reproduced). Fixed with a per-host advisory lock, proven by a Postgres-gated test.
  • The bare expo-notifications plugin entry writes aps-environment: development, while the phone reports production for every release build. TestFlight and App Store builds would have registered a token Apple never delivers to. Fixed with app.config.js driving the mode from an env var the iOS release workflow sets; verified by a real expo prebuild into a temp copy.
  • A foreground push on a session never seeded from disk reset the watermark to 0, so the next reconnect replayed the desktop's whole buffer as banners. Fixed by seeding before epoch adoption.
  • The shared Cloud SQL instance had 5 spare connections; the gateway's undeclared default pool asked for 40. The pool is now declared in Terraform (2 per instance, max 2 instances), the gateway is in the connection budget script, and the budget is green with 1 to spare. Applied to production.

Also fixed: push_hosts written before proof, no pruner, no rate limit on the public routes; chunked-body bypass of the 16 KiB cap; per-host device cap; challenge TTL 40 s instead of 10 s; no rollback after a failed post-shift check; deploy workflow mutating Terraform-owned scaling; lease held across the Docker build; consent race on switch-off; latched capability probe blocking unregister; duplicate banner on reopen; orphaned gateway registration on persist failure; outbox never retrying; devices past 20 starved.

Verification: root typecheck, push/relay/registry/ipc suites (672 pass), max-lines ratchet, changed-code quality gate all green. Mobile typecheck, lint, format, 808 tests green. Cloud typecheck, full test script 515 + push 102 + contract 18 green, terraform fmt/validate clean. One local-only failure is pre-existing on main (an Xvfb test that times out on this Mac).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
cloud/infra/terraform/push-gateway.tf (1)

304-307: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The variable defaults cannot satisfy this precondition.

push_max_instances defaults to 4 and push_database_pool_max defaults to 2, so the default product is 8. Production passes because production.tfvars sets push_max_instances = 2. Any new environment that sets only push_gateway_enabled = true fails at plan time with this error message and no obvious cause. Align the defaults with the bound, or state the required override in the variable description.

♻️ Proposed change in cloud/infra/terraform/variables.tf
 variable "push_max_instances" {
   type    = number
-  default = 4
+  # Why: push_max_instances x push_database_pool_max must stay within the gateway's
+  # 4-connection share of the shared Cloud SQL instance; see push-gateway.tf.
+  default = 2
cloud/apps/push/src/send-quota-postgres.test.ts (1)

81-82: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Bind the per-host reserve count to PUSH_LIMITS.hostSendsPerRollingHour.

The current limit is 60, so 40 reserves pass today. If the limit drops below 40, the test can fail because of quota exhaustion instead of testing host isolation. Use Math.min(40, PUSH_LIMITS.hostSendsPerRollingHour) for the per-host count.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 1e55d71e-a635-4b7d-880d-189443ac5e7c

📥 Commits

Reviewing files that changed from the base of the PR and between 334190c and 9fb95133099c6941366d93210f3fc9af9e9e2e0e.

📒 Files selected for processing (69)
  • .github/workflows/cloud-push-deploy.yml
  • .github/workflows/mobile-ios-release.yml
  • cloud/apps/push/src/apns-http2-transport.ts
  • cloud/apps/push/src/apns-stream-response.test.ts
  • cloud/apps/push/src/apns-stream-response.ts
  • cloud/apps/push/src/client-ip-rate-limit.test.ts
  • cloud/apps/push/src/client-ip-rate-limit.ts
  • cloud/apps/push/src/device-registry-store.test.ts
  • cloud/apps/push/src/device-registry-store.ts
  • cloud/apps/push/src/host-challenge-store.test.ts
  • cloud/apps/push/src/host-challenge-store.ts
  • cloud/apps/push/src/index.ts
  • cloud/apps/push/src/push-database.ts
  • cloud/apps/push/src/push-observability.ts
  • cloud/apps/push/src/push-schema.ts
  • cloud/apps/push/src/push-server-auth.test.ts
  • cloud/apps/push/src/push-server-limits.test.ts
  • cloud/apps/push/src/push-server.ts
  • cloud/apps/push/src/send-quota-postgres.test.ts
  • cloud/apps/push/src/send-quota.ts
  • cloud/dev/scripts/push-gateway-workflow.test.mjs
  • cloud/dev/scripts/relay-cloud-sql-connection-budget.mjs
  • cloud/dev/scripts/relay-cloud-sql-connection-budget.test.mjs
  • cloud/docs/push-gateway.md
  • cloud/docs/relay-workflows.md
  • cloud/infra/terraform/environments/production.tfvars
  • cloud/infra/terraform/push-gateway.tf
  • cloud/infra/terraform/relay-github-actions.tf
  • cloud/infra/terraform/variables.tf
  • cloud/packages/push-contract/src/contract.test.ts
  • cloud/packages/push-contract/src/device-registration-messages.ts
  • cloud/packages/push-contract/src/host-auth-messages.ts
  • cloud/packages/push-contract/src/push-limits.ts
  • cloud/packages/push-contract/src/send-messages.test.ts
  • cloud/packages/push-contract/src/send-messages.ts
  • docs/reference/mobile-push-contract.md
  • docs/site/content/docs/notifications.mdx
  • mobile/app.config.js
  • mobile/app/_layout.tsx
  • mobile/src/notifications/desktop-notification-channel.test.ts
  • mobile/src/notifications/desktop-notification-channel.ts
  • mobile/src/notifications/local-notification-scheduling.ts
  • mobile/src/notifications/mobile-notifications.test.ts
  • mobile/src/notifications/mobile-notifications.ts
  • mobile/src/notifications/notification-catchup-failure-quarantine.test.ts
  • mobile/src/notifications/notification-delivery-ordering.test.ts
  • mobile/src/notifications/notification-reconnect-teardown.test.ts
  • mobile/src/notifications/notification-reopen-push-duplicate.test.ts
  • mobile/src/notifications/notification-watermark-seed-race.test.ts
  • mobile/src/notifications/push-payload.ts
  • mobile/src/notifications/push-receive.test.ts
  • mobile/src/notifications/push-receive.ts
  • mobile/src/notifications/push-registration.test.ts
  • mobile/src/notifications/push-registration.ts
  • mobile/src/notifications/push-tray-seen-seed.test.ts
  • mobile/src/notifications/push-tray-seen-seed.ts
  • mobile/src/notifications/use-remote-push-capable-hosts.test.tsx
  • mobile/src/notifications/use-remote-push-capable-hosts.ts
  • src/main/runtime/push/desktop-push-service.test.ts
  • src/main/runtime/push/desktop-push-service.ts
  • src/main/runtime/push/push-dispatcher.test.ts
  • src/main/runtime/push/push-dispatcher.ts
  • src/main/runtime/push/push-gateway-client.test.ts
  • src/main/runtime/push/push-gateway-client.ts
  • src/main/runtime/push/push-gateway-response.ts
  • src/main/runtime/push/push-gateway-session.test.ts
  • src/main/runtime/push/push-gateway-session.ts
  • src/main/startup/main-process-quit.ts
  • src/shared/mobile-push-contract.ts
🚧 Files skipped from review as they are similar to previous changes (10)
  • cloud/infra/terraform/relay-github-actions.tf
  • cloud/apps/push/src/push-schema.ts
  • mobile/src/notifications/push-receive.test.ts
  • cloud/apps/push/src/push-server-auth.test.ts
  • cloud/apps/push/src/push-database.ts
  • mobile/src/notifications/push-receive.ts
  • cloud/docs/push-gateway.md
  • mobile/src/notifications/push-registration.ts
  • cloud/apps/push/src/index.ts
  • cloud/docs/relay-workflows.md

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.

Comment thread docs/reference/mobile-push-contract.md Resolved
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 6


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: bf097547-8596-44bc-ac06-4882d9d5891f

📥 Commits

Reviewing files that changed from the base of the PR and between f36c03e and 044e91a.

⛔ Files ignored due to path filters (1)
  • cloud/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (159)
  • .github/workflows/cloud-push-deploy.yml
  • .github/workflows/mobile-ios-release.yml
  • .gitignore
  • cloud/README.md
  • cloud/apps/push/Dockerfile
  • cloud/apps/push/package.json
  • cloud/apps/push/src/apns-authentication-token.ts
  • cloud/apps/push/src/apns-client.test.ts
  • cloud/apps/push/src/apns-client.ts
  • cloud/apps/push/src/apns-http2-transport.ts
  • cloud/apps/push/src/apns-stream-response.test.ts
  • cloud/apps/push/src/apns-stream-response.ts
  • cloud/apps/push/src/canonical-base64.ts
  • cloud/apps/push/src/client-ip-rate-limit.test.ts
  • cloud/apps/push/src/client-ip-rate-limit.ts
  • cloud/apps/push/src/coalescer.test.ts
  • cloud/apps/push/src/coalescer.ts
  • cloud/apps/push/src/config.test.ts
  • cloud/apps/push/src/config.ts
  • cloud/apps/push/src/desktop-host-proof-interop.test.ts
  • cloud/apps/push/src/device-registry-store.test.ts
  • cloud/apps/push/src/device-registry-store.ts
  • cloud/apps/push/src/fcm-access-token.ts
  • cloud/apps/push/src/fcm-client.test.ts
  • cloud/apps/push/src/fcm-client.ts
  • cloud/apps/push/src/host-challenge-answering.test-fixture.ts
  • cloud/apps/push/src/host-challenge-store.test.ts
  • cloud/apps/push/src/host-challenge-store.ts
  • cloud/apps/push/src/host-fingerprint.ts
  • cloud/apps/push/src/host-session-store.test.ts
  • cloud/apps/push/src/host-session-store.ts
  • cloud/apps/push/src/index.ts
  • cloud/apps/push/src/push-database.ts
  • cloud/apps/push/src/push-delivery-message.ts
  • cloud/apps/push/src/push-dispatcher.ts
  • cloud/apps/push/src/push-observability.ts
  • cloud/apps/push/src/push-provider-outcome.ts
  • cloud/apps/push/src/push-readiness.ts
  • cloud/apps/push/src/push-schema.ts
  • cloud/apps/push/src/push-server-auth.test.ts
  • cloud/apps/push/src/push-server-harness.test-fixture.ts
  • cloud/apps/push/src/push-server-limits.test.ts
  • cloud/apps/push/src/push-server-send.test.ts
  • cloud/apps/push/src/push-server.ts
  • cloud/apps/push/src/send-quota-postgres.test.ts
  • cloud/apps/push/src/send-quota.test.ts
  • cloud/apps/push/src/send-quota.ts
  • cloud/apps/push/tsconfig.build.json
  • cloud/apps/push/tsconfig.json
  • cloud/apps/push/vitest.config.ts
  • cloud/dev/fixtures/terraform-root-partition/families.json
  • cloud/dev/scripts/cloud-sql-rollout-lock-census.mjs
  • cloud/dev/scripts/push-gateway-workflow.test.mjs
  • cloud/dev/scripts/relay-cloud-sql-connection-budget.mjs
  • cloud/dev/scripts/relay-cloud-sql-connection-budget.test.mjs
  • cloud/dev/scripts/relay-production-identity-boundaries.test.mjs
  • cloud/dev/scripts/relay-public-workflow-contract.test.mjs
  • cloud/dev/scripts/workload-identity-attribute-conditions.test.mjs
  • cloud/docs/push-gateway.md
  • cloud/docs/relay-workflows.md
  • cloud/infra/terraform/environments/production.tfvars
  • cloud/infra/terraform/environments/staging.tfvars
  • cloud/infra/terraform/outputs.tf
  • cloud/infra/terraform/push-gateway.tf
  • cloud/infra/terraform/relay-github-actions.tf
  • cloud/infra/terraform/variables.tf
  • cloud/package.json
  • cloud/packages/push-contract/package.json
  • cloud/packages/push-contract/src/contract.test.ts
  • cloud/packages/push-contract/src/device-registration-messages.ts
  • cloud/packages/push-contract/src/host-auth-messages.ts
  • cloud/packages/push-contract/src/index.ts
  • cloud/packages/push-contract/src/push-host-proof-transcript.test.ts
  • cloud/packages/push-contract/src/push-host-proof-transcript.ts
  • cloud/packages/push-contract/src/push-host-proof-vector.json
  • cloud/packages/push-contract/src/push-limits.ts
  • cloud/packages/push-contract/src/send-messages.test.ts
  • cloud/packages/push-contract/src/send-messages.ts
  • cloud/packages/push-contract/src/wire-scalars.ts
  • cloud/packages/push-contract/tsconfig.build.json
  • cloud/packages/push-contract/tsconfig.json
  • docs/reference/headless-linux-server.md
  • docs/reference/mobile-push-contract.md
  • docs/site/content/docs/mobile.mdx
  • docs/site/content/docs/notifications.mdx
  • mobile/app.config.js
  • mobile/app.json
  • mobile/app/_layout.tsx
  • mobile/app/notifications.tsx
  • mobile/google-services.json
  • mobile/src/home/use-mobile-home-host-connections.ts
  • mobile/src/notifications/BackgroundNotificationsSection.test.tsx
  • mobile/src/notifications/BackgroundNotificationsSection.tsx
  • mobile/src/notifications/desktop-notification-channel.test.ts
  • mobile/src/notifications/desktop-notification-channel.ts
  • mobile/src/notifications/local-notification-scheduling.ts
  • mobile/src/notifications/mobile-notifications.test.ts
  • mobile/src/notifications/mobile-notifications.ts
  • mobile/src/notifications/notification-catchup-failure-quarantine.test.ts
  • mobile/src/notifications/notification-delivery-ordering.test.ts
  • mobile/src/notifications/notification-reconnect-teardown.test.ts
  • mobile/src/notifications/notification-reopen-push-duplicate.test.ts
  • mobile/src/notifications/notification-watermark-seed-race.test.ts
  • mobile/src/notifications/push-host-fingerprint.test.ts
  • mobile/src/notifications/push-host-fingerprint.ts
  • mobile/src/notifications/push-payload.ts
  • mobile/src/notifications/push-receive.test.ts
  • mobile/src/notifications/push-receive.ts
  • mobile/src/notifications/push-registration.test.ts
  • mobile/src/notifications/push-registration.ts
  • mobile/src/notifications/push-token.test.ts
  • mobile/src/notifications/push-token.ts
  • mobile/src/notifications/push-tray-dismissal.test.ts
  • mobile/src/notifications/push-tray-dismissal.ts
  • mobile/src/notifications/push-tray-seen-seed.test.ts
  • mobile/src/notifications/push-tray-seen-seed.ts
  • mobile/src/notifications/use-remote-push-capable-hosts.test.tsx
  • mobile/src/notifications/use-remote-push-capable-hosts.ts
  • mobile/src/storage/preferences.ts
  • mobile/src/transport/host-removal-lifecycle.test.ts
  • mobile/src/transport/host-removal-lifecycle.ts
  • src/main/global-fetch-call-site-audit.test.ts
  • src/main/ipc/notification-options.ts
  • src/main/ipc/notifications-message-formatting.test.ts
  • src/main/ipc/notifications-mobile-fanout.test.ts
  • src/main/ipc/notifications.ts
  • src/main/orca-profiles/profile-cloud-auth-config.ts
  • src/main/runtime/device-registry.ts
  • src/main/runtime/host-challenge-envelope.ts
  • src/main/runtime/push/desktop-push-service.test.ts
  • src/main/runtime/push/desktop-push-service.ts
  • src/main/runtime/push/push-device-registration-persistence.test.ts
  • src/main/runtime/push/push-dispatcher.test.ts
  • src/main/runtime/push/push-dispatcher.ts
  • src/main/runtime/push/push-gateway-client.test.ts
  • src/main/runtime/push/push-gateway-client.ts
  • src/main/runtime/push/push-gateway-response.ts
  • src/main/runtime/push/push-gateway-session.test.ts
  • src/main/runtime/push/push-gateway-session.ts
  • src/main/runtime/push/push-host-challenge-fixtures.ts
  • src/main/runtime/push/push-host-proof-vector.test.ts
  • src/main/runtime/push/push-host-proof.test.ts
  • src/main/runtime/push/push-host-proof.ts
  • src/main/runtime/push/push-registration-rpc.test.ts
  • src/main/runtime/push/push-unregister-outbox.test.ts
  • src/main/runtime/push/push-unregister-outbox.ts
  • src/main/runtime/relay/relay-host-proof.ts
  • src/main/runtime/rpc/methods/notifications.ts
  • src/main/runtime/runtime-mobile-notification-controller.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-pairing.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-state.ts
  • src/main/runtime/runtime-service-command-surface.ts
  • src/main/startup/main-process-push-startup.ts
  • src/main/startup/main-process-quit.ts
  • src/main/startup/main-process-runtime-launch.ts
  • src/main/startup/main-process-state.ts
  • src/shared/mobile-push-contract.ts
  • src/shared/protocol-version.ts
🚧 Files skipped from review as they are similar to previous changes (139)
  • cloud/package.json
  • cloud/packages/push-contract/tsconfig.build.json
  • cloud/packages/push-contract/src/push-limits.ts
  • cloud/infra/terraform/environments/staging.tfvars
  • cloud/apps/push/src/host-fingerprint.ts
  • cloud/packages/push-contract/tsconfig.json
  • mobile/app.json
  • cloud/apps/push/tsconfig.build.json
  • mobile/google-services.json
  • cloud/apps/push/src/fcm-access-token.ts
  • src/main/global-fetch-call-site-audit.test.ts
  • cloud/dev/scripts/relay-public-workflow-contract.test.mjs
  • cloud/apps/push/src/push-readiness.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-state.ts
  • cloud/apps/push/src/host-session-store.test.ts
  • cloud/apps/push/tsconfig.json
  • src/main/startup/main-process-push-startup.ts
  • cloud/apps/push/src/canonical-base64.ts
  • mobile/src/notifications/use-remote-push-capable-hosts.ts
  • mobile/src/notifications/push-tray-dismissal.test.ts
  • mobile/src/notifications/notification-delivery-ordering.test.ts
  • cloud/packages/push-contract/src/push-host-proof-vector.json
  • cloud/dev/scripts/cloud-sql-rollout-lock-census.mjs
  • cloud/apps/push/vitest.config.ts
  • mobile/src/notifications/push-registration.test.ts
  • cloud/apps/push/package.json
  • src/main/startup/main-process-quit.ts
  • src/main/runtime/push/push-dispatcher.test.ts
  • src/main/runtime/push/push-unregister-outbox.test.ts
  • cloud/apps/push/src/send-quota.test.ts
  • cloud/apps/push/src/push-provider-outcome.ts
  • mobile/app.config.js
  • mobile/src/notifications/notification-catchup-failure-quarantine.test.ts
  • .github/workflows/mobile-ios-release.yml
  • cloud/apps/push/src/send-quota-postgres.test.ts
  • src/main/ipc/notifications.ts
  • src/main/ipc/notifications-message-formatting.test.ts
  • cloud/apps/push/src/config.test.ts
  • mobile/src/notifications/push-host-fingerprint.ts
  • cloud/apps/push/src/client-ip-rate-limit.test.ts
  • cloud/dev/scripts/relay-cloud-sql-connection-budget.mjs
  • mobile/src/transport/host-removal-lifecycle.test.ts
  • mobile/src/notifications/push-payload.ts
  • cloud/apps/push/src/host-session-store.ts
  • mobile/src/notifications/push-host-fingerprint.test.ts
  • mobile/src/transport/host-removal-lifecycle.ts
  • mobile/src/notifications/desktop-notification-channel.ts
  • mobile/src/notifications/push-token.ts
  • mobile/src/notifications/mobile-notifications.test.ts
  • src/main/ipc/notifications-mobile-fanout.test.ts
  • cloud/apps/push/src/push-dispatcher.ts
  • cloud/infra/terraform/outputs.tf
  • cloud/packages/push-contract/package.json
  • cloud/dev/scripts/workload-identity-attribute-conditions.test.mjs
  • cloud/packages/push-contract/src/push-host-proof-transcript.test.ts
  • cloud/apps/push/src/desktop-host-proof-interop.test.ts
  • cloud/apps/push/Dockerfile
  • src/main/runtime/push/push-gateway-session.ts
  • mobile/src/notifications/push-receive.test.ts
  • mobile/src/notifications/notification-watermark-seed-race.test.ts
  • cloud/apps/push/src/coalescer.ts
  • cloud/infra/terraform/relay-github-actions.tf
  • cloud/apps/push/src/push-server-auth.test.ts
  • src/main/runtime/push/desktop-push-service.ts
  • cloud/apps/push/src/client-ip-rate-limit.ts
  • mobile/src/notifications/BackgroundNotificationsSection.tsx
  • mobile/src/notifications/push-tray-seen-seed.ts
  • cloud/packages/push-contract/src/index.ts
  • cloud/apps/push/src/push-observability.ts
  • cloud/infra/terraform/environments/production.tfvars
  • cloud/apps/push/src/apns-stream-response.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts
  • src/main/runtime/rpc/methods/notifications.ts
  • src/main/runtime/push/push-host-proof.test.ts
  • cloud/dev/scripts/relay-cloud-sql-connection-budget.test.mjs
  • mobile/src/notifications/push-token.test.ts
  • mobile/app/_layout.tsx
  • cloud/dev/scripts/relay-production-identity-boundaries.test.mjs
  • src/main/runtime/push/push-gateway-response.ts
  • mobile/src/notifications/desktop-notification-channel.test.ts
  • cloud/apps/push/src/config.ts
  • mobile/src/notifications/BackgroundNotificationsSection.test.tsx
  • mobile/src/home/use-mobile-home-host-connections.ts
  • src/main/runtime/push/push-gateway-session.test.ts
  • src/main/runtime/push/desktop-push-service.test.ts
  • src/main/runtime/push/push-device-registration-persistence.test.ts
  • cloud/apps/push/src/push-database.ts
  • cloud/dev/fixtures/terraform-root-partition/families.json
  • mobile/app/notifications.tsx
  • cloud/apps/push/src/apns-authentication-token.ts
  • src/main/runtime/device-registry.ts
  • src/main/runtime/host-challenge-envelope.ts
  • mobile/src/notifications/push-tray-seen-seed.test.ts
  • cloud/apps/push/src/fcm-client.test.ts
  • cloud/apps/push/src/push-schema.ts
  • src/main/runtime/runtime-mobile-notification-controller.ts
  • mobile/src/notifications/mobile-notifications.ts
  • src/main/runtime/push/push-host-proof.ts
  • cloud/apps/push/src/fcm-client.ts
  • src/main/runtime/push/push-unregister-outbox.ts
  • src/shared/protocol-version.ts
  • cloud/apps/push/src/push-delivery-message.ts
  • mobile/src/notifications/local-notification-scheduling.ts
  • cloud/apps/push/src/index.ts
  • cloud/apps/push/src/apns-stream-response.test.ts
  • cloud/packages/push-contract/src/send-messages.test.ts
  • src/main/startup/main-process-state.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-pairing.ts
  • cloud/apps/push/src/apns-client.ts
  • mobile/src/notifications/use-remote-push-capable-hosts.test.tsx
  • src/main/runtime/runtime-service-command-surface.ts
  • src/main/orca-profiles/profile-cloud-auth-config.ts
  • mobile/src/notifications/notification-reopen-push-duplicate.test.ts
  • src/main/runtime/relay/relay-host-proof.ts
  • cloud/apps/push/src/apns-client.test.ts
  • cloud/apps/push/src/coalescer.test.ts
  • cloud/docs/relay-workflows.md
  • mobile/src/notifications/push-registration.ts
  • cloud/apps/push/src/host-challenge-answering.test-fixture.ts
  • src/main/runtime/push/push-registration-rpc.test.ts
  • .gitignore
  • cloud/packages/push-contract/src/push-host-proof-transcript.ts
  • cloud/dev/scripts/push-gateway-workflow.test.mjs
  • cloud/apps/push/src/push-server-harness.test-fixture.ts
  • src/main/ipc/notification-options.ts
  • src/main/startup/main-process-runtime-launch.ts
  • mobile/src/notifications/notification-reconnect-teardown.test.ts
  • cloud/apps/push/src/push-server-send.test.ts
  • cloud/docs/push-gateway.md
  • src/main/runtime/push/push-gateway-client.ts
  • cloud/apps/push/src/host-challenge-store.test.ts
  • cloud/packages/push-contract/src/wire-scalars.ts
  • cloud/infra/terraform/variables.tf
  • cloud/packages/push-contract/src/device-registration-messages.ts
  • src/main/runtime/push/push-host-challenge-fixtures.ts
  • cloud/apps/push/src/push-server-limits.test.ts
  • mobile/src/storage/preferences.ts
  • cloud/packages/push-contract/src/send-messages.ts
  • cloud/apps/push/src/push-server.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Comment on lines +89 to +90
'SELECT COUNT(*) AS devices FROM push_devices WHERE host_fingerprint = ?',
[input.hostFingerprint]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Exclude retired registrations from the device cap.

This count includes rows that markDead retired. After a host reaches the cap with dead registrations, upsert rejects a replacement device with too_many_devices even when no active registration can receive a push. Count only rows with dead_at IS NULL, or delete retired rows. Add a regression test for this case.

Comment on lines +164 to +173
const [updated] = await transaction.query(
'UPDATE push_hosts SET last_seen_at = ?, host_public_key = ? WHERE host_fingerprint = ?',
[now, hostPublicKeyB64, hostFingerprint]
)
if (Number(updated?.changes ?? 0) > 0) return
await transaction.query(
`INSERT INTO push_hosts (host_fingerprint, host_public_key, created_at, last_seen_at)
VALUES (?, ?, ?, ?)`,
[hostFingerprint, hostPublicKeyB64, now, now]
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Use one atomic upsert for push_hosts.

Lines 164-173 perform an update-then-insert sequence. Two concurrent valid proofs for the same new host can both observe zero updated rows. One insert succeeds and the other fails on the host_fingerprint primary key. The second session request then fails although its proof is valid.

Use INSERT ... ON CONFLICT (host_fingerprint) DO UPDATE in one statement.

Proposed fix
-    const [updated] = await transaction.query(
-      'UPDATE push_hosts SET last_seen_at = ?, host_public_key = ? WHERE host_fingerprint = ?',
-      [now, hostPublicKeyB64, hostFingerprint]
-    )
-    if (Number(updated?.changes ?? 0) > 0) return
     await transaction.query(
       `INSERT INTO push_hosts (host_fingerprint, host_public_key, created_at, last_seen_at)
-       VALUES (?, ?, ?, ?)`,
+       VALUES (?, ?, ?, ?)
+       ON CONFLICT (host_fingerprint) DO UPDATE
+       SET host_public_key = EXCLUDED.host_public_key,
+           last_seen_at = EXCLUDED.last_seen_at`,
       [hostFingerprint, hostPublicKeyB64, now, now]
     )
📝 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
const [updated] = await transaction.query(
'UPDATE push_hosts SET last_seen_at = ?, host_public_key = ? WHERE host_fingerprint = ?',
[now, hostPublicKeyB64, hostFingerprint]
)
if (Number(updated?.changes ?? 0) > 0) return
await transaction.query(
`INSERT INTO push_hosts (host_fingerprint, host_public_key, created_at, last_seen_at)
VALUES (?, ?, ?, ?)`,
[hostFingerprint, hostPublicKeyB64, now, now]
)
await transaction.query(
`INSERT INTO push_hosts (host_fingerprint, host_public_key, created_at, last_seen_at)
VALUES (?, ?, ?, ?)
ON CONFLICT (host_fingerprint) DO UPDATE
SET host_public_key = EXCLUDED.host_public_key,
last_seen_at = EXCLUDED.last_seen_at`,
[hostFingerprint, hostPublicKeyB64, now, now]
)

import { readOrcaPushPayload, type OrcaPushPayload } from './push-payload'

async function resolvePushHostId(payload: OrcaPushPayload): Promise<string | null> {
const hosts = await loadHostCatalog().catch(() => [])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not treat a catalog read failure as host removal.

Line 12 converts a temporary loadHostCatalog() failure into []. The later !hostId branch then suppresses every valid foreground push. Users can miss input-required notifications while local host storage is temporarily unavailable.

Return a distinct load-failure state. Suppress only when the catalog loads successfully and the fingerprint has no match. Add a regression test for a rejected catalog load.

Comment on lines +66 to +69
sources: filter.sources.filter((entry) => isStringMember(entry, MOBILE_PUSH_SOURCES)),
agentStates: filter.agentStates.filter((entry) =>
isStringMember(entry, MOBILE_PUSH_AGENT_STATES)
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject malformed persisted filters.

parseFilter removes unsupported entries and returns the remaining entries. As a result, parseMobilePushRegistration accepts corrupted data instead of returning undefined as its contract states. For example, a filter with one unsupported source and one valid source remains an active registration with a modified filter.

Require both arrays to contain only supported, unique values. Reject an empty filter and unknown filter keys so persisted parsing matches the gateway contract. Add regression tests for invalid, duplicate, and empty persisted filters.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

The new unauthenticated-route rate limiter keys on the first x-forwarded-for hop, which a caller can spoof — so the flood control added in this delta is trivially bypassed.

Reviewed changes

  • Gateway — moved the host key onto the challenge row so an unauthenticated challenge writes no push_hosts row until the proof verifies, added pruneStaleHosts, capped devices per host (64) with an advisory-lock-serialized count, stream-limited every POST body, and added a per-IP token bucket on the challenge/session routes.
  • Desktop — split the handshake into PushGatewaySession (401 re-auth, refusal/rate-limit backoff), fanned the dispatcher out into ≤20-registration requests, and gave the unregister outbox a re-reading drain with capped backoff.
  • Mobile — suppressed foreground pushes whose host is no longer paired, seeded the persisted watermark before epoch adoption, and guarded the register/unregister race with a consent generation.
  • Deploy — builds before the Cloud SQL lease, asserts Terraform-owned scaling instead of writing it, retries the FCM/origin probes, and rolls back (or deletes the candidate) on failure.

The prior review's unbounded-push_hosts finding is resolved: the host row is only written on a verified proof, stale hosts are pruned, and the routes are rate-limited.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • Rate-limit keying — readClientIp now reads the rightmost x-forwarded-for hop (Cloud Run appends the peer after any caller-supplied values), with ORCA_PUSH_TRUSTED_PROXY_HOPS (default 0) for a future fronting load balancer and a fail-closed shared bucket when the header is too short. My prior spoofable-first-hop finding is resolved.
  • Push startup — startDesktopPushService reads state.runtime from shared state and now waits on initialProxyApplicationReady before its first gateway request, keeping the app-owned fetcher behind the persisted proxy.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@Jinwoo-H

Jinwoo-H commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Final security review (4 Opus reviewers + 1 cross-cutting pass), fixed in 5854883 … 724e33d

Reviewed at 92fccee with attacker models: unauthenticated internet, a self-minted host keypair, a malicious paired phone, a compromised gateway/edge, and a Cloud SQL reader.

Fixed

# Finding Fix
1 Authenticated routes had no per-IP cap, and every forged bearer spent one of two pool connections on the session lookup before it could be refused Second per-IP bucket (240/min) runs ahead of bearerSession on every /v1 route; test proves a forged-bearer flood never reaches push_sessions
2 A push with no notificationEpoch could claim a seq-derived dedup key, so ~50 forged seq:N pushes silently swallowed the real terminal bells at those seqs Both the foreground handler and the tray seed now skip a key unless the epoch equals the live one; the desktop always sends an epoch
3 push_hosts rows from any keypair lived 30 d with no cap; push_sessions kept one row per proof Device-less hosts pruned after 1 h (every 30 min); minting a session deletes the host's earlier one
4 Tap on a cold start with a locked keychain fell back to the raw payload's hostId and skipped the known-host allowlist A provider-delivered push stays unrouted when the catalog cannot be read, or when it carries no fingerprint at all
5 registerPush had no throttle: each call is a gateway write plus a synchronous registry write on the main thread Per-device bucket, 10/min, new reason throttled; the earlier registration stands
6 Desktop fetch followed redirects, so a 307 would replay the host proof and the phone token to another origin redirect: 'error' on both call sites, asserted in the client test
7 registerPush params not .strict(), and the paired deviceId was spread first Strict schema, identity spread last, test rejects a caller-supplied deviceId
8 Hono's default error handler printed the whole error (a pg error carries the row in detail) app.onError logs only error.name, answers a bare 500, new request_error counter
9 notificationId charset unvalidated on the way into apns-collapse-id Printable-ASCII regex on the contract
10 Impersonated FCM probe token never ::add-mask::ed Masked right after minting
11 Apple .p8 secrets and orca_push database had no destroy guard prevent_destroy on both

Accepted residuals, now documented in the contract

  • A host can register a token it does not own and push text to it within its own quota. The phone drops it in the foreground and never routes a tap, but a backgrounded OS banner shows. Reaching it needs the victim's native token, which the gateway never returns.
  • Tray dismissal matches notificationId across hosts; ids embed the worktree path, so a cross-host collision is not realistic.
  • Generated DB password sits in Terraform state, readable by the shared deploy identity. Same pattern as the relay.
  • tokenCreator on the runtime SA reaches further than the probe, but grants nothing the deploy identity's run.developer + serviceAccountUser did not already allow.

Verified safe by the reviewers: cross-tenant isolation on delete/list/send, challenge replay, timing-safe proof compare, hashed session tokens, strict wire schemas, streaming body cap, no token/title/body/full-fingerprint in any log path, APNs key never in logs/state/health, transcript validation of all ten fields, gateway origin only from config, mobile payload parser (no prototype pollution, no route escape), WIF pinned to repo/branch/environment/workflow, no ${{ inputs }} in any run: block.

Local verification: gateway 107 tests, contract 19, cloud pretest + full suite green, desktop push/rpc 89, mobile 4146 (full suite), root typecheck, root lint (only the pre-existing host-client-hooks ↔ client-context import-cycle warning, which is on main and untouched here), terraform fmt/validate.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: ab741921-9c34-4af9-8ae6-43caa274ac51

📥 Commits

Reviewing files that changed from the base of the PR and between 92fccee and 724e33d.

📒 Files selected for processing (30)
  • .github/workflows/cloud-push-deploy.yml
  • cloud/apps/push/src/host-session-store.test.ts
  • cloud/apps/push/src/host-session-store.ts
  • cloud/apps/push/src/index.ts
  • cloud/apps/push/src/push-observability.ts
  • cloud/apps/push/src/push-server-harness.test-fixture.ts
  • cloud/apps/push/src/push-server-limits.test.ts
  • cloud/apps/push/src/push-server.ts
  • cloud/dev/scripts/push-gateway-workflow.test.mjs
  • cloud/docs/push-gateway.md
  • cloud/infra/terraform/push-gateway.tf
  • cloud/packages/push-contract/src/contract.test.ts
  • cloud/packages/push-contract/src/push-limits.ts
  • cloud/packages/push-contract/src/send-messages.test.ts
  • cloud/packages/push-contract/src/send-messages.ts
  • docs/reference/mobile-push-contract.md
  • mobile/app/_layout.tsx
  • mobile/src/notifications/push-receive.test.ts
  • mobile/src/notifications/push-receive.ts
  • mobile/src/notifications/push-tray-seen-seed.test.ts
  • mobile/src/notifications/push-tray-seen-seed.ts
  • src/main/runtime/push/desktop-push-service.test.ts
  • src/main/runtime/push/desktop-push-service.ts
  • src/main/runtime/push/push-gateway-client.test.ts
  • src/main/runtime/push/push-gateway-client.ts
  • src/main/runtime/push/push-gateway-response.ts
  • src/main/runtime/push/push-register-throttle.ts
  • src/main/runtime/push/push-registration-rpc.test.ts
  • src/main/runtime/rpc/methods/notifications.ts
  • src/shared/mobile-push-contract.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • cloud/dev/scripts/push-gateway-workflow.test.mjs
  • cloud/docs/push-gateway.md

Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • Gateway session + rate limiting — PushHostSessionStore.create now deletes the host's prior session in a transaction so the table holds one live row per host (revokeForHost removed); host retention dropped to 1 h with a 30 m stale-host prune since nothing reads a host row. A second per-IP token bucket (240/min) now runs ahead of the bearer lookup so forged-bearer floods cannot exhaust the two-connection pool, and an onError handler plus request_error counter log only the error name.
  • Payload and infra hardening — notificationId is constrained to printable ASCII (it becomes the APNs collapse header); Terraform gains prevent_destroy on the push database and Apple signing secrets, and the deploy probe masks its impersonated token.
  • Desktop client — every gateway fetch sets redirect: 'error' so a 307 cannot replay the host proof or phone token; a new PushRegisterThrottle caps registration at 10/min/device; notifications.registerPush uses a .strict() schema with the paired deviceId spread last.
  • Mobile receive — isRemotePushTrigger distinguishes provider-delivered pushes, an unresolvable remote fingerprint now routes to nothing instead of a stray hostId, and epoch-less pushes are shown but never claimed into the seen set.

The prior spoofable-rate-limit-key and unbounded-push_hosts findings remain resolved, and the final security-round closures in this delta are well covered (107 gateway tests and the desktop push suites pass locally).

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

)

A small open-source service that holds the APNs key and FCM credentials and
sends background push to paired phones on the desktop's behalf. Hosts
authenticate with a box challenge and HMAC proof on their pairing key, the
same shape the relay uses, so signed-in and accountless desktops share one
path. Tokens are stored; alert text is held only for the coalescing window.

The contract doc in docs/reference is the source of truth for every wire
shape. The interop test runs the real desktop answerer against a real
gateway-issued challenge so transcript drift fails in CI.
…#8129)

Adds the notifications.remote-push.v1 capability, the registerPush and
unregisterPush RPCs on the mobile allowlist, a gateway client with a cached
session and 401 re-auth, a durable unregister outbox, and a dispatcher that
offers every mobile notification to the gateway after the socket fan-out.
The dispatcher is fire-and-forget with one retry and drops registrations the
gateway reports dead.

Puts agentState on the mobile frame and fixes the #4375 wording so a working
agent is never announced as finished. The relay host-proof code moves onto a
shared envelope module with no behaviour change.
)

Fetches the native APNs or FCM token, registers it with every paired host
that advertises the capability, and re-registers on token change. Foreground
pushes are suppressed inside handleNotification against the same seen set
the socket path uses, so nothing shows twice. Taps route by host fingerprint.
One Background notifications switch, off by default, with the disclaimer and
needs-input / finished sub-switches; hidden until a paired desktop is new
enough. Adds google-services.json and the expo-notifications plugin.
Declares the Cloud Run service, runtime account, secrets, and orca_push
database behind push_gateway_enabled, true only in production. The deploy
workflow is gated like the relay's, deploys with no traffic, probes /ready
and a validate-only FCM send, then shifts traffic. It runs as the shared
production deploy account because the Cloud SQL rollout lease grant is
foundation-owned; its extra authority is three bindings on the push service.
docs/push-gateway.md carries the import commands for the resources created
by hand and the APNs key rotation procedure.
Seven committed files cite it as the source of truth for every wire shape;
docs/reference is allowlisted per file, so add the entry.
)

Cloud Verify installs only the cloud workspace, so the gateway suite cannot
import the desktop answerer. Replace the cross-workspace import with a fixed
challenge vector generated from the contract package; the gateway fixture and
the desktop answerer each replay it and must produce the same HMAC. A
transcript drift on either side now fails in that side's own suite.
…llUsers binding (#8129)

The production domain-restricted-sharing policy rejects an allUsers
run.invoker member, which the runbook anticipated. Opt the service out of
invoker IAM the way the relay director already does; the host proof is the
authentication either way.
- Quota reservation takes a per-host advisory lock; READ COMMITTED admitted
  a whole burst past the cap (80/80 without, 60/80 with, against Postgres 16).
- Challenge issuance no longer writes push_hosts; the row lands on proof
  verification. Stale hosts prune after 30 days. Per-IP token bucket on the
  two unauthenticated routes.
- Streaming body limit via hono bodyLimit; a chunked body bypassed the
  Content-Length check.
- registrationIds deduped in the schema; per-host device cap of 64; list
  bounded to its schema.
- Gateway-side challenge TTL is the specified 10 s, not 40 s.
- APNs stream settles on close as well as end/error.
- A gateway registration the registry cannot persist is enqueued for delete
  instead of leaking a live token.
- Unregister outbox re-reads pending per pass, honours enqueues during a
  drain, and retries with backoff instead of waiting for the next launch.
- Dispatcher batches registrations by 20 rather than starving the rest.
- 401 compare-and-clear; a 401 after re-auth is unreachable; refused
  handshakes and 429s are cached briefly instead of re-handshaking per event.
- Service is stopped on quit.
…8129)

- Consent generation guards a register that finishes after the switch went
  off; the host is re-queued for unregister instead of recorded live.
- Foreground pushes seed the watermark before adopting the epoch, so a push
  on a never-connected session cannot wipe a valid watermark.
- aps-environment follows the build via app.config.js; the iOS release
  workflow sets it to production. A bare plugin entry wrote development.
- Pushes the OS showed while closed are marked seen before catch-up replay.
- Token null result is not cached; failed capability probes are retried and
  never block an unregister; coalesced summaries are shown but not marked.
- Unresolvable fingerprint routes nowhere and is suppressed in foreground.
- Android channel ensured at boot; capability hook diffs clients by identity.
…he budget (#8129)

- Roll traffic back on a failed post-shift check; delete a candidate that
  never took traffic; retry the origin probe and the FCM probe.
- Assert Terraform-owned scaling instead of mutating it from the workflow.
- Build before taking the Cloud SQL rollout lease.
- Declare the database pool in Terraform (2 per instance, max 2 instances)
  and add the gateway to the connection budget; the previous default put the
  shared instance 65 connections over its ceiling.
- State plainly that the shared deploy identity's relay authority is inherited.
Threading the runtime through launchDesktopMode put the launch module one
line over the 300-line lint budget after the rebase.
…ote (#8129)

Cloud Run appends the connecting peer to x-forwarded-for; the limiter read
the left-most value, which the caller controls, so a forged first hop earned
a fresh bucket per request.
…d infra (#8129)

- app.onError logs only the error name and answers a bare 500; hono's default
  handler printed the whole error, and a pg error carries the row in detail
- a second per-IP bucket (240/min) runs ahead of the bearer lookup on every
  authenticated route, so forged bearers cannot spend the two-connection pool
- one live session per host: minting deletes the host's earlier row
- device-less hosts are pruned after 1 h, not 30 d; any keypair mints one free
- notificationId is printable ASCII, since it becomes the APNs collapse header
- the impersonated FCM probe token is masked in the workflow log
- prevent_destroy on the Apple secrets and the orca_push database
…ient (#8129)

- fetch never follows a redirect: a 307 would replay the host proof and the
  phone's token to whatever origin the redirect named
- registerPush params are strict and the paired identity is spread last
- a per-device bucket (10/min) bounds a phone looping registerPush, which
  costs a gateway write and a synchronous registry write each time
…#8129)

- a push with no epoch can no longer claim a seq-derived dedup key, in the
  foreground or from the tray; a forged seq:N could otherwise swallow the
  real bell at that seq
- a provider-delivered push with no host catalog, or no fingerprint at all,
  stays unrouted instead of falling back to the hostId its raw data carries
…meout aborts (#8129)

Ports the relay's #18722 pattern to the gateway: DDL runs on a one-connection
pool with statement_timeout 0 that is closed before the serving pool opens, and
SQLSTATE 57014 joins the bounded transaction retry path.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — one commit (41ace23a) landed since the prior review, porting the relay's #18722 startup pattern to the push gateway.

  • Untimed schema pool — openPushDatabase now applies the Postgres DDL on a one-connection pool (max: 1, statement_timeout: 0) that is closed before the serving pool opens, so a CREATE INDEX on a grown table is no longer killed by the 5 s request deadline and no longer fails every startup at the same statement.
  • Statement-timeout retry — SQLSTATE 57014 (query_canceled from the pool statement_timeout) joins the bounded retry path alongside 40P01/40001/55P03, still capped at three attempts.
  • Tests + docs — a new push-database-postgres-startup.test.ts asserts the pool lifecycle and the retry path; push-gateway.md records that the revision is the schema step on the untimed pool.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — one commit (b71ffe09) landed since the prior review, hardening push delivery and the deploy path.

  • Provider-aware retry — the gateway dispatcher now retries transient APNs/FCM failures up to three times within two minutes, honoring Retry-After (parsed from delta-seconds or HTTP-date via providerRetryAfter). Each attempt re-reads the registration, and dead-token marking is conditional on the token/platform/environment that was actually sent, so a stale dead response can no longer retire a refreshed token.
  • Send idempotency — PushSendQuota.reserve hashes a deterministic send_id from host, registration, epoch, and sequence, so a desktop retry that reached the gateway returns queued without reserving quota twice or enqueueing a second delivery.
  • Graceful shutdown — a request-drain middleware refuses new requests with 503, waits for in-flight handlers, then drains coalescing windows and active deliveries before closing transports and SQL, under a nine-second hard deadline that stays beneath Cloud Run's termination grace.
  • Session uniqueness — a unique push_sessions_host index (with a migration that keeps the newest session when upgrading a DB with duplicate hosts) plus a per-host advisory lock makes one-live-session-per-host a real invariant, including across concurrent mints on PostgreSQL.
  • Deploy recovery — the push deploy workflow derives the revision name deterministically from --revision-suffix, records promotion intent before shifting traffic, rolls back on failure() or cancelled(), and only deletes a rejected candidate when it never took traffic or was verifiably rolled back.
  • Shared schema package — the relay's Postgres schema-retry logic is extracted to @orca-cloud/postgres-schema and reused by the push gateway with an event-prefix parameter; relay behavior is unchanged.
  • Desktop serialization — register, unregister, and outbox deletes are serialized per device via runKeyedSerializedOperation, so a stable gateway id can't inherit a stale delete and a re-register settles earlier cleanup first. Failure categories are counted and logged at most once per minute.
  • Identity limits — notificationId/worktreeId widen to 2048 characters with a 3000-byte UTF-8 budget on the whole notification, rejecting (not truncating) oversized identities.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — a few minor nits below.

Reviewed changes — one commit (11cc732) landed since the prior review, aligning the phone's notification preferences with desktop delivery.

  • Per-phone delivery preferences — a new NotificationDeliveryPreferences store (a followDesktop mirror plus independent task-finished/needs-input/terminal-bell/plugin toggles and phone-local sound/suppressWhileViewing) replaces the two background agent-state sub-switches, and one shared allowsMobileNotification predicate drives both socket/replay and background push.
  • Desktop-side gating — notifications.ts computes desktopAllowed and fans out every event (with desktopAllowed: false + emittedAt) instead of early-returning; notifications.subscribe/getMissedSince accept opt-in includeDesktopSuppressed so legacy phones keep the filtered stream while new phones can choose independently.
  • Sound preference — sound?: boolean flows from the phone filter through registration, dispatch (the dispatcher splits targets into audible/silent groups), the gateway (APNs omits aps.sound; FCM targets orca-desktop-silent), and local scheduling; the silent channel is created at boot.
  • Completion detection — isAgentTaskCompleteTrackingEnabledFromState now stays active when desktop banners and attention are off, so completion still reaches the phone; OS-notification gating moved into main's desktopAllowed.

ℹ️ Nitpicks

  • src/renderer/src/components/terminal-pane/agent-task-complete-policy.ts:35 — the JSDoc still says completion tracking runs "when either consumer (OS notification or the experimental terminal-attention marker) is enabled", but the body now returns state.settings !== null; the comment describes the pre-change behavior.
  • suppressOsNotification is now dead — dispatchTerminalNotification no longer reads it (the OS-notification gate moved to main's desktopAllowed), yet agent-task-complete-notify.ts, parked-terminal-byte-watcher.ts, and agent-hook-completion-notifications.ts still set it.
  • mobile/src/notifications/local-notification-scheduling.ts:17 — NotificationEvent.source is typed DesktopNotificationSource ('agent-task-complete' | 'terminal-bell' | 'test'), which omits 'plugin' even though the desktop dispatches it (runtime-mobile-notification-controller.ts:113) and the new UI exposes a "Plugin notifications" toggle. Runtime is unaffected (the string flows through), but the type is inaccurate.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — one commit (07058a4) landed since the prior review, fixing the APNs device-token length assumption.

  • Variable-length APNs tokens — PushDeviceRegistrationRequestSchema now accepts iOS tokens as any even-length hex byte string (/^(?:[0-9a-fA-F]{2})+$/) instead of exactly 64 hex chars, so real and simulator device tokens that Apple can emit past 32 bytes register correctly. The z.string().min(1).max(4096) bound, the TEXT token column, and the APNs client's verbatim URL-path usage all remain consistent with the wider range; the error message and inline comment were updated to match.
  • Contract test — apns-token-length.test.ts covers acceptance of 32/64/160/256-hex tokens and rejection of empty, odd-length, non-hex, spaced, and over-max(4096) values.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — one commit (fdbb980) landed since the prior review, deduplicating native APNs delivery against the background socket copy.

  • Native payload reader — a new readNativeNotificationData falls back to Expo iOS's trigger.payload when content.data is null (APNs custom fields exist only in the former), so foreground suppression, tap routing, tray-seen seeding, and tray dismissal all read one canonical payload shape; Android and locally scheduled notifications keep content.data.
  • Socket→push handoff — waitForSocketPushHandoff makes socket delivery wait while the app is inactive (iOS can keep the socket alive), then on foreground checks the presented push tray and suppresses the local fallback when APNs/FCM already drew the banner; unsubscribe aborts the wait without claiming delivery, and hosts without a push registration keep local delivery.
  • Replay dedup — deliverMissedEvent short-circuits events already in the seen set and re-checks disposed, and fetchMissed re-checks disposed before its request.

The dedup keys are consistent across the socket's numeric notificationSeq and FCM's stringified value (both normalize via readSeq/template literals), and dismiss events carry their own seq from dispatch(), so the new seen-set guard cannot swallow a dismiss. The new socket-push-delivery-handoff.test.ts and native-notification-data.test.ts cover the handoff, fallback, teardown, and both payload shapes.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@Jinwoo-H
Jinwoo-H merged commit 3160b54 into main Sep 7, 2026
36 checks passed
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
…#8129) (stablyai#18554)

* feat(cloud): add the mobile push gateway and its contract package (stablyai#8129)

A small open-source service that holds the APNs key and FCM credentials and
sends background push to paired phones on the desktop's behalf. Hosts
authenticate with a box challenge and HMAC proof on their pairing key, the
same shape the relay uses, so signed-in and accountless desktops share one
path. Tokens are stored; alert text is held only for the coalescing window.

The contract doc in docs/reference is the source of truth for every wire
shape. The interop test runs the real desktop answerer against a real
gateway-issued challenge so transcript drift fails in CI.

* feat(push): register phones and send background push from the desktop (stablyai#8129)

Adds the notifications.remote-push.v1 capability, the registerPush and
unregisterPush RPCs on the mobile allowlist, a gateway client with a cached
session and 401 re-auth, a durable unregister outbox, and a dispatcher that
offers every mobile notification to the gateway after the socket fan-out.
The dispatcher is fire-and-forget with one retry and drops registrations the
gateway reports dead.

Puts agentState on the mobile frame and fixes the stablyai#4375 wording so a working
agent is never announced as finished. The relay host-proof code moves onto a
shared envelope module with no behaviour change.

* feat(mobile): background push registration, receive, and settings (stablyai#8129)

Fetches the native APNs or FCM token, registers it with every paired host
that advertises the capability, and re-registers on token change. Foreground
pushes are suppressed inside handleNotification against the same seen set
the socket path uses, so nothing shows twice. Taps route by host fingerprint.
One Background notifications switch, off by default, with the disclaimer and
needs-input / finished sub-switches; hidden until a paired desktop is new
enough. Adds google-services.json and the expo-notifications plugin.

* chore(cloud): Terraform and deploy workflow for the push gateway (stablyai#8129)

Declares the Cloud Run service, runtime account, secrets, and orca_push
database behind push_gateway_enabled, true only in production. The deploy
workflow is gated like the relay's, deploys with no traffic, probes /ready
and a validate-only FCM send, then shifts traffic. It runs as the shared
production deploy account because the Cloud SQL rollout lease grant is
foundation-owned; its extra authority is three bindings on the push service.
docs/push-gateway.md carries the import commands for the resources created
by hand and the APNs key rotation procedure.

* docs: describe background notifications on the phone (stablyai#8129)

* docs: check in the mobile push contract (stablyai#8129)

Seven committed files cite it as the source of truth for every wire shape;
docs/reference is allowlisted per file, so add the entry.

* test(push): replay one checked-in host-proof vector on both sides (stablyai#8129)

Cloud Verify installs only the cloud workspace, so the gateway suite cannot
import the desktop answerer. Replace the cross-workspace import with a fixed
challenge vector generated from the contract package; the gateway fixture and
the desktop answerer each replay it and must produce the same HMAC. A
transcript drift on either side now fails in that side's own suite.

* fix(cloud): open the push gateway with invoker_iam_disabled, not an allUsers binding (stablyai#8129)

The production domain-restricted-sharing policy rejects an allUsers
run.invoker member, which the runbook anticipated. Opt the service out of
invoker IAM the way the relay director already does; the host proof is the
authentication either way.

* docs(cloud): the push.onorca.dev record exists and is hand-managed (stablyai#8129)

* fix(push): close review findings in the gateway (stablyai#8129)

- Quota reservation takes a per-host advisory lock; READ COMMITTED admitted
  a whole burst past the cap (80/80 without, 60/80 with, against Postgres 16).
- Challenge issuance no longer writes push_hosts; the row lands on proof
  verification. Stale hosts prune after 30 days. Per-IP token bucket on the
  two unauthenticated routes.
- Streaming body limit via hono bodyLimit; a chunked body bypassed the
  Content-Length check.
- registrationIds deduped in the schema; per-host device cap of 64; list
  bounded to its schema.
- Gateway-side challenge TTL is the specified 10 s, not 40 s.
- APNs stream settles on close as well as end/error.

* fix(push): close review findings in the desktop client (stablyai#8129)

- A gateway registration the registry cannot persist is enqueued for delete
  instead of leaking a live token.
- Unregister outbox re-reads pending per pass, honours enqueues during a
  drain, and retries with backoff instead of waiting for the next launch.
- Dispatcher batches registrations by 20 rather than starving the rest.
- 401 compare-and-clear; a 401 after re-auth is unreachable; refused
  handshakes and 429s are cached briefly instead of re-handshaking per event.
- Service is stopped on quit.

* fix(mobile): close review findings in push registration and receive (stablyai#8129)

- Consent generation guards a register that finishes after the switch went
  off; the host is re-queued for unregister instead of recorded live.
- Foreground pushes seed the watermark before adopting the epoch, so a push
  on a never-connected session cannot wipe a valid watermark.
- aps-environment follows the build via app.config.js; the iOS release
  workflow sets it to production. A bare plugin entry wrote development.
- Pushes the OS showed while closed are marked seen before catch-up replay.
- Token null result is not cached; failed capability probes are retried and
  never block an unregister; coalesced summaries are shown but not marked.
- Unresolvable fingerprint routes nowhere and is suppressed in foreground.
- Android channel ensured at boot; capability hook diffs clients by identity.

* fix(cloud): harden the push deploy workflow and size the gateway to the budget (stablyai#8129)

- Roll traffic back on a failed post-shift check; delete a candidate that
  never took traffic; retry the origin probe and the FCM probe.
- Assert Terraform-owned scaling instead of mutating it from the workflow.
- Build before taking the Cloud SQL rollout lease.
- Declare the database pool in Terraform (2 per instance, max 2 instances)
  and add the gateway to the connection budget; the previous default put the
  shared instance 65 connections over its ceiling.
- State plainly that the shared deploy identity's relay authority is inherited.

* fix(push): read the runtime from shared state at push startup (stablyai#8129)

Threading the runtime through launchDesktopMode put the launch module one
line over the 300-line lint budget after the rebase.

* fix(push): key the unauthenticated rate limit on the hop Cloud Run wrote (stablyai#8129)

Cloud Run appends the connecting peer to x-forwarded-for; the limiter read
the left-most value, which the caller controls, so a forged first hop earned
a fresh bucket per request.

* fix(push): close the final security review findings in the gateway and infra (stablyai#8129)

- app.onError logs only the error name and answers a bare 500; hono's default
  handler printed the whole error, and a pg error carries the row in detail
- a second per-IP bucket (240/min) runs ahead of the bearer lookup on every
  authenticated route, so forged bearers cannot spend the two-connection pool
- one live session per host: minting deletes the host's earlier row
- device-less hosts are pruned after 1 h, not 30 d; any keypair mints one free
- notificationId is printable ASCII, since it becomes the APNs collapse header
- the impersonated FCM probe token is masked in the workflow log
- prevent_destroy on the Apple secrets and the orca_push database

* fix(push): close the final security review findings in the desktop client (stablyai#8129)

- fetch never follows a redirect: a 307 would replay the host proof and the
  phone's token to whatever origin the redirect named
- registerPush params are strict and the paired identity is spread last
- a per-device bucket (10/min) bounds a phone looping registerPush, which
  costs a gateway write and a synchronous registry write each time

* fix(mobile): close the final security review findings in push receive (stablyai#8129)

- a push with no epoch can no longer claim a seq-derived dedup key, in the
  foreground or from the tray; a forged seq:N could otherwise swallow the
  real bell at that seq
- a provider-delivered push with no host catalog, or no fingerprint at all,
  stays unrouted instead of falling back to the hostId its raw data carries

* docs(push): record the ip buckets, session and host retention, and the token-ownership limit (stablyai#8129)

* fix(push): apply the schema on an untimed pool and retry statement-timeout aborts (stablyai#8129)

Ports the relay's stablyai#18722 pattern to the gateway: DDL runs on a one-connection
pool with statement_timeout 0 that is closed before the serving pool opens, and
SQLSTATE 57014 joins the bounded transaction retry path.

* fix: harden mobile push delivery and deployment recovery

* feat: align mobile notification preferences with desktop delivery

* fix: accept variable-length APNs device tokens

* fix: deduplicate native APNs and background socket notifications
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.

[Feature]: Real push notifications for mobile — events are lost while the app is backgrounded/disconnected

1 participant