Skip to content

feat: installable PWA with push notifications for new pull request diffs - #131

Merged
flexdinesh merged 2 commits into
mainfrom
pwa-push
Oct 28, 2026
Merged

flexdinesh merged 2 commits into
mainfrom
pwa-push

Conversation

@flexdinesh

Copy link
Copy Markdown
Owner
  • Behavior owner and resulting change:
    • githubsync owns the trigger rule:
      • A completed refresh notifies unmuted followers when the captured head SHA differs from the last recorded head.
      • A PR's first snapshot notifies only when a pull_request opened/reopened/ready_for_review webhook requested the refresh, so first follow, startup reconcile and Resync import silently.
      • Metadata edits, close and merge never notify.
      • Notification intents commit in the same CompleteRefresh transaction as each follower's ingestion job.
    • New pushservice (remote only) owns:
      • Subscriptions: upsert by endpoint, rebind across accounts.
      • VAPID keys: created once, persisted, never rotated.
      • The outbox delivery worker:
        • Waits for the linked ingestion job, drops the notification if that job failed, then resolves the stream.
        • Sends per device with idempotent per-device records.
        • 404/410 removes the device; retries honor Retry-After with exponential backoff capped at 1h; gives up after 24h.
    • New webpush wire adapter wraps webpush-go.
    • Web:
      • PWA manifest, "dx" monogram icons, and an import-free typed service worker (/sw.js). The worker always shows the notification; on click it focuses a window already on that stream, otherwise opens a new one, so it never navigates an open review.
      • Settings › Notifications with a state per case: disabled, unsupported, iOS install steps, install, default, granted, denied.
      • Per-repo bells in Followed repositories.
      • Sign-out removes this device.
  • Contracts/invariants affected; local/remote implications:
    • OpenAPI:
      • GET /api/v2/push.
      • PUT /api/v2/push/subscriptions.
      • DELETE /api/v2/push/subscriptions/{id}.
      • POST /api/v2/push/subscriptions/{id}/test.
      • PATCH /api/v2/github/follows/{id} {notify}.
      • New required field GitHubRepository.notify.
    • All routes need a browser session and CSRF; API tokens are refused.
    • Schema 15 adds tables only and upgrades in place:
      • push_keys, push_subscriptions, push_notifications, push_deliveries.
      • github_pr_heads, github_follow_mutes.
    • Push is enabled for an https --public-url, or a loopback http public URL in development. Plain-http endpoints are accepted only to loopback hosts and only when the public URL is itself loopback.
    • Local composition has no push routes, manifest or service worker, because local ports change and installs would be orphaned.
  • Validation results; remaining risks:
    • Checks:
      • mise run check passes.
      • go test -race passes on webpush, pushservice, githubsync, remoteserver and reviewstore.
    • End-to-end test: signed opened webhook → fetcher → ingestion → fake push endpoint, which decrypts exactly one RFC 8291 payload.
    • Real diffx-server binary serves:
      • manifest.webmanifest as application/manifest+json.
      • /sw.js as text/javascript with no-cache.
      • /api/v2/push as 401 without a session.
    • New tests:
      • DOM tests for every Settings state.
      • Playwright: remote-only PWA, enable flow, bells, sign-out, and service-worker push and click.
    • Risks:
      • No live GitHub or real push-service testing (FCM/Mozilla/Apple, installed iOS 16.4+); tracked in docs/github-live-testing.md item 10.
      • The VAPID private key is stored unsealed in SQLite (0600).
      • No per-account device cap.
      • A PR merged with a changed head while the server was down notifies "Updated by".
      • A send cut off by a crash may duplicate to a device that never recorded acceptance.
      • The service-worker Playwright spec needs full Chromium.
  • Intentional architecture/design changes and rationale:
    • New modules webpush and pushservice, classified in check-boundaries.
    • The outbox follows the external-delivery rule: intent is recorded atomically with admission.
    • Docs updated:
      • docs/system.md, docs/architecture.md, docs/development.md.
      • DESIGN.md: settings section, bell, 44px touch toggles, app icons.
      • PRODUCT.md.
      • docs-website: GitHub page, api schema 15.
  • UI evidence:
    • Notifications section and bells checked at desktop and 390px, light and dark.
    • Impeccable critique/audit findings fixed:
      • Device removal confirm.
      • Bell hint.
      • Bells hidden when push is disabled.
      • Spacing.
    • Bells are aria-pressed with the name "Notify for owner/repo".
    • Touch toggles reach 44px.
    • No Pierre geometry changes.

Remote server only. Followed GitHub pull requests notify subscribed devices
when a webhook-announced PR opens or its head changes. Intents commit with
CompleteRefresh; a delivery worker sends after ingestion succeeds.

- webpush: VAPID wire adapter (webpush-go)
- pushservice: subscriptions, keys, outbox delivery worker
- githubsync: head-change trigger rule, per-repo mutes
- reviewstore: schema 15 additive push and PR head tables
- web: manifest, icons, service worker, Settings Notifications, repo bells
@flexdinesh
flexdinesh merged commit cff414f into main Oct 28, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant