Skip to content

feat(notifications): add local sender avatars across surfaces - #42627

Merged
Jasonnnz merged 29 commits into
mainfrom
Jasonnnz/notification-avatar-local-v2
Sep 14, 2026
Merged

Jasonnnz merged 29 commits into
mainfrom
Jasonnnz/notification-avatar-local-v2

Conversation

@Jasonnnz

Copy link
Copy Markdown
Contributor

Summary

  • Add independent, default-off push-avatar-sender and local-notification-avatar client flags.
  • Carry exact scoped assistant identity through notification intents, preparation, IPC, permission confirmation, and tap routing.
  • Add an iOS app-local native notification owner while keeping APNs Notification Service Extension delivery separate.
  • Unify Android app-local and data-only FCM delivery under one process-wide coordinator with page-bound ownership negotiation, shared focus suppression, full-key deduplication, and one owned sound.
  • Add prepared browser notification icons, macOS Communication Notification sender presentation, Windows helper sender parity, and Linux Electron icon preservation.
  • Document architecture, compatibility boundaries, manual QA, and rollout gates in docs/notification-avatar-local-qa.md.

Platform dependency

Depends on vellum-assistant-platform#10502 at 8694633c77b4183a1e0f24442b30fc41b6345bfa.

That platform PR is open, merge-clean, and green, but it is not merged or applied. Before this application PR merges, both flag definitions must exist in the target platform configuration, the configuration must be applied, and both flags must be verified OFF.

Validation

  • Exact 28-file changed TypeScript manifest: 695 passed, 0 failed, one Bun process per file. Electron native-auth passed 7/7 with loopback access.
  • Six focused Wave 8 web suites: 155 passed, 0 failed.
  • Notification tap navigation: 15 passed, 0 failed.
  • iOS host-less AppTests: passed on an iPhone 17 simulator with CODE_SIGNING_ALLOWED=NO.
  • Scoped changed-web ESLint over an asserted 41-file manifest: passed.
  • Typechecks: web, assistant fast typecheck, ipc-contract, electron-desktop, macOS, Windows, and Linux passed.
  • Focused Android Gradle tests: blocked because the validation host has no Java runtime.

Manual QA and rollout status

This PR is draft and is not rollout-ready. No manual route is marked PASS.

Blocked evidence includes signed physical-device iOS app-local and APNs/NSE runs, Android device and Firebase runs, signed packaged macOS native delivery, packaged Windows parity, and a defined browser/PWA support matrix. macOS fallback and permission confirmation are not run. Linux has one non-gating regression smoke that is not run.

The canonical case matrix, evidence format, prerequisites, current statuses, and rollout gates are in docs/notification-avatar-local-qa.md. No release, feature-flag activation, or platform apply is included.

Review focus

  • Android coordinator ownership and FCM/SSE arrival-order invariants.
  • iOS one-owner behavior across native rewrite timeouts and ambiguous bridge completion.
  • Exact identity isolation across scope changes, pop-outs, logout, rename, and late preparation.
  • Backward compatibility for old web/new shell, new web/old shell, notification-block FCM, plain fallbacks, acknowledgments, actions, and tap metadata.

Vellum Assistant added 26 commits September 11, 2026 14:53
@Jasonnnz

Copy link
Copy Markdown
Contributor Author

Review attention: risk is concentrated in the Android process-wide ownership and deduplication state machine, the iOS one-owner timeout behavior, and scoped identity invalidation across renderer and account transitions. Please review those paths before presentation details. The exact changed-TypeScript manifest is green, but focused Android Gradle execution is still blocked by the missing Java runtime, and all signed device/package QA remains blocked or not run as recorded in docs/notification-avatar-local-qa.md. The companion platform flag PR #10502 is green but not merged or applied; both flags must be applied and verified off before this PR can merge.

@Jasonnnz

Copy link
Copy Markdown
Contributor Author

@codex review the current head f887e1f

@Jasonnnz

Copy link
Copy Markdown
Contributor Author

@devin review the current head f887e1f

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-12T02:29:07.386846Z aa9f720 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: f887e1fbaa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Jasonnnz
Jasonnnz marked this pull request as ready for review September 12, 2026 01:03

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f887e1fbaa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread clients/web/src/runtime/notification-avatar.ts
@Jasonnnz

Copy link
Copy Markdown
Contributor Author

@codex review
@devin review

Please re-review current head e1406ea. The renderer reload generation finding is addressed with eager session registration and source-keyed native generation translation across Electron, iOS, and Android. Focused web, IPC, and Electron tests pass; 72 iOS logic tests pass; and the full iOS simulator build succeeds. Android local execution remains unavailable on this host because no Java runtime is installed, so the PR Android checks remain the execution gate.

@Jasonnnz

Jasonnnz commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

@codex review
@devin review

Please review current head aa9f720. This follow-up removes the sandbox-unsafe node:crypto import exposed by the Linux, macOS, and Windows preload guards, using the Web Crypto global already available to the preload. All three preload-external tests, the notification preload tests, and the shared Electron type-check pass locally.

@Jasonnnz

Copy link
Copy Markdown
Contributor Author

@codex review current head aa9f720

@Jasonnnz

Copy link
Copy Markdown
Contributor Author

@devin review current head aa9f720

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: aa9f720fa6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Jasonnnz
Jasonnnz merged commit 7a5af32 into main Sep 14, 2026
36 checks passed
@Jasonnnz
Jasonnnz deleted the Jasonnnz/notification-avatar-local-v2 branch September 14, 2026 15:42
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