feat(mobile): add the contextual identity-name resolver - #7894
Conversation
🔐 Codex Security Review
|
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested: two P2 provider defects. The pure resolver is faithful to the pinned v1 contract; the blockers are at the channel-provider boundary, detailed inline. Merge criteria: preserve the current-scope roster during membership reloads, consume the bot roles already present in that roster, and add provider regressions including the relay/account reset fence. Screen wiring remains out of scope.
Reviewed head bee2de9f71e40d917329f50dd212122fe3eae454 against base/merge-base 930b8bb800d8149ce29a881ba4c5d9f424580434. All three independent review lanes are integrated. Verified the 31 fixtures are byte-identical to buzz-app 92c2fb2; the policy lane’s preserved differential harness/logs report 145,003 matching valid-input cases. Exact-head mobile CI passed analyze, 2,484 tests (4 skipped), and Android debug build. I independently reproduced both provider failures with production providers and a mocked transport; the cross-relay reset test passes on the current head. No device/UI claim: this PR has no screen consumers yet. No screenshot/generated-artifact additions in the diff.
18c3b78 to
381d592
Compare
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: REQUEST CHANGES
Reviewed: b65cff31a4c5f4a0af63952b60a21fd73195321a..381d5926d0a44bfd1872210265622bc8bff00a91 (exact head 381d5926d0a44bfd1872210265622bc8bff00a91)
Risk: medium — reusable identity-label policy and relay/account-scoped provider foundation; no screen consumer in this PR.
Blocking finding
mobile/lib/shared/identity_names/identity_name_policy.dart:115-161—resolveIdentityNamesdocuments that an invalid public key throwsArgumentError, but validation happens only in the lazynpubForpath used when a collision needs a suffix. A unique malformed key, e.g.NamingIdentity(pubkey: 'not-a-key', name: 'Unique'), is accepted and returned as a resolved identity. The current invalid-key test collides onHoney, so it exercises only the suffix path and cannot detect this case (mobile/test/shared/identity_names/identity_name_policy_test.dart:55-66). This leaves the public foundational contract false and permits trusted-looking labels for non-identities when future callers use the resolver directly.
Author action: validate each supplied identity key before alias construction and add a non-colliding malformed-key regression bound to resolveIdentityNames; prove the test fails when eager validation is removed. Clarify/validate viewer, candidate, and owner keys consistently with the intended public contract.
Verification owner: author for fix and causal regression; reviewer for exact-new-head delta and mobile gates.
Behavior/contracts traced: resolver precedence and suffix growth; owner lookup facts; channel roster reload snapshots; offline bot classification; relay/account/profile-cache isolation; #7895/#7896 stack boundaries. The current reload and cached-bot fixes are sound, and no additional concrete defect was found.
Validation: git diff --check passed; just mobile-check passed (593 files formatted, 0 changed; flutter analyze clean). Exact-head GitHub Mobile/Clients, DCO, Semgrep, and zizmor checks are green. Local just mobile-test could not start because this reviewer host's Xcode license is unaccepted (xcrun --show-sdk-path exits 69); that is a tooling confidence gap, not additional author action.
Manual/native evidence: none; this foundation intentionally has no screen consumer.
Residual risk: full local mobile suite was not independently rerun; exact-head CI owns that evidence. Any new head expires this review.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: REQUEST CHANGES
Reviewed: b65cff31a4c5f4a0af63952b60a21fd73195321a..381d5926d0a44bfd1872210265622bc8bff00a91 (exact head 381d5926d0a44bfd1872210265622bc8bff00a91)
Risk: high — this is reusable identity-labeling policy plus relay/account/channel-scoped provider state; a malformed-key acceptance path can turn a non-identity into a trusted-looking identity label.
Blocking finding
[P2] Validate every identity key before resolution, not only after a collision.
resolveIdentityNames documents that it throws ArgumentError for an invalid public key (mobile/lib/shared/identity_names/identity_name_policy.dart:109-119), and NamingIdentity requires 64-character hex keys (:8-22). But validation lives only in npubFor (:154-161), which is called only when a collision needs a key suffix (:163-211). A single non-colliding fact such as NamingIdentity(pubkey: 'not-a-key', name: 'Unique') therefore returns a result keyed by not-a-key instead of rejecting it.
The current regression does not protect the advertised contract: it pairs the malformed key with a valid identity using the same Honey label (mobile/test/shared/identity_names/identity_name_policy_test.dart:55-66), forcing the suffix path where validation happens. The pinned portable contract says identity keys MUST be valid 32-byte/64-hex keys and invalid keys must be handled before resolution; silently returning a label fabricates an identity rather than handling it.
Consequence: this foundational public API accepts malformed input on the common no-collision path and emits a plausible contextual label for a value that is not a Nostr identity. Current adapters filter their inputs, limiting immediate screen exposure, but that does not make the reusable resolver's explicit contract true.
Author action: eagerly validate every supplied identity key before constructing aliases, and add a non-colliding malformed-key regression. Clarify and consistently enforce the intended treatment of viewer, candidate, and owner keys. Mutation-prove the load-bearing regression by bypassing eager validation and confirming it fails.
Verification owner: author for the fix/test; reviewer for exact-head source review and the full mobile gate.
Integrated contract review
The provider fixes from the prior round are sound at this head:
- Same-scope roster reloads read the active relay/account/channel-keyed snapshot, while relay/account switches cannot borrow the old roster (
mobile/lib/features/channels/channel_management_provider.dart:76-116,519-605; production-path regressions inmobile/test/features/channels/channel_identity_names_provider_test.dart:59-124). - Offline bot classification unions
member.isBotfrom the roster being resolved with relay-backed hints (mobile/lib/features/channels/channel_identity_names_provider.dart:28-40), including the disconnected cached-roster regression. - Profile/cache ownership is generation-fenced by relay configuration, and channel snapshots are account-keyed. Resolver precedence and owner qualification otherwise match the pinned policy.
- Stack scope is clean: this PR adds the standalone resolver/provider foundation and no screen consumer; UI/accessibility proof belongs to #7895/#7896.
Validation
At exact clean head 381d5926d0a44bfd1872210265622bc8bff00a91:
- live PR base/head matched, PR was mergeable, and
git diff --check b65cff31..HEADpassed; - exact-head
Clients / Mobile, aggregate Clients,Mobile, Mobile Swift aggregate, Semgrep, zizmor, and DCO checks were successful; - vendored portable fixture bytes matched
block/buzz-app@92c2fb2(SHA-2565b26b9818f8d092792bfdfcc8c84b05f29ee00807459a06a3002c6003a44c762); - local
just mobile-install mobile-check mobile-test: install and format/analyze passed (No issues found); the full test lane could not start because this reviewer host'sxcrun --show-sdk-pathexits 69 until the Xcode license is accepted.
Confidence gaps
- The local full mobile suite was not independently completed due to reviewer-host Xcode licensing. That is tooling, not additional author rework; exact-head mobile CI is green.
- No simulator/UI/accessibility observation was performed because this PR has no screen consumer. The stacked screen-wiring reviews own those journeys.
The malformed-key contract violation is source-proven and author-actionable, so it remains blocking despite green CI.
Add a Dart resolver for contextual identity names v1, the portable contract in block/buzz-app (src/bundled/identity-naming, pinned at 92c2fb2). It gives distinct public keys distinct labels: humans before agents, the viewer's identities before others, owner names before public-key suffixes. Vendor the unmodified portable fixtures and run all 31 cases against the production resolver. Name trimming uses the contract's explicit code-point set, because Dart String.trim also strips U+0085. Add providers that compare a channel's names against its members (plus any referenced non-member) and load the profiles of displayed identities. No screen uses them yet. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
A live membership update reloads the channel roster. While it reloaded, the channel's names fell back to an older or empty roster, so two identities named "Honey" briefly showed the same label. Keep the last roster loaded for the same relay, account and channel instead. Another relay or account never sees that roster. Also treat members with the roster's bot role as agents. Offline, the relay-backed bot lookup and agent directory are empty, so a cached bot with no owner was ranked as a human. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
The resolver checked a key only when a collision needed its npub suffix, so a malformed key with a unique name got a normal label. Every fact key, owner key, viewer and candidate is now checked first. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
381d592 to
0b59f57
Compare
|
Response to Jude's review, finding [P2] Validate every identity key before resolution: Fixed in 0b59f57. New regressions: a malformed key with a unique name and no collision, a colliding malformed key, a malformed owner key, a malformed viewer, a malformed candidate, and a 64-character value that is not hex. An upper-case hex key is still accepted. Mutation check: when I skip the early fact check, the unique-name, colliding, owner and non-hex cases fail. The app adapter ( |
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: ebe99a46e8802b9ff20fdf6a1028ce93bdefaa43..0b59f5757e71a9199234856a8675e334c4d4a8fc (exact head 0b59f5757e71a9199234856a8675e334c4d4a8fc)
Risk: medium — reusable identity-label policy and relay/account/channel-scoped provider foundation; no screen consumer in this PR.
Findings: no unresolved author-actionable defect.
The previous malformed-key blocker is closed. resolveIdentityNames now validates every fact key and owner key, the viewer, and every candidate before alias construction (mobile/lib/shared/identity_names/identity_name_policy.dart:89-95,125-145). Uppercase hex is accepted and canonicalized only after validation. The new unique-name malformed-key regression directly exercises the former no-collision escape path; additional cases cover collision, owner, viewer, candidate, 64-character non-hex input, and uppercase-valid input (mobile/test/shared/identity_names/identity_name_policy_test.dart:55-127).
The full delta preserves the intended boundaries:
- identity labels remain presentation-only and exact pubkeys retain authority;
- profile/cache state is relay-scoped and generation-fenced;
- channel roster snapshots remain keyed by relay URL + account + channel, including same-scope reload and relay/account switch regressions;
- offline bot classification retains roster bot facts;
- precedence and owner qualification still match the pinned portable contract;
- this PR has no screen consumer, so native presentation/accessibility verification belongs to #7895/#7896.
Author action: none.
Verification owner: CI for the still-running Mobile Swift gate; downstream reviewers for wired UI journeys.
Validation at exact clean head:
- live PR base/head matched; PR was mergeable;
git diff --check ebe99a4...HEADpassed; just mobile-installandjust mobile-checkpassed (593 files formatted, 0 changed;flutter analyzereported no issues);- exact-head
Clients / Mobile, aggregate Clients,Mobile, Semgrep, zizmor, and DCO passed; - the vendored 31-case fixture remained byte-identical to pinned
block/buzz-app@92c2fb2, SHA-2565b26b9818f8d092792bfdfcc8c84b05f29ee00807459a06a3002c6003a44c762.
Confidence gaps: local just mobile-test could not start the Flutter test lane because the reviewer host's unaccepted Xcode license breaks native-asset setup; exact-head hosted Mobile passed. Mobile Swift remained in progress at review time and is not causally related to this pure-Dart fix; merge readiness remains with that external gate. No simulator/UI observation was performed because this foundation renders nothing.
Any new head expires this approval.
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Reviewed: ebe99a46e8802b9ff20fdf6a1028ce93bdefaa43..0b59f5757e71a9199234856a8675e334c4d4a8fc (exact head 0b59f5757e71a9199234856a8675e334c4d4a8fc)
Risk: medium — shared identity-name resolution and tenant-scoped profile/member inputs.
Behavior/contracts traced: eager key validation, resolver precedence/fallback, relay/account/channel cache fencing, verified owner hints, and stacked-PR boundaries. The prior malformed-key blocker is resolved: every fact/owner/viewer/candidate key is validated before alias construction, and the new non-colliding malformed-key regression directly covers the missed path.
Findings: no unresolved author-actionable defect. The foundation remains presentation-only and has no screen consumer in this PR.
Author action: none.
Verification owner: CI/reviewer tooling for residual local/native gaps.
Validation: exact-head just mobile-install and just mobile-check passed; git diff --check passed; hosted Clients / Mobile, aggregate Mobile, Semgrep, zizmor, and DCO passed. Local just mobile-test reached native-asset setup but did not execute tests because the reviewer host's Xcode license blocks xcrun; hosted Mobile CI supplies the exact-head full test/build evidence.
Manual/native evidence: none; this foundation has no UI consumer.
Residual risk: no independent local mutation run or simulator observation. These are reviewer/tooling confidence gaps, not author rework.
— :bot: Jude’s code review agent
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Follow-up verdict: code is ready for approval. Reviewed base ebe99a46e8802b9ff20fdf6a1028ce93bdefaa43 → head 0b59f5757e71a9199234856a8675e334c4d4a8fc.
Both blockers from my earlier review are addressed: same-scope reloads reuse the existing relay/account/channel-keyed roster snapshot, and offline naming includes the roster’s bot roles. The added provider regressions exercise the real membership/naming providers, including relay/account resets. I also checked the eager fact/owner/viewer/candidate key validation and its regression cases; no remaining blocker found in this follow-up.
Exact-head hosted Mobile passed formatting, analysis, tests and Android debug build. Mobile Swift now also passed its tests and complete simulator/release builds. This follow-up used source inspection and hosted evidence, not a fresh local suite or device run; the PR still has no screen consumer.
Approval is not being submitted by this comment. My earlier changes-requested review remains active until superseded or dismissed. Separately, the Codex security review still requires a run for this exact range; its authorization workflow succeeding is not a completed review. Codex is not listed among the required status checks returned by the current main-branch rules.
…in-ui * origin/main: feat(buzz-relay): NIP-FI stateless enforcement (S3) — upgrade gate, NIP-42 pairing, session lifetime, JWKS warm (#7224) feat(web): add Browse releases link next to invite download (#2255) docs(nips): fix stray angle brackets in created_at clauses (#4486) docs: specify desktop-driven mobile push suppression (#7809) feat(mobile): add the contextual identity-name resolver (#7894) Automate owner deletion preparation (#7830) feat(relay): implement NIP-AR channel artifacts (#7919) fix(desktop): resolve unlisted project channel requests (#7619) Schedule the deletion drain safely (#7827) fix(mobile): stale community selection during mobile invite setup (#7951) Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz>
🤖
## Summary
In a channel conversation, two identities with the same display name
looked identical. In the example below, the channel shows "Bad Janet"
twice, for two different identities. A reader cannot tell which one was
added and then removed.
This PR uses the contextual names from the previous PR everywhere a
conversation names someone:
- Message authors, threads, forum posts and replies
- Reactions, the typing indicator, and system rows (joined, added,
removed)
- Inline mentions and the agent activity sheet
- The profile sheet title, which now matches the row that opened it
Names are compared against the channel's members, so a label only gets
longer when a real collision exists in that channel.
The mention picker labels each choice against every candidate, so two
"Honey" choices look different. When you pick one, it still inserts the
identity's own name ("@honey"), so the message text does not change.
Part 2 of 3: #7894 (resolver) → #7895 (conversations) → #7896 (lists,
Search, Pulse).
### Related issue
Related to #2910.
### Testing
- A widget test checks that the profile sheet names an author the same
way the channel does.
- A unit test checks that two same-name mention choices get different
labels but insert the same wire name.
- `flutter analyze` is clean, and the full `flutter test` suite passes.
- Checked on an Android emulator against the live relay, in an archived
channel with two identities named "Bad Janet".
| Before | After |
| --- | --- |
| <img width="300" alt="Before: the channel shows two different
identities both as Bad Janet"
src="https://github.com/user-attachments/assets/c7f6c345-12d8-4e11-9434-bbd2c63722db"
/> | <img width="300" alt="After: the second identity shows as Bad Janet
· sqxl"
src="https://github.com/user-attachments/assets/51c6b9fc-8900-4200-8ce0-ad1887f41dba"
/> |
---------
Signed-off-by: Logan Johnson <loganj@squareup.com>
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
🤖 ## Summary This PR finishes the change on the remaining mobile screens. After it, every place that names a person or agent uses a contextual name: - The channel list, DM labels, and the new-DM picker - The channel header and details, and the members and add-members sheets - Huddle avatars, spotlight, and overlay - The Activity inbox - Search results for people and messages - Pulse notes, agent cards, and reply previews Screens that belong to a channel compare names against its members. Search, Pulse, and the new-DM picker have no channel, so they compare the identities shown together. In the example below, Search showed four results all named "murderbot". Now the human keeps the plain name, and baxen's three agents are labeled by owner and key suffix. Part 3 of 3: #7894 (resolver) → #7895 (conversations) → #7896 (lists, Search, Pulse). ### Related issue Related to #2910. The desktop search change in #6483 addresses the same problem in a different app. ### Testing - Widget tests cover the Activity inbox, Search, and Pulse notes with colliding names. - `flutter analyze` is clean, and the full `flutter test` suite passes. - Checked on an Android emulator against the live relay by searching for "murderbot". The after image also shows "classy-murderbot", a new account that was created between the two captures. | Before | After | | --- | --- | | <img width="300" alt="Before: Search shows four people all named murderbot" src="https://github.com/user-attachments/assets/21bbcc52-fea8-4b31-ab8c-ff3ec9b32a39" /> | <img width="300" alt="After: three results show as baxen's murderbot with a key suffix, and the human shows as murderbot" src="https://github.com/user-attachments/assets/d59584d9-4a35-46da-b39a-83addfad3cd2" /> | --------- Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
🤖
Summary
When two identities share a display name, Buzz mobile shows the same label for both. A reader cannot tell a person from an agent, or one person's agent from another's. For example, a channel can show three agents and one human all named "murderbot".
This PR adds the naming rules that fix this, without changing any screen yet. It ports the contextual identity-name contract (v1) from the desktop app to Dart. Given the identities shown together, the resolver gives each public key a distinct label:
It also adds two providers that screens will use:
channelIdentityNamesProvidercompares names against a channel's members, plus any referenced non-member such as an old author or an outside mention.watchIdentityNamescompares a set of identities shown together outside a channel, and loads their profiles.The next PRs in this stack connect these to the screens.
Part 1 of 3: #7894 (resolver) → #7895 (conversations) → #7896 (lists, Search, Pulse).
Related issue
Related to #2910 (default "Fizz" agents from different users collide in one channel).
Testing
flutter analyzeis clean.No UI change, so there are no screenshots.