diff --git a/.github/workflows/_ci-desktop.yml b/.github/workflows/_ci-desktop.yml index 108e18a2152..15893a1cb4e 100644 --- a/.github/workflows/_ci-desktop.yml +++ b/.github/workflows/_ci-desktop.yml @@ -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 diff --git a/desktop/src/features/channels/lib/memberUtils.test.mjs b/desktop/src/features/channels/lib/memberUtils.test.mjs new file mode 100644 index 00000000000..751e7d415e2 --- /dev/null +++ b/desktop/src/features/channels/lib/memberUtils.test.mjs @@ -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", + ]); +}); diff --git a/desktop/src/features/channels/lib/memberUtils.ts b/desktop/src/features/channels/lib/memberUtils.ts index 10ad4b7ff0a..401fc1f51cb 100644 --- a/desktop/src/features/channels/lib/memberUtils.ts +++ b/desktop/src/features/channels/lib/memberUtils.ts @@ -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 = { 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); } diff --git a/desktop/src/features/channels/ui/MembersSidebar.tsx b/desktop/src/features/channels/ui/MembersSidebar.tsx index 3c9afbfaeca..2a125e0d014 100644 --- a/desktop/src/features/channels/ui/MembersSidebar.tsx +++ b/desktop/src/features/channels/ui/MembersSidebar.tsx @@ -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 = { diff --git a/desktop/tests/e2e/channels.spec.ts b/desktop/tests/e2e/channels.spec.ts index 371cffdfb0b..b7cf2c23cbf 100644 --- a/desktop/tests/e2e/channels.spec.ts +++ b/desktop/tests/e2e/channels.spec.ts @@ -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, }) => {