fix(desktop): npub identity displays for mention, member, and workflow surfaces - #7495
Conversation
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Review verdict: request changes
Reviewed base 5f3a4a8111998c8aa41ad77cf66992bd1c85343c → exact live head a1ffa774b291557c57860baecc05f2220b6dc8ae as :bot: Jude’s code review agent.
[P2] Relay-only thread facepiles still expose raw hex for unnamed participants
desktop/src/features/messages/lib/threadPanel.ts:411 falls back from a missing profile name directly to pubkey. That value is passed as displayName to UserAvatar by desktop/src/features/messages/ui/MessageThreadSummaryRow.tsx:42-50, so a cold/relay thread summary exposes the raw 64-character hex identity through its avatar label/fallback instead of the compact npub promised by this PR.
This is not hypothetical: passing a valid deadbeef × 8 participant through buildMainTimelineEntries at this head produced:
{"actual":"deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef","expected":"npub1m6k…zuz0"}The existing relay-summary test currently preserves the defect with the invalid fixture/assertion { id: "bob", author: "bob" } at desktop/src/features/messages/lib/threadPanel.test.mjs:617-638. This conflicts with the PR summary’s explicit “same compact npub everywhere” message-identity contract and with the local/live message formatting path.
Author action: use truncateNpub(pubkey) for the unnamed relay-summary participant fallback. Replace/add a regression using a valid 64-hex key and assert the compact npub; ideally bind the avatar-visible/accessible label too. Confirm the test fails if the fallback is mutated back to raw pubkey.
Verification owner: author for the patch and falsifiable regression; reviewer will inspect the exact-head delta and rerun affected/full desktop gates.
Integrated validation
At exact head:
just desktop-typecheck/cd desktop && pnpm typecheck— pass.just desktop-check/cd desktop && pnpm check— exit 0; reported advisories are in unrelated unchanged paths, and px-text/pubkey-truncation guards pass.just desktop-buildandpnpm build:e2e— pass.- Full desktop unit package — pass, 6,464/6,464. One concurrent cold run showed an unrelated order-sensitive
useKnownAgentPubkeysfailure; an immediate full-suite retry passed. threadPanel.test.mjs— pass, 23/23, while demonstrating that the current test preserves the raw fallback.- Browser smoke run reached case 666 with displayed cases passing, including mention clipboard cases 629–645 and ambiguity/exact-key/keyboard/reopen cases 646–666; the 1,405-case run exceeded the 20-minute harness cap before completion.
- Live GitHub checks are terminal green at the reviewed head, including desktop core/smoke/integration, macOS/Windows builds, Semgrep, zizmor, and DCO.
- Authenticated reviewer
jedwards27is distinct from PR authorloganj.
The complementary user-trust review found no additional concrete defect across compact-npub validity/fail-closed behavior, mention clipboard rebinding and ambiguity, workflow destinations, member/community/profile/moderation/huddle/activity fallbacks, and avatar labels. That does not neutralize the reproducible thread-summary defect above. Green CI is evidence; apparently raw hex also enjoys passing grades.
Residual risk
No fresh native Tauri lifecycle run or screenshot/semantic receipt was completed. The long browser smoke run did not reach its later workflow/member cases before timeout. These are confidence gaps, not additional author-actionable defects; no renderer↔IPC/Tauri persistence behavior changed in this frontend-only diff.
jedwards27 review of PR #7495 (exact head a1ffa77, 22:10:51Z) flagged [P2] that buildRelayThreadSummary fell back from a missing profile name to the raw pubkey, so a cold/relay-only thread facepile exposed the raw 64-char hex identity via UserAvatar's displayName label while the client-assembled path (resolveUserLabel) already renders the compact npub. Fall back to truncateNpub(pubkey) instead — the same canonical compact identity form used everywhere else in this PR. Authored profile names are untouched; distinct key-tail avatar initials come for free because getInitials derives them from the compact npub shape. Replace the review-flagged invalid relay-summary fixture ("bob") with a valid 64-hex key and assert the compact npub label — the value MessageThreadSummaryRow binds to UserAvatar's visible/accessible displayName. Verified falsifiable: reverting the fallback to raw pubkey fails the test. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
|
🤖 Addressing review 5147520343 at published head 8cce3c0 (
The review is not dismissed; a re-review request will follow once the exact-head CI is green. Source: threadPanel.ts fallback · regression |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Review verdict: approve
Reviewed base 5f3a4a8111998c8aa41ad77cf66992bd1c85343c → exact live head 8cce3c0bfdd74c53fcaf5cee89a8634132aa91d3 as :bot: Jude’s code review agent.
The prior relay-only thread-summary defect is repaired. desktop/src/features/messages/lib/threadPanel.ts:417-418 now applies truncateNpub(pubkey) to unnamed relay participants, and desktop/src/features/messages/lib/threadPanel.test.mjs:608-647 drives the production timeline seam with a valid 64-hex key and asserts the compact npub.
Both independent review lanes found no remaining author-actionable defect. Each mutated the production fallback back to raw pubkey; the focused thread-panel suite then failed on the exact raw-hex-versus-compact-npub mismatch, and passed 23/23 after restoration. The regression therefore bites the repaired seam rather than merely decorating it.
Exact-head validation
- Full Desktop unit package: 6,464/6,464 passed.
- Desktop typecheck, check, build, and
git diff --check: passed. - Focused browser journeys: 8/8 passed, covering chat/forum/edit ambiguity and draft preservation, alias repair, member virtualization, and workflow keyboard select/save/reopen.
- Windows and macOS builds, both relay-backed integration shards/aggregate, DCO, Semgrep, and zizmor: passed at this head.
- Smoke shards 1, 2, and 4 passed. Shard 3 reported one terminal failure in unchanged
navigation.spec.ts:448plus four retry-pass flakes in unchanged messaging/onboarding/overscroll tests (287 passed, 1 skipped). The PR's changed-head delta is only the thread fallback and its unit regression, and the independently run focused affected journeys passed; I found no evidence attributing that failure to this PR. Desktop Core remained in progress at submission. These are reviewer/CI confidence gaps, not author rework.
Author action: none.
Verification owner: CI/reviewer for the outstanding Desktop Core result and any rerun/triage of the unrelated smoke shard. Any new head invalidates this approval.
Residual risk
No fresh native Tauri screenshot/semantic receipt, exhaustive 1,405-case local smoke run, or real relay-backed UI journey was completed. This two-file frontend remediation changes no renderer↔IPC/Tauri/persistence contract.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Changed-head re-review: code clear; CI verification remains
Reviewed base 5f3a4a8111998c8aa41ad77cf66992bd1c85343c → exact live head 8cce3c0bfdd74c53fcaf5cee89a8634132aa91d3 as :bot: Jude’s code review agent.
Prior P2: resolved
The changed-head delta is limited to desktop/src/features/messages/lib/threadPanel.ts and its test. buildRelayThreadSummary now sends unnamed relay participants through truncateNpub(pubkey) at threadPanel.ts:417-418. The regression at threadPanel.test.mjs:608-647 uses a valid 64-hex key and asserts npub1m6k…zuz0 on the production summary object consumed by MessageThreadSummaryRow/UserAvatar while retaining the named-profile path.
Two independent mutation checks replaced only ?? truncateNpub(pubkey) with ?? pubkey: the focused suite changed from 23/23 pass to 22 pass / 1 fail, with the expected raw-hex-versus-compact-npub mismatch. Restoring the candidate returned 23/23 pass and a clean exact-head tree. The regression bites the exact prior defect rather than merely admiring it from across the room.
No additional material defect was found across relay summary flow, mention identity/ambiguity and clipboard binding, member/profile/community/huddle/activity labels, workflow identity and keyboard round trips, full-key metadata, malformed-key fail-closed behavior, or repo policy.
Author action: none for reviewed code.
Exact-head validation
All reported local gates were bracketed by HEAD=8cce3c0bfdd74c53fcaf5cee89a8634132aa91d3 and an empty tracked diff:
just desktop-test— 6,464/6,464 pass.just desktop-typecheck— pass.just desktop-check— exit 0; px-text and pubkey-truncation guards pass. Reported advisories are non-failing.just desktop-build— pass.- Focused Playwright identity/ambiguity/member/workflow journeys — 8/8 pass.
git diff --check 5f3a4a8...HEAD— pass; merge-base matches the supplied base.- macOS/Windows builds, relay-backed integration shards/aggregate, DCO, Semgrep, and zizmor — green at this head.
CI confidence gap — no author rework assigned
The now-terminal GitHub run has one failed smoke shard: Desktop Smoke E2E (3) reported 287 passed, 1 failed, 4 flaky, 1 skipped. The sole persistent failure was tests/e2e/navigation.spec.ts:448, “composer Buzz chip labels wrap without orphaning their icons,” after all retries; four unrelated interaction/timing cases passed on retry. The failed head delta changes only relay thread-summary formatting and its unit regression, not the navigation/composer implementation or test. The same broader PR state had green smoke CI at prior head a1ffa774…, and focused changed-surface browser evidence is green, so this is currently a required-gate confidence gap rather than a demonstrated PR-caused defect.
Author action: none. Do not modify the npub remediation to appease an unbound smoke failure.
Verification owner: CI/reviewer should rerun the failed smoke shard (or otherwise obtain a green required gate) and re-check live-head freshness before merge. If the navigation failure reproduces consistently and is causally tied to this PR’s broader identity-display changes, that new evidence requires a separate author-actionable finding.
Residual risk
No fresh native Tauri launch, screenshot/semantic receipt, exhaustive local 1,405-case smoke run, or real relay-backed UI journey was completed. This two-file changed-head delta is frontend-only and does not alter IPC/native/persistence contracts. These remain bounded confidence gaps, not code defects.
jedwards27 review of PR #7495 (exact head a1ffa77, 22:10:51Z) flagged [P2] that buildRelayThreadSummary fell back from a missing profile name to the raw pubkey, so a cold/relay-only thread facepile exposed the raw 64-char hex identity via UserAvatar's displayName label while the client-assembled path (resolveUserLabel) already renders the compact npub. Fall back to truncateNpub(pubkey) instead — the same canonical compact identity form used everywhere else in this PR. Authored profile names are untouched; distinct key-tail avatar initials come for free because getInitials derives them from the compact npub shape. Replace the review-flagged invalid relay-summary fixture ("bob") with a valid 64-hex key and assert the compact npub label — the value MessageThreadSummaryRow binds to UserAvatar's visible/accessible displayName. Verified falsifiable: reverting the fallback to raw pubkey fails the test. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
8cce3c0 to
30cac57
Compare
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: b3310c24832b29d8ee90ea76a7878ac01be13ea3..30cac57f71a374d1176667c5c7efcf4c76070e05 (exact head 30cac57f71a374d1176667c5c7efcf4c76070e05)
Risk: medium — broad Desktop identity presentation with bounded changed-head relay-summary delta.
Behavior/contracts traced: identity pubkeys remain full identifiers for relay summary merge/deduplication while presentation fallbacks use canonical compact npub; mention copy binding, member/community/profile/huddle/activity/workflow surfaces, malformed-input fail-closed behavior, and accessibility metadata.
Findings: no unresolved author-actionable defect. Both assigned review lanes are code-clear at this exact head; older findings were re-derived rather than carried forward.
Author action: none.
Verification owner: CI/release gate for remaining exact-head nonterminal checks; reviewer/tooling for optional native observation. A later PR-caused required-gate failure or any new head requires fresh review, but neither is current author rework.
Validation: Clean exact-head lanes passed git diff check, Desktop check/typecheck/build, full Desktop unit suites (6,465+ tests), a mutation-causal thread-panel production-seam regression, and 70 focused Playwright cases before reviewer-environment cleanup interrupted the wider run. Windows/macOS builds, relay integration, DCO, Semgrep, and zizmor were green at final refresh; Desktop Core and Smoke shard 4 remained in progress with no failure.
Manual/native evidence: no fresh installed-native artifact was produced in this re-review.
Residual risk: The interrupted wider Playwright run and lack of native Tauri observation leave bounded visual/platform confidence gaps; automated exact-head required checks own completion.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — exact-head re-review
Verdict: COMMENT (code-clear; one required CI gate still running)
Reviewed base b3310c24832b29d8ee90ea76a7878ac01be13ea3 through exact live head 30cac57f71a374d1176667c5c7efcf4c76070e05. No concrete defect was found across the systems/integration and product/UI/adversarial review.
The changed-head delta fixes the remaining relay-summary presentation path without changing identity semantics: participant IDs remain full pubkeys for merge/deduplication, while the display fallback now uses the canonical validated npub formatter. The relay-only regression test exercises the production handoff and was mutation-proved: reverting the fallback to raw hex made the intended test fail.
The full review also covered mention rendering/copy rebinding, member/channel/community/profile identity surfaces, avatars, huddles/transcription, activity, moderation/projects, and workflow controls. Compact legacy mention text is accepted only when bound to the declared recipient key; mismatched or malformed identity inputs remain rejected or fail closed. Exact identity remains available in accessibility/title/data metadata where those surfaces expose it. No persistence, network, auth, subscription, lifecycle, accessibility, keyboard, or draft-preservation regression was identified.
Exact-head evidence
just desktop-test: 6,465/6,465 passedjust desktop-typecheck: passedjust desktop-check: passed (non-blocking existing Biome advisories only)just desktop-build: passed, including the protected-artifact matrixpnpm build:e2e: passedgit diff --check b3310c24832b29d8ee90ea76a7878ac01be13ea3...30cac57f71a374d1176667c5c7efcf4c76070e05: passed- Changed-head relay-summary mutation check: passed by failing on the raw-hex reversion and passing after restoration
- Fresh GitHub check at
30cac57f71a374d1176667c5c7efcf4c76070e05: all completed checks passed or were intentionally skipped; all four Desktop Smoke E2E shards, Windows/macOS builds, relay-backed integration shards, DCO, Semgrep, and zizmor were green
Confidence gaps
Desktop Domain / Desktop Coreremains in progress. Author action: none unless it reports a PR-caused failure. Verification owner: merge owner / CI.- A focused browser run reached 70 passing tests before reviewer-worktree cleanup terminated workers; 182 tests did not run, so it is not claimed as a complete pass. No fresh native Tauri visual artifact was produced. These are reviewer-verification gaps, not evidence of an author defect. Author action: none. Verification owner: reviewer or release owner if additional native/browser evidence is required.
No author rework requested. Approval is appropriate once the remaining required exact-head CI gate is terminal green. Any new head invalidates this review.
…gate, strict parser) (#7488) 🤖 ## Summary Identity keys in the desktop app are displayed as raw 64-character hex. A person's key shows up as something like `953d3363…` — unreadable, impossible to recognize as the same identity on another screen, and a hazard when copied by hand. Nostr (the protocol Buzz runs on) has a human-readable spelling for identity keys — the `npub1…` form — but the desktop app did not use it consistently. This is the foundation of the desktop npub changes: it adds the shared pieces every identity surface builds on, and two follow-up slices stack directly on this branch — #7489 converts the identity controls (profile, settings, allowlist, workflow key fields) and #7495 converts the everyday display surfaces (mentions, member lists, sidebar, and other name fallbacks). After this change: - The shared identity widget shows the compact npub form — `npub1j57...fjmv` — instead of a hex prefix, everywhere it renders (for example the owned-agent public-key row on a profile). Copying it puts the full npub on the clipboard. - Copy is a real interaction, verified end-to-end: both popover variants put the exact canonical npub on the actual clipboard — never the raw hex the popover also lists, never a truncation — and a portaled popover's clicks no longer steal focus from the new-DM To-field mid-copy. Pointer copy, a natural Space-then-Enter path, and inner/outer Escape are covered. - Anything that isn't a valid identity key fails neutrally: short or corrupt values — including degenerate values that technically encode to a checksum-valid npub but aren't real identity keys — show "Unavailable" with no copy button, instead of a misleading value. - Both valid npub spellings display: all-lowercase `npub1…` and all-uppercase `NPUB1…` (Bech32, npub's encoding, permits either casing) both render the same canonical lowercase npub. Mixed case is rejected by the display path as written — `canonicalNpub` and the widget don't case-normalize input — while input parsing (`parsePubkeyInput`) keeps its trim-and-lowercase normalization and accepts mixed-case npubs; both paths require the decoded payload to be exactly a 64-character identity key. - Identity-key input is strict on payload: an npub whose decoded payload isn't exactly a 64-character identity key is rejected, matching the validation the app's Rust side already applies to agent allowlists. Intentional scope boundary: only surfaces that render through the shared widget change here. Outer profile copy, settings identity cards, the respond-to allowlist, and workflow key fields still show hex — they move to npub in the controls follow-up (#7489). Nothing else changes identity representation: display names, private keys, event IDs, and the hex the app stores, sends, and matches internally are untouched; only the user-facing spelling of an identity key changes. ## Details - `desktop/src/shared/lib/pubkey.ts` — `canonicalNpub()`: strict canonical full-npub helper (64-char hex in any case, or a checksum-validated npub, returns the canonical npub; anything else returns `null`); `truncateNpub()`: the compact display form; existing exports unchanged. - `desktop/src/shared/ui/PubKey.tsx` — the shared widget's identity gate validates through `canonicalNpub`; the popover copies the npub only. - `desktop/src/shared/lib/nostrUtils.ts` — `parsePubkeyInput` rejects npubs whose payload is not exactly a 64-character identity key. - `desktop/src/features/messages/ui/NewMessageScreen.tsx` — the To-field focuses its search input only for clicks that land inside the field itself, so portaled recipient popovers keep their focus while open (a popover click previously dismissed it mid-copy). - Unit suites cover the helper, widget, and parser (including the degenerate-encode and uppercase regressions); the e2e specs that render these rows assert the npub display. ### Related issue - Fixes: N/A. Searched existing issues/PRs for npub identity display — no existing match. - Stack: #7489 is based on this branch and builds on these primitives; it does not stand alone on main. ### Testing At head `b3310c248` (base: main `44316ff72`; 12 files, +440/−39): - Focused unit suites (pubkey, PubKey, parsePubkeyInput): 20/20 green; mutation-checked — removing the decoded-length predicate fails the short/empty checksum-valid-npub assertions in `canonicalNpub` and the widget, and a wrong-identity clipboard value fails the new copy assertions. - `pnpm typecheck` and `pnpm check`: pass; full desktop unit suite 6459/6459 at this exact head. - Targeted e2e at this exact head: 8/8 across the two specs that own the clipboard flows — `agent-access-warning.spec.ts` (compact variant, agent-access owner hint) and `pubkey-display-screenshots.spec.ts` (full variant, new-DM recipient verification: pointer copy, popover surviving the copy, inner/outer Escape, Space-then-Enter). - No Rust-side or build files change in this PR, so those results are unaffected. ### Task provenance Buzz channel: `1f0e4a3d-7e01-4efe-bb16-843b357f85c9` Task: buzz://message?channel=1f0e4a3d-7e01-4efe-bb16-843b357f85c9&id=86b34eb4bd84a1472419e9af22636c011c0fe273e3c196f967d7a36996e149b6 --------- Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
PR-D2 of the npub identity display standardization (stacks on PR-D1's truncateNpub/PubKey foundation). ## Summary - Mention identity: mentionDisplay/markdown render compact npubs (truncateNpub); copied-mention and clipboard qualification keeps byte-exact identity roundtrips for both npub and legacy hex-truncated chips, rejecting wrong/missing/tampered key qualification. - Member/participant surfaces (channels, community members, huddle, projects, pulse, search, sidebar, tray, notifications): identity fallbacks render compact npubs; titles/aria stay machine-readable full labels. - Profile labels: resolveUserLabel and formatOwnerLabel fall back to compact npub (never raw hex); profile panel/popover identity rows and copy actions follow. - Workflows: author picker secondary labels, step destination keys, and trigger author references render compact npubs while event/blob IDs keep truncatePubkey; roles/templates pass through. - Avatar initials: key-form labels derive initials from the key tail (validated full npub or exact compact npub shape over the bech32 alphabet) so key-fallback avatars stay distinct; authored names that merely resemble npubs keep name-path initials. - Unit tests across all touched surfaces pin npub compacts, invalid-key behavior, and legacy-hex chip roundtrips; e2e specs assert npub compacts for author options, mention chips, hover identity, and profile owner rows. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
… labels
Astra P2 follow-up on PR-D2. Generated role-prefixed fallback labels
("Participant npub1…", "Agent npub1…") are ordinary words to getInitials,
so every unnamed participant/agent avatar collapsed onto the same "PN"/"AN"
initials — losing the distinct key-tail initials raw hex keys used to
produce.
- ProfileAvatar/UserAvatar: optional initialsLabel (defaults to label /
displayName) drives fallback initials only; alt text, visible label, and
the displayName color hash are unchanged.
- ParticipantList and ChannelActivityPopover working-agent rows keep the
visible "Agent/Participant npub1…" display but pass the unprefixed
compact key as initialsLabel, so key-fallback avatars render distinct
key-tail initials; authored names keep name initials.
- WorkingAgentRows exported for consumer tests.
- Consumer regressions: ParticipantList.test.mjs mounts the real
HuddleParticipantsControl (CommunitiesProvider + relay-agents query
wiring); ChannelActivityPopover.test.mjs mounts WorkingAgentRows with a
real root and the production fallback-delay path. Both pin distinct
key-tail initials (RC/04, not PN/AN) and authored name initials (AE/HR).
Validation at final state (rebased onto D1a 2c68ddd): pnpm check,
pnpm typecheck, full desktop unit suite (6476 pass, incl. these 4),
focused mock-bridge e2e — workflow author picker (2), pubkey-display
key-row/widget-scope (4), mentions profile panel/popover (2),
mention-recipient qualified chips (8) all pass; workflow-local-controls:149
snapshot remains the documented preexisting 438px darwin drift
(pinned-baseline-equivalent, not regenerated).
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
Tests-only cleanup for PR-D2 per the proportionality audit. No production change: every edit lands in unit suites and the two existing e2e specs, so the slice's build, typecheck, and unit evidence still binds. ## Summary - Delete the dedicated consumer drivers added with the initials fix (ParticipantList.test.mjs and ChannelActivityPopover.test.mjs, 381 lines): their minimal real-avatar assertions move into the harnesses that already mount the real UI — the huddle roster and the channel-activity mock-UI e2e specs now pin named authored controls (initials AL/C) and unnamed key-fallback rows (key-tail initials X2/SQ/RC/04, never the collapsed PN/AN). - Slim the npub unit matrices in mentionClipboard, timelineMentionCopy, identity, workflowStepDescription, workflowTriggerDescription, initials, and mentionDisplay suites, preserving the load-bearing cases: clipboard byte-exact identity roundtrips for new and legacy chips (wrong/missing/tampered qualification rejected), initials lookalike gating and checksum-valid key tails, authored-name paths, and workflow author/eventID boundaries. Validation at final state (tests-only edits; predecessor evidence reused, no suite escalation): 9 changed/related unit files 100/100; typecheck, e2e build, biome, and px text/truncation checks clean; huddle roster focused run green; channel-activity full e2e 11/11 green. Wiring mutation checks that failed huddle (X2 collapsing to PN) and activity (RC missing/AN) were restored and the rebuild was green. Known limitation, unchanged by this cleanup: full huddle-transcription e2e remains 25 pass / 1 fail on distinct agent voices at the voice menu, reproduced on pristine HEAD via stash run (preexisting, outside this cleanup); workflow-local-controls:149 keeps the documented preexisting 438px darwin caret drift (separate baseline, not regenerated). The last edit was a biome line-join in the huddle spec only (formatting-only, no rerun for it). Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
Tests-only fix for PR-D2 per Astra review of the revised candidate (approved 4b2a046b; the original 4d9add22 candidate was rejected and not applied). No production change: both edits land in the existing members-sidebar virtualization spec in channels.spec.ts. ## Summary - First-window endpoint: generated members have no display name, so the roster sorts them by their npub fallback label; sequential pubkeys with a shared `npub1qqq...` prefix order by checksum tail, not numeric position. Resolving the first generated row from the pubkeys the initial window actually rendered replaces the unstable `pubkeys[0]` assumption. - Tail endpoint: the fully-scrolled tail check no longer samples `pubkeys.at(-1)` (subject to window luck). The sidebar's own "Members - N" header must equal the fixture-known roster total (3 seeded + 500 generated), and the final row is located by VirtualizedList's stamped `data-index` on the last item, a fixed target a virtualizer clamped mid-roster can never render. Validation at this exact delta (tests-only, evidence reused from the approved candidate; no full suite run, no re-review): focused virtualization spec 1/1 green, members-sidebar shard 12/12 green, biome and e2e build/typecheck clean. A midpoint negative control (skipping to the middle of the roster) fails the new tail endpoint. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
jedwards27 review of PR #7495 (exact head a1ffa77, 22:10:51Z) flagged [P2] that buildRelayThreadSummary fell back from a missing profile name to the raw pubkey, so a cold/relay-only thread facepile exposed the raw 64-char hex identity via UserAvatar's displayName label while the client-assembled path (resolveUserLabel) already renders the compact npub. Fall back to truncateNpub(pubkey) instead — the same canonical compact identity form used everywhere else in this PR. Authored profile names are untouched; distinct key-tail avatar initials come for free because getInitials derives them from the compact npub shape. Replace the review-flagged invalid relay-summary fixture ("bob") with a valid 64-hex key and assert the compact npub label — the value MessageThreadSummaryRow binds to UserAvatar's visible/accessible displayName. Verified falsifiable: reverting the fallback to raw pubkey fails the test. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
30cac57 to
bd05f63
Compare
🔐 Codex Security Review
|
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — APPROVE
Reviewed: bfc384855889432df4a333a0edf3080f332ee169..bd05f631fc1ecd10307ee85234db56a474f7206f (exact head bd05f631fc1ecd10307ee85234db56a474f7206f)
Risk: medium — user-visible participant identity fallback in the thread summary.
Behavior/contracts traced: relay participant full-key ownership/deduplication, named and unnamed labels, validated canonical npub fallback, avatar consumption, ordering/counts, local-summary override, invalid-input recovery, and exact production regression seam.
Findings: no blocking code/product defect. The changed-head delta is limited to threadPanel.ts and threadPanel.test.mjs: relay participants retain full pubkeys as identity keys while unnamed presentation uses validated truncateNpub; invalid input fails closed to Unavailable, not raw key text. The regression drives buildMainTimelineEntries and mutation back to raw pubkey fails the intended assertion.
Author action: none.
Verification owner: CI for terminal Desktop Core and failed Smoke E2E shard 4 triage/retry; reviewer/release owner for optional native visual observation. The current CI state is an external confidence/gate item, not an established author-actionable defect, so it does not convert sound code into REQUEST CHANGES.
Validation
At clean exact head:
just desktop-test— PASS, 6,471/6,471.just desktop-typecheck,just desktop-check,just desktop-build— PASS.- Focused production-seam test — PASS, 23/23; mutation to raw pubkey failed exactly the unnamed relay participant assertion, restoration passed and tree returned clean.
git diff --check— PASS.- Relevant CI builds, integration, relay E2E, DCO, Semgrep, and zizmor were green; Desktop Core remained pending and Smoke E2E shard 4 reported failure while the overall run was still active and logs unavailable.
- Live head remained exact and mergeable before submission.
Manual/native evidence: no fresh native visual journey. This delta changes deterministic display conversion only; no IPC, persistence, network, async lifecycle, or native code changed.
Residual risk: the terminal Smoke E2E failure cause is not yet available. CI owns merge readiness and must distinguish flaky/infrastructure behavior from a PR-caused required-gate failure.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — changed-head re-review
Verdict: REQUEST CHANGES at exact head bd05f631fc1ecd10307ee85234db56a474f7206f against base bfc384855889432df4a333a0edf3080f332ee169.
Blocking finding
- Required Desktop Smoke E2E shard 4 is red on a PR-touched workflow regression. The exact-head run failed
desktop/tests/e2e/workflow-local-controls.spec.ts:268(round-trips and reopens structured message-text conditions) on all three attempts. Attempts 1 and 3 timed out waiting for the edit-workflow trigger control at helper line 56; retry 1 reached the layout assertion at line 298 but found the second operator left of the first (1091.219versus1188.740) instead of to its right. This PR changes that spec and the workflow trigger/step description surfaces, so this is a PR-caused required-gate failure until shown otherwise—not merely missing reviewer evidence. The same shard labeled six other tests flaky, but only this test exhausted all retries and made the required check fail.- Author action: make the changed workflow behavior/test deterministic and restore the required exact-head shard to green; if the result is infrastructure-only, provide a green exact-head rerun demonstrating that.
- Verification owner: author for the fix/rerun; reviewer will re-check the resulting exact head and terminal CI.
- Evidence: https://github.com/block/buzz/actions/runs/34365934552/job/102514840928
Code review and local evidence
No additional concrete defect was found across the systems/integration and product/UI/adversarial review. Relay participants retain full pubkeys as identity keys for merge/deduplication while only unnamed presentation falls back through validated truncateNpub; malformed input fails closed. Mention binding/legacy compatibility, roster/profile/search/new-DM, avatar, huddle/activity, workflow, keyboard, and accessibility paths were reviewed without another blocker.
At the pinned clean head:
just desktop-test: 6,471/6,471 passedjust desktop-typecheck: passedjust desktop-check: passed (existing non-fatal Biome advisories only)just desktop-build: passed, including protected-artifact checksgit diff --check: passed- Focused relay-summary regression: 23/23 passed; mutation back to raw pubkey failed the intended assertion (22 pass / 1 fail) and restoration returned the tree clean
Fresh GitHub state remained mergeable at the same base/head, with 23 successful checks and no other terminal failure; Desktop Core was still running at final refresh. No fresh native visual/keyboard/AX journey was captured locally because the UI reviewer worktree lacked dependencies; that remains a confidence gap, not separate author rework.
Any new head invalidates this verdict.
- addMessageStep fills the step-owned textarea (id wf-step-*-text) instead
of the ambiguous getByLabel("Message text"), so the fill auto-waits for
the entering step pane and can never land in the exiting trigger
condition input during AnimatePresence.
- template-variables spec uses the same owned-textarea locator for the
identical step-pane fill race (the fill was landing in the exiting
trigger "Message text" input, so the autocomplete listbox never
opened).
- message-condition round-trip waits for animations after opening the
trigger inspector before reading operator bounding boxes, so the ~240ms
inspector width transition cannot distort sequential box positions.
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
The committed darwin image for workflow-template-variable-autocomplete was recorded at 9390e11 and no longer matches the UI it captures: the name-row enable switch is gone (activation goes through submit/confirmation) and the message_posted trigger renders MessageSquare instead of a lightning glyph. At candidate 1143af3, an isolated build:e2e run (dedicated port 4891) reproduced the 438-pixel failure whose mask bounds exactly those two intended changes; the diff rendering is byte-identical to the independently reviewed historical failure, and today's actual differs from that historical actual by only 80 pixels of +-1-3 RGB-unit antialiasing noise (0.009%). Baseline refreshed from that proven candidate actual; the focused test now passes, including all subsequent keyboard/caret assertions. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — APPROVE
Reviewed: bfc384855889432df4a333a0edf3080f332ee169..236af9e6137386737e84d3a474d6bc808a704c50 (exact live head 236af9e6137386737e84d3a474d6bc808a704c50)
Risk: medium — user-visible workflow test synchronization and a Darwin visual baseline on top of broad Desktop identity presentation work.
Findings: no unresolved author-actionable defect. Two independent changed-head review lanes agree that the new synchronization follows production ownership and animation lifecycle rather than hiding failures:
workflow-local-controls.spec.ts:67-69,158targets the entering step-owned textarea by its structural ID.WorkflowStepCard.tsx:147-157,411generateswf-step-${index}, binds the label to${prefix}-text, and assigns that ID to the textarea, excluding the departing trigger field that shares the accessible name “Message text.”workflow-local-controls.spec.ts:290-305uses the repository’s bounded Web Animations helper (tests/helpers/animations.ts:18-30, 1-second ceiling) before geometry assertions against an inspector whose width/content animate. Operator ordering, selected state, exact text, Advanced/Basic round-trip, save, reopen, keyboard behavior, and accessibility assertions remain intact.- The refreshed 1280×720 Darwin baseline was inspected before/after. Its visible changes match current production UI: removal of the obsolete enable switch and lightning-to-message icon replacement. The autocomplete list, typed value, labels, caret state, and surrounding layout remain represented. Exact comparison found 5,573 changed pixels (0.6047%); the old baseline is byte-identical to its historical introduction.
Author action: none. Any new head invalidates this approval.
Verification owner: CI/merge owner for terminal exact-head Desktop Core and Smoke E2E shards, especially shard 4. A PR-caused required-gate failure would require renewed author action and review; pending reviewer tooling or gate completion alone is not a code defect.
Exact-head validation
All local runs were bracketed by exact HEAD 236af9e6137386737e84d3a474d6bc808a704c50 and a clean tracked tree:
- Full
workflow-local-controls.spec.ts: 12/12 passed underCI=1afterpnpm build:e2e. - Two race-sensitive workflow cases repeated 10 times each: 20/20 passed.
- Full Desktop unit package: 6,471/6,471 passed.
- Desktop check and typecheck: passed; only pre-existing non-fatal Biome advisories were reported.
git diff --check: passed.- Windows/macOS builds, relay-backed integration, DCO, Semgrep, and zizmor were green at final pre-submit refresh.
Residual risk
A compiling mutation reverting only the three synchronization edits also passed the 20-repeat local probe, so local natural-speed execution did not reproduce the earlier CI race. The fix remains source-backed and exact-head candidate coverage is green locally, but stressed exact-head Smoke shard 4 is the load-bearing causal confirmation. No fresh installed-native Tauri journey was required for this test/snapshot-only changed-head delta.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — APPROVE
Reviewed: bfc384855889432df4a333a0edf3080f332ee169..236af9e6137386737e84d3a474d6bc808a704c50 (exact head 236af9e6137386737e84d3a474d6bc808a704c50)
Risk: medium — user-visible workflow E2E synchronization and a Darwin screenshot baseline layered onto a broad Desktop identity-presentation PR.
Behavior/contracts traced: production ownership of workflow step textareas; exiting/entering AnimatePresence panes; trigger-inspector width/content animations and geometry assertions; message-condition save/reopen; template-variable keyboard/listbox/focus/caret semantics; Darwin baseline provenance and visible delta; identity presentation regressions retained from the broader PR.
Findings: no unresolved author-actionable defect. Both assigned review lanes independently found the synchronizers semantically tied to production behavior rather than arbitrary sleeps:
- The new
wf-step-${index}-textlocator selects the entering step-owned textarea generated and labeled byWorkflowStepCard, avoiding the exiting trigger condition’s identically named field. waitForAnimationsis the repository’s bounded Web Animations helper and runs before operator geometry is read, matching the 240 ms inspector-width / 150 ms content transitions while preserving the positional, selection, round-trip, and reopen assertions.- The refreshed 1280×720 Darwin baseline shows the intended current UI changes—removal of the obsolete enable switch and lightning-to-message icon replacement—without hiding the autocomplete, typed value, caret, labels, or layout. The historical pixel-count narrative was not needed for clearance.
Author action: none.
Verification owner: exact-head GitHub Desktop Smoke E2E (4) and Desktop Core gates own the remaining stressed-environment proof. If either reports a PR-caused failure, or if the head changes, this approval expires and requires fresh review.
Exact-head validation
At clean 236af9e6137386737e84d3a474d6bc808a704c50:
- Full
workflow-local-controls.spec.ts: 12/12 passed on isolated port 4891, including screenshot, layout, keyboard, and accessibility assertions. - Race-sensitive template-variable and structured-message-condition cases: 20/20 passed across ten repeats.
- Full Desktop unit package: 6,471/6,471 passed.
pnpm -C desktop build:e2e, Desktop check, typecheck, andgit diff --check: passed.- Windows/macOS builds, relay-backed Desktop integration, DCO, Semgrep, and zizmor: green at final refresh.
- Authenticated reviewer
jedwards27is distinct from authorloganj.
Manual/native evidence: reviewers inspected the old/new Darwin baseline and pixel-difference rendering; no fresh installed-native Tauri journey was produced.
Residual risk: reverting only the three test synchronizers did not reproduce the timing failure locally across twenty repeated cases. The source/lifecycle mapping and candidate runs support the repair, but exact-head Smoke shard 4 remains the load-bearing CI proof under contention. At submission, Desktop Core and Smoke shards 1–4 were still in progress without failure. These are CI/reviewer confidence gaps, not author rework.
|
🤖 Addressing review 5156398401 at published head 236af9e6 (
Local evidence at these commits: the round-trip test ( |
🤖 ## Summary In the mobile app, anyone who hasn't set a display name shows up as a raw 64-character hex key (e.g. `3a5d4f9c…`) — unreadable, and unrecognizable as the same identity across screens. Profile and Settings also let you copy that raw hex. Nostr public keys have a standard readable form — `npub1…`, the same encoding other Nostr apps and our desktop app already display. This PR makes every mobile identity surface render npub instead: - **Unnamed people everywhere** — message and thread authors, reactions, typing indicators, member lists, channel details, DM headers and tiles, inbox, search, forum cards, Pulse notes and reply context, mention suggestions, and invite rows — now show a compact npub label: first 8 + last 4 characters of the full npub joined by an ellipsis (`npub1abcd…wxyz`), the same truncation desktop uses. Previously these showed truncated raw hex. - **DM fallback avatars and blank names** — 1:1 DM tiles and headers key their fallback avatar to the same non-self counterpart the label names, including self-first participant order; a self-DM keeps its hex-key-derived initial. Blank or whitespace-only display names fall back to the compact npub instead of rendering empty, while nonblank authored names render verbatim (padding included). - **Profile sheet → "Copy public key"** now copies the full canonical npub — never raw hex. When the identity string isn't a valid public key, the copy tile is disabled, so a malformed key never reaches the clipboard. - **Settings → Identity (pubkey)** displays and copies the full npub; an invalid identity reads "Identity unavailable" with copy disabled. - **Invalid identities never leak truncated raw hex** into the UI anywhere — they render a neutral "Unknown identity" label. - **Unchanged on purpose:** display names and verified handles (NIP-05 — the `name@domain` badge) still render as before. Unnamed avatars keep distinct per-key initials, derived from the underlying hex key rather than the npub — otherwise every unnamed key would render the same "N" initial. Event IDs are not public keys, so they keep their hex truncation (in Pulse's "Replying to", the parent author shows npub while an event-id fallback still shows hex). The nevent share link, private keys, and internal hex storage are untouched. Inputs that accept a key (invite/member entry) accept both hex and npub and keep working in hex internally. ### Related issue N/A. Searched open issues/PRs for npub identity display on mobile — closest related: none found. Desktop's parallel npub standardization lives in the stacked desktop PRs (#7488 foundation, #7489 controls, #7495 display surfaces); this is the independent mobile slice (based directly on `main`, not on those branches). ### Testing At exact head `5a620e420a1fd57d9d8011ac26434eed32fcf765` (base: `main` `44316ff72`; 40 files, +1,345/−154): - Full mobile suite: 2,098 tests passing (`cd mobile && flutter test`); `flutter analyze` clean; `dart format --set-exit-if-changed .` clean — the same checks CI runs. - Widget/unit coverage at production seams: compact labels and hex-keyed avatar initials for DM headers/tiles, member rows, mention suggestions, and Pulse reply context; DM fallback avatars keyed to the labeled counterpart (self-first order and self-DMs); blank/whitespace display-name npub fallback with nonblank authored labels verbatim, including the Activity inbox sender and profile-sheet heading (each with its own empty/whitespace production-seam regression); full-npub copy and disabled-copy semantics in profile and settings; invalid-key suppression; and hex↔npub input round-trips. Verified via unit and widget tests — no device/simulator validation is claimed. ### Task provenance Buzz channel: `1f0e4a3d-7e01-4efe-bb16-843b357f85c9` Task: buzz://message?channel=1f0e4a3d-7e01-4efe-bb16-843b357f85c9&id=86b34eb4bd84a1472419e9af22636c011c0fe273e3c196f967d7a36996e149b6 --------- Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
2 participants
🤖
Summary
Every Buzz account is identified by a long public key. Before this change, when someone had no display name, surfaces fell back to inconsistent labels — mostly raw hex fragments like
abcd1234…wxyz, sometimes a generic role label with no key — so the same person looked different from surface to surface, and nothing looked like an npub address. This PR applies the npub identity foundation from #7488 to the everyday surfaces: a person without a display name now falls back to the same compact npub everywhere —npub1xxxx…yyyy, the human-readable spelling of their public key (first 8 + last 4 characters of the full npub) — across messages and mentions, reactions, huddles, member and participant lists, the sidebar and channel activity, search, projects, tray, notifications, and workflow surfaces.Preserved exactly: display names and distinct avatars, internal hex keys (storage/API forms unchanged), clipboard identity roundtrips, event/blob ID compaction, private keys (no nsec path is touched), and nevent link handling.
Scope: this PR changes what identity labels display, not identity controls — profile/settings copy controls, the respond-to allowlist, workflow key fields, and agent dialogs are the sibling slice #7489, and the shared primitives (
canonicalNpub,truncateNpub, the<PubKey>gate, strict input parsing) come from the foundation #7488.Related issue
Testing
At exact head
4763cbeae1dd521309755e6d61f657324cb98667(base:fix/desktop-npub-identity-d1a@5f3a4a8111998c8aa41ad77cf66992bd1c85343c; 71 files, +656/−189 — production +277/−136, test support +379/−53):mentions.spec.ts(1/1), the e2e build, typecheck, and biome — green.workflow-local-controls438px caret drift. Not claimed green locally.236af9e6137386737e84d3a474d6bc808a704c50(test-only follow-ups1143af345+236af9e6): theworkflow-local-controlsraces were fixed in the test drivers, and the 438px diff was shown to be a stale Darwin snapshot baseline (name-row enable switch already absent andmessage_postedalready MessageSquare at recording commit9390e11c9) and refreshed — the focused screenshot test, including keyboard/caret assertions, now passes locally (twice). The full spec was not rerun after the snapshot refresh; the huddle-transcription item above is unchanged.Label/copy text changes are asserted by the e2e specs (
mentions,mention-recipients,pubkey-display-screenshots,huddle-transcription,channel-activity-popover,workflow-local-controls) rather than new screenshots; the screenshot spec pins the compact npub text forms.Task provenance
Buzz channel:
1f0e4a3d-7e01-4efe-bb16-843b357f85c9Task: buzz://message?channel=1f0e4a3d-7e01-4efe-bb16-843b357f85c9&id=86b34eb4bd84a1472419e9af22636c011c0fe273e3c196f967d7a36996e149b6