mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
fix(desktop): order unnamed roster members by full canonical npub (#7503)
🤖 ## Summary - Channel members appear in the Members sidebar. A member who has never set a display name is listed under an abbreviated form of their public key (npub), and the sidebar previously sorted those unnamed members by that short label. Short labels are not unique — different keys can share one — so the order of unnamed members could look arbitrary or unstable. Unnamed members now sort by their full public key, so the order is deterministic. - When two members display the same name, the previous tiebreak was membership order (who joined first), which is not visible to a reader and can shift as roster data loads in. The tiebreak is now the full public key, so identical display names always land in the same order. - Nothing gets noisier on screen: the full key is used only for sorting, and the sidebar still shows the compact abbreviated form. Priorities are unchanged — authored (custom) names still outrank fallback labels, and role/current-user grouping still applies. - Scope is the desktop app's Members sidebar and member management: the two existing sort comparators. Mobile and other lists in the app are untouched. ### Related issue Based on #7495 (introduced the abbreviated npub labels this follows up on). The original five presentation PRs remain independently reviewable. No closer duplicate found. ### Testing - 6469 desktop unit tests, typecheck, and check pass. - The 3 existing consumer-seam E2E tests still pass; a new E2E test asserts the sidebar lists unnamed members in full-key order, with fixture members deliberately inserted in the opposite order so incoming membership order cannot mask the sort. - Negative check: reverting only this change makes the new ordering assertion fail, so it genuinely binds the new sort. - CI has not run on this PR yet. Buzz provenance: channel 1f0e4a3d-7e01-4efe-bb16-843b357f85c9 / task 340c3de9b27dbedb8453c0c7652220f9080d30fcc70a7c4f6e27fdd4fa378056 --------- Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
This commit is contained in:
5
.github/workflows/_ci-desktop.yml
vendored
5
.github/workflows/_ci-desktop.yml
vendored
@@ -24,7 +24,10 @@ jobs:
|
||||
desktop-core:
|
||||
name: Desktop Core
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 45
|
||||
# The compiled-flag verification step rebuilds the workspace for each
|
||||
# BUZZ_BUILD_* state and runs the full suite under all three compile
|
||||
# states; the complete recipe needs ~46m, so budget 60m.
|
||||
timeout-minutes: 60
|
||||
if: github.event_name == 'push' || inputs.desktop || inputs.desktop_rust || inputs.rust
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
121
desktop/src/features/channels/lib/memberUtils.test.mjs
Normal file
121
desktop/src/features/channels/lib/memberUtils.test.mjs
Normal file
@@ -0,0 +1,121 @@
|
||||
import { strict as assert } from "node:assert";
|
||||
import test from "node:test";
|
||||
|
||||
import { canonicalNpub, truncateNpub } from "@/shared/lib/pubkey";
|
||||
import { compareMembersByRole, formatMemberName } from "./memberUtils.ts";
|
||||
|
||||
// Sequential-value pubkeys, the same shape as the members-sidebar e2e
|
||||
// roster fixture: every full npub shares the `npub1qqq…` head, so the
|
||||
// compact label is decided by the checksum tail while the full npub
|
||||
// diverges mid-key. These two keys disagree between the two orders.
|
||||
const V5_HEX =
|
||||
"0000000000000000000000000000000000000000000000000000000000000005";
|
||||
const V5_NPUB =
|
||||
"npub1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqzsfj2hcx";
|
||||
const V24_HEX =
|
||||
"0000000000000000000000000000000000000000000000000000000000000018";
|
||||
const V24_NPUB =
|
||||
"npub1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqvq532w5c";
|
||||
|
||||
function member(pubkey, overrides = {}) {
|
||||
return {
|
||||
pubkey,
|
||||
role: "member",
|
||||
isAgent: false,
|
||||
joinedAt: "2026-09-08T00:00:00Z",
|
||||
displayName: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function rosterOrder(members, currentPubkey) {
|
||||
return [...members]
|
||||
.sort((left, right) => compareMembersByRole(left, right, currentPubkey))
|
||||
.map((item) => item.pubkey);
|
||||
}
|
||||
|
||||
test("unnamed members order by full canonical npub, not the compact label", () => {
|
||||
assert.equal(canonicalNpub(V5_HEX), V5_NPUB);
|
||||
assert.equal(canonicalNpub(V24_HEX), V24_NPUB);
|
||||
// Compact labels order V5 first (`…2hcx` < `…2w5c`); the full npubs
|
||||
// disagree (`…vq53…` < `…zsfj…`). Ordering follows the full key.
|
||||
assert.ok(truncateNpub(V5_HEX).localeCompare(truncateNpub(V24_HEX)) < 0);
|
||||
assert.ok(V24_NPUB.localeCompare(V5_NPUB) < 0);
|
||||
|
||||
const v5 = member(V5_HEX);
|
||||
const v24 = member(V24_HEX);
|
||||
|
||||
assert.deepEqual(rosterOrder([v5, v24]), [V24_HEX, V5_HEX]);
|
||||
assert.deepEqual(rosterOrder([v24, v5]), [V24_HEX, V5_HEX]);
|
||||
|
||||
// The compact 8+4 label stays the display form.
|
||||
assert.equal(formatMemberName(v5), "npub1qqq…2hcx");
|
||||
assert.equal(formatMemberName(v24), "npub1qqq…2w5c");
|
||||
|
||||
// Authored names still order against npub surfaces as before.
|
||||
const bob = member("0".repeat(64), { displayName: "Bob" });
|
||||
assert.deepEqual(rosterOrder([v5, bob, v24]), [
|
||||
"0".repeat(64),
|
||||
V24_HEX,
|
||||
V5_HEX,
|
||||
]);
|
||||
});
|
||||
|
||||
test("duplicate authored names tie-break by the full identity key", () => {
|
||||
const lowHexName = member(V5_HEX, { displayName: "Ada" });
|
||||
const highHexName = member(V24_HEX, { displayName: "Ada" });
|
||||
|
||||
// Same name, distinct keys: the tie breaks by canonical npub (which
|
||||
// reverses the raw hex order of these two keys), in both input orders.
|
||||
assert.deepEqual(rosterOrder([lowHexName, highHexName]), [V24_HEX, V5_HEX]);
|
||||
assert.deepEqual(rosterOrder([highHexName, lowHexName]), [V24_HEX, V5_HEX]);
|
||||
});
|
||||
|
||||
test("role precedence still outranks the name stage", () => {
|
||||
const owner = member("1".repeat(64), { role: "owner", displayName: "Zed" });
|
||||
const admin = member("2".repeat(64), { role: "admin", displayName: "Yan" });
|
||||
const plain = member("3".repeat(64), { role: "member", displayName: "Xan" });
|
||||
const guest = member("4".repeat(64), { role: "guest", displayName: "Wes" });
|
||||
const bot = member("5".repeat(64), { role: "bot", displayName: "Ann" });
|
||||
|
||||
assert.deepEqual(rosterOrder([bot, guest, plain, admin, owner]), [
|
||||
"1".repeat(64),
|
||||
"2".repeat(64),
|
||||
"3".repeat(64),
|
||||
"4".repeat(64),
|
||||
"5".repeat(64),
|
||||
]);
|
||||
});
|
||||
|
||||
test("the current member still sorts first in compareMembersByRole", () => {
|
||||
const current = member(V24_HEX);
|
||||
const owner = member("1".repeat(64), { role: "owner", displayName: "Ada" });
|
||||
|
||||
assert.deepEqual(rosterOrder([owner, current], V24_HEX), [
|
||||
V24_HEX,
|
||||
"1".repeat(64),
|
||||
]);
|
||||
assert.ok(compareMembersByRole(current, owner, V24_HEX) < 0);
|
||||
assert.ok(compareMembersByRole(owner, current, V24_HEX) > 0);
|
||||
|
||||
// Without a current pubkey, roles lead again.
|
||||
assert.ok(compareMembersByRole(current, owner) > 0);
|
||||
});
|
||||
|
||||
test("invalid keys keep the neutral surface and break ties deterministically", () => {
|
||||
const first = member("not-a-key");
|
||||
const second = member("zzz-definitely-not-a-key");
|
||||
|
||||
// Both render the neutral label — never raw input — and the label
|
||||
// collision breaks by the normalized key, not incoming order.
|
||||
assert.equal(formatMemberName(first), "Unavailable");
|
||||
assert.equal(formatMemberName(second), "Unavailable");
|
||||
assert.deepEqual(rosterOrder([second, first]), [
|
||||
"not-a-key",
|
||||
"zzz-definitely-not-a-key",
|
||||
]);
|
||||
assert.deepEqual(rosterOrder([first, second]), [
|
||||
"not-a-key",
|
||||
"zzz-definitely-not-a-key",
|
||||
]);
|
||||
});
|
||||
@@ -1,5 +1,10 @@
|
||||
import type { ChannelMember } from "@/shared/api/types";
|
||||
import { truncateNpub } from "@/shared/lib/pubkey";
|
||||
import {
|
||||
canonicalNpub,
|
||||
normalizePubkey,
|
||||
truncateNpub,
|
||||
UNAVAILABLE_KEY_LABEL,
|
||||
} from "@/shared/lib/pubkey";
|
||||
|
||||
export const roleOrder: Record<ChannelMember["role"], number> = {
|
||||
owner: 0,
|
||||
@@ -20,6 +25,59 @@ export function formatMemberName(
|
||||
return member.displayName ?? truncateNpub(member.pubkey);
|
||||
}
|
||||
|
||||
/**
|
||||
* Ordering surface for a member's name: the authored name when present,
|
||||
* else the FULL canonical npub. Separate from `formatMemberName` — the
|
||||
* compact `npub1abcd…wxyz` label is display-only, and ordering on it would
|
||||
* collapse two distinct identities that merely share a prefix and tail.
|
||||
* Undisplayable keys keep the neutral label as their surface, exactly as
|
||||
* they render.
|
||||
*/
|
||||
function memberNameSurface(member: ChannelMember): string {
|
||||
return (
|
||||
member.displayName ?? canonicalNpub(member.pubkey) ?? UNAVAILABLE_KEY_LABEL
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Full identity key used to break name-surface ties: the canonical npub of
|
||||
* a valid identity, else the normalized raw key so every tie is decided.
|
||||
* An ordering key only — never rendered or copied.
|
||||
*/
|
||||
function memberIdentityKey(member: ChannelMember): string {
|
||||
return canonicalNpub(member.pubkey) ?? normalizePubkey(member.pubkey);
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared name/key ordering authority for the roster comparators.
|
||||
*
|
||||
* Authored names keep the existing `localeCompare` semantics; unnamed
|
||||
* members order by their full canonical npub (the compact label stays
|
||||
* display-only); collation-equal surfaces — duplicate authored names,
|
||||
* matching labels, invalid keys — break by full identity key, so the
|
||||
* incoming membership-event order is never the tie policy. Comparators
|
||||
* layer their own role/current-user precedence around this stage; this
|
||||
* helper owns only the name ordering.
|
||||
*/
|
||||
export function compareMemberNames(
|
||||
left: ChannelMember,
|
||||
right: ChannelMember,
|
||||
): number {
|
||||
const surfaceDelta = memberNameSurface(left).localeCompare(
|
||||
memberNameSurface(right),
|
||||
);
|
||||
if (surfaceDelta !== 0) {
|
||||
return surfaceDelta;
|
||||
}
|
||||
|
||||
const leftKey = memberIdentityKey(left);
|
||||
const rightKey = memberIdentityKey(right);
|
||||
if (leftKey === rightKey) {
|
||||
return 0;
|
||||
}
|
||||
return leftKey < rightKey ? -1 : 1;
|
||||
}
|
||||
|
||||
export function compareMembersByRole(
|
||||
left: ChannelMember,
|
||||
right: ChannelMember,
|
||||
@@ -35,5 +93,5 @@ export function compareMembersByRole(
|
||||
if (roleDelta !== 0) {
|
||||
return roleDelta;
|
||||
}
|
||||
return formatMemberName(left).localeCompare(formatMemberName(right));
|
||||
return compareMemberNames(left, right);
|
||||
}
|
||||
|
||||
@@ -16,7 +16,10 @@ import {
|
||||
} from "@/features/agents/lib/agentAutocompleteEligibility";
|
||||
import { useIsArchivedPredicate } from "@/features/identity-archive/hooks";
|
||||
import { useClassifiedMembers } from "@/features/channels/lib/useClassifiedMembers";
|
||||
import { formatMemberName } from "@/features/channels/lib/memberUtils";
|
||||
import {
|
||||
compareMemberNames,
|
||||
formatMemberName,
|
||||
} from "@/features/channels/lib/memberUtils";
|
||||
import {
|
||||
canAddChannelMembers,
|
||||
PRIVATE_CHANNEL_ADD_DENIED_MESSAGE,
|
||||
@@ -120,7 +123,7 @@ function compareMembersForModal(
|
||||
if (currentPubkey && left.pubkey === currentPubkey) return -1;
|
||||
if (currentPubkey && right.pubkey === currentPubkey) return 1;
|
||||
|
||||
return formatMemberName(left).localeCompare(formatMemberName(right));
|
||||
return compareMemberNames(left, right);
|
||||
}
|
||||
|
||||
type MembersSidebarProps = {
|
||||
|
||||
@@ -27,6 +27,15 @@ const OWNED_RELAY_AGENT_PUBKEY =
|
||||
"a1b2c3d4e5f60718293a4b5c6d7e8f90112233445566778899aabbccddeeff00";
|
||||
const DM_RELAY_AGENT_PUBKEY =
|
||||
"eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee";
|
||||
// Unnamed roster fixtures whose two plausible orders disagree (the e2e twin
|
||||
// of the memberUtils unit pair): both keys share the `npub1qqq…` head, so
|
||||
// the compact display labels order V5 first (`…2hcx` < `…2w5c`) while the
|
||||
// full canonical npubs order V24 first (`…vq53…` < `…zsfj…`). Only the
|
||||
// full-npub order is correct for the roster.
|
||||
const UNNAMED_MEMBER_V5_PUBKEY =
|
||||
"0000000000000000000000000000000000000000000000000000000000000005";
|
||||
const UNNAMED_MEMBER_V24_PUBKEY =
|
||||
"0000000000000000000000000000000000000000000000000000000000000018";
|
||||
|
||||
type MockFeedWindow = Window & {
|
||||
__BUZZ_E2E_EMIT_MOCK_MESSAGE__?: (input: {
|
||||
@@ -4177,8 +4186,9 @@ test("members sidebar virtualizes large channel rosters", async ({ page }) => {
|
||||
expect(await memberRows.count()).toBeLessThan(50);
|
||||
|
||||
// Generated members have no display name, so the roster sorts them by
|
||||
// their npub fallback label: these sequential pubkeys share a `npub1qqq…`
|
||||
// prefix and order by the checksum tail, not their numeric value. Resolve
|
||||
// their full canonical npub: these sequential pubkeys share an
|
||||
// `npub1qqq…` head and diverge mid-key, while the compact label's
|
||||
// checksum tail is display-only and decides nothing. Resolve
|
||||
// a generated member from the rows the initial window actually rendered
|
||||
// instead of assuming `pubkeys[0]` sorts into that window.
|
||||
const generatedPubkeySet = new Set(pubkeys);
|
||||
@@ -4253,6 +4263,57 @@ test("members sidebar virtualizes large channel rosters", async ({ page }) => {
|
||||
).toBeVisible();
|
||||
});
|
||||
|
||||
test("members sidebar orders unnamed members by full canonical npub", async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.goto("/");
|
||||
const channelId = await page
|
||||
.getByTestId("channel-random")
|
||||
.getAttribute("data-channel-id");
|
||||
if (!channelId) {
|
||||
throw new Error("Random channel id missing.");
|
||||
}
|
||||
|
||||
// Added in the opposite of the expected order, so incoming membership
|
||||
// order can never satisfy the assertion on its own.
|
||||
await invokeMockCommand(page, "add_channel_members", {
|
||||
channelId,
|
||||
pubkeys: [UNNAMED_MEMBER_V5_PUBKEY, UNNAMED_MEMBER_V24_PUBKEY],
|
||||
role: "member",
|
||||
});
|
||||
|
||||
await openMembersSidebar(page, "random");
|
||||
// "random" seeds alice, the mock identity, and bob, so the two unnamed
|
||||
// fixtures round out a five-row roster that the initial virtual window
|
||||
// renders entirely — both fixtures are visible without scrolling.
|
||||
await expect(
|
||||
page.getByTestId(`sidebar-member-${UNNAMED_MEMBER_V24_PUBKEY}`),
|
||||
).toBeVisible();
|
||||
await expect(
|
||||
page.getByTestId(`sidebar-member-${UNNAMED_MEMBER_V5_PUBKEY}`),
|
||||
).toBeVisible();
|
||||
|
||||
// The compact labels (`npub1qqq…2hcx` < `npub1qqq…2w5c`) would order V5
|
||||
// first; the full canonical npubs disagree and order V24 first. The
|
||||
// rendered roster must follow the full key, not the display label.
|
||||
const renderedOrder = await page
|
||||
.getByTestId("members-sidebar-people")
|
||||
.locator('[data-index] > [data-testid^="sidebar-member-"]')
|
||||
.evaluateAll((rows) =>
|
||||
rows.map(
|
||||
(row) =>
|
||||
(row as HTMLElement).dataset.testid?.slice(
|
||||
"sidebar-member-".length,
|
||||
) ?? "",
|
||||
),
|
||||
);
|
||||
const v24Position = renderedOrder.indexOf(UNNAMED_MEMBER_V24_PUBKEY);
|
||||
const v5Position = renderedOrder.indexOf(UNNAMED_MEMBER_V5_PUBKEY);
|
||||
expect(v24Position).toBeGreaterThanOrEqual(0);
|
||||
expect(v5Position).toBeGreaterThanOrEqual(0);
|
||||
expect(v24Position).toBeLessThan(v5Position);
|
||||
});
|
||||
|
||||
test("opening a human-only members sidebar skips managed runtime discovery", async ({
|
||||
page,
|
||||
}) => {
|
||||
|
||||
Reference in New Issue
Block a user