Repository navigation
fix(ios): fetch profiles and avatars in notification extension - #8369
Conversation
Signed-off-by: Tom Brow <[email protected]>
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Signed-off-by: Tom Brow <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e5e1b38df
ℹ️ 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".
Signed-off-by: Tom Brow <[email protected]>
Signed-off-by: Tom Brow <[email protected]>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Allow valid inline profiles through the response cap
Increase this bounded cap to include the JSON event envelope and all requested records: a kind-0 event whose inline avatar is near the separately accepted 256 KiB metadata limit necessarily serializes above 256 KiB, so the size check discards the entire refresh—including the sender name and channel data—and the notification remains unenriched. Size the bound from the maximum accepted profile plus channel/membership envelopes, or fetch them separately, while retaining a hard limit.
AGENTS.md reference: AGENTS.md:L240-L248
ℹ️ 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".
Signed-off-by: Tom Brow <[email protected]>
Signed-off-by: Tom Brow <[email protected]>
|
@codex review |
|
🤖 Device validation on iPhone SE. Before first app launch after upgradeThe extension fetched the sender name, avatar, channel, and message before the updated main app was opened.
|
brow
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ed6c009c0
ℹ️ 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".
Signed-off-by: Tom Brow <[email protected]>
|
🤖 Addressed the response-size finding in 861f443. The regression reproduced loss of a valid near-limit profile's name under the old cap. The response now has a hard 2 MiB bound, with coverage for a near-limit profile plus large channel and membership records, including JSON escaping. Per-record validation remains in place; responses over the new limit still fail safely. |
Signed-off-by: Tom Brow <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c9c3fe748
ℹ️ 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".
Signed-off-by: Tom Brow <[email protected]>
Signed-off-by: Tom Brow <[email protected]>
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
Signed-off-by: Tom Brow <[email protected]>
github-actions
Bot
removed
the
codex-security-review-current
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d40bb6a65
ℹ️ 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".
Signed-off-by: Tom Brow <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c897da09c6
ℹ️ 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".
Signed-off-by: Tom Brow <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a479739569
ℹ️ 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".
Signed-off-by: Tom Brow <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64783321db
ℹ️ 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".
Signed-off-by: Tom Brow <[email protected]>
github-actions
Bot
added
the
codex-security-review-current
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: 5397ea49e27fd56d4e0346e2120906d09ac91579..d4945d1f73698e362d91a88ed0de545953c3e062
Risk: critical — notification-extension account/key ownership, authenticated network reads, cross-process persistence, revocation, and user-visible stale/partial presentation
Behavior/contracts traced: main-app snapshot/keychain ownership → account/community/session scope; independent profile/channel/membership downloads; NIP-98 nonce and canonical origin/IPv6 authority; bounded responses and raster decoding; locked atomic cache replacement, event ordering, partial-refresh counterpart invalidation, avatar races/removal, revocation, one-shot deadline and late cache warming; package/entitlement and presentation fallback paths.
Findings: no unresolved author-actionable defect. A successful one-record channel refresh removes its cached counterpart atomically (mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzNotificationCache.swift:120-180), while a genuinely older replaceable event cannot invalidate or partially replace the current pair. Identical event ID/timestamp is accepted and does clear the counterpart; at equal timestamps, a lexicographically larger ID is correctly stale under the repository’s established replaceable-event ordering (BuzzPushPresentationCache.swift:637-646). Regression coverage exercises both partial directions and stale-batch preservation (BuzzNotificationCacheTests.swift:142-187). Independent bounded profile/channel/membership work preserves sender/avatar enrichment when a large or slow roster fails, and scope checks fence removed or changed account keys before cache use and final presentation.
Author action: none.
Verification owner: CI/release gate for Mobile Swift Native; reviewer/device tooling for any additional exact-head removal-during-flight, VoiceOver, and physical-device lifecycle spot-check.
Validation: at clean exact HEAD d4945d1f73698e362d91a88ed0de545953c3e062, git diff --check 5397ea49..HEAD passed. Local swift test --package-path mobile/ios/BuzzPushKit could not start because this host has not accepted the Xcode license (exit 69); this is reviewer-tooling debt, not a code failure. Hosted Mobile Checks, test shards 1/3–3/3, Android, Mobile aggregate, Swift Release, Codex Security Review, Semgrep, zizmor, and DCO were green at the final pre-submit check.
Manual/native evidence: the author reports signed-device notification enrichment and cache reuse at this head; I did not independently reproduce the physical-device journey.
Residual risk: Mobile Swift Native remained in progress at pre-submit. Any new head or PR-caused required-gate failure invalidates this approval.
A notification can arrive from someone whose profile the main app has never loaded. The iOS notification extension now fetches and caches sender profiles, channel details, and avatars itself. The main app supplies account configuration and Keychain credentials only.
After resolving the message, the extension allows 1.5 seconds for sender and channel details. It uses available stale data when a refresh is slow or fails, delivers once, and lets late results update its private cache. Profiles, channel details, and avatar refresh checks share a one-hour freshness threshold. A verified avatar removal clears the previous image. Account removal prevents further use of that sign-in's cached data.
Profile, channel metadata, and membership use separate bounded downloads, so an oversized roster cannot discard the smaller records. Sender enrichment can finish while channel requests are pending. A successful partial channel refresh atomically removes its old counterpart, preventing a mixed group identity or recipient count. Group-specific notification presentation still requires a verified recipient count.
The specification describes the display, caching, and account-removal behavior. General persistent image caching in Flutter remains separate.
Validation
Hosted CI passed on the final commit, including all Flutter shards, Android, the full iOS release and simulator builds, and native controls/push-navigation tests. The current-commit security review found no issues.
The native package passed 159 tests, including new coverage for cache freshness, avatar replacement and removal, sign-in isolation, concurrent storage, late results, single delivery, bounded downloads, and SVG validation. A regression test exposed identical HTTP authentication event IDs for requests in the same second; each request now includes a unique nonce, matching the Dart and CLI clients.
Full local CI passed before that small authentication fix. The complete native suite and signed device build passed again on the latest commit, including response-size limits, overlapping avatar downloads, canonical relay-origin matching, independent profile/channel refreshes, account-key changes before and during requests, and a single initial snapshot read across 64 accounts, signed IPv6 avatar authorities, and partial channel refreshes. Focused Flutter regression tests passed 198 tests with three configuration-specific skips; seven avatar widget tests also passed. Flutter analysis is clean, and the notification extension builds for the simulator.
On an iPhone SE, the notification displayed the sender name, avatar, channel, and message before the updated main app was opened. Repeat notifications used a byte-identical private cache. Three notifications delivered together displayed the avatar after the authentication fix. The next notification also displayed correctly after terminating the extension process. The existing generic-push design can select the same newest unread message for several pushes; this change does not alter message selection.
Authenticated HTTP avatar fetching and persistent HTTP cache reuse also passed on the SE. The first notification downloaded a relay thumbnail. After terminating the extension and expiring its separate avatar entry, the next notification displayed the avatar and refreshed that entry without another media request in the relay logs. The main app remained unopened throughout. The original app build and test profile were restored afterward.
Automated tests cover slow and failed requests, stale images, removal, and account revocation; those cases have not all been exercised on the phone.
Before merge