From 0e7bf1ffe0cb65c39d657d5ae799a83c75bb7cfb Mon Sep 17 00:00:00 2001 From: Carl Date: Thu, 17 Sep 2026 08:18:07 -1000 Subject: [PATCH 1/2] feat(channels): save sidebar groups and starred channels Expose group assignment and Star/Unstar through confirmed host-owned preference writes. Preserve unrelated encrypted data, fence stale reads and cancelled sessions, and restore keyboard focus after exclusive row relocation. Signed-off-by: Carl --- dev/relay-broker.mjs | 174 ++++++++++ dev/sidebar-preference-writes.test.mjs | 179 ++++++++++ dev/sidebar-preferences.d.mts | 28 ++ dev/sidebar-preferences.mjs | 135 +++++++- dev/sidebar-stars.mjs | 90 +++++ dev/sidebar-stars.test.mjs | 211 ++++++++++++ docs/channels.md | 23 ++ src/bundled/channels/Channels.module.css | 23 ++ src/bundled/channels/ChannelsPage.tsx | 252 +++++++++++++- src/bundled/channels/sidebar-sections.test.ts | 19 ++ src/bundled/channels/useSidebarPreferences.ts | 4 + src/features/relay/session.ts | 28 ++ .../relay/sidebar-preferences-store.test.ts | 312 +++++++++++++++++- .../relay/sidebar-preferences-store.ts | 132 +++++++- .../relay/sidebar-preferences.test.ts | 174 ++++++++++ src/features/relay/sidebar-preferences.ts | 20 +- src/features/relay/transport.ts | 60 +++- tests/browser/fixture.mjs | 66 +++- tests/browser/navigation-groups.spec.mjs | 179 ++++++++++ 19 files changed, 2069 insertions(+), 40 deletions(-) create mode 100644 dev/sidebar-preference-writes.test.mjs create mode 100644 dev/sidebar-preferences.d.mts create mode 100644 dev/sidebar-stars.mjs create mode 100644 dev/sidebar-stars.test.mjs diff --git a/dev/relay-broker.mjs b/dev/relay-broker.mjs index b6e56d46..7d18a87f 100644 --- a/dev/relay-broker.mjs +++ b/dev/relay-broker.mjs @@ -1,3 +1,7 @@ +import { + assertSidebarStarIntent, + mutateSidebarStar, +} from "./sidebar-stars.mjs"; import { validateWorkflowEvent, WORKFLOW_KINDS, @@ -23,6 +27,8 @@ import { import { readAgentLibrary } from "./agent-library.mjs"; import { decodeSidebarPreferences, + assertSidebarAssignmentIntent, + mutateSidebarAssignment, SIDEBAR_REQUEST_BYTES, SIDEBAR_UPLOAD_MS, SIDEBAR_UPLOAD_SLOTS, @@ -59,6 +65,7 @@ const MAX_FILTERS = 4, MAX_LIMIT = 500, MAX_INFLIGHT = 6, MAX_MEDIA_BYTES = 20 * 1024 * 1024, + SIDEBAR_HEAD_BYTES = SIDEBAR_REQUEST_BYTES + 4096, UPSTREAM_TIMEOUT_MS = 20000, KEEPALIVE_MS = 60000; @@ -300,6 +307,27 @@ export function relayBrokerPlugin({ const upstream = createUpstream(); // Injected fixtures bypass the pool; the live relay always uses the warm agent. const fetchUpstream = upstreamFetch ?? upstream.fetch; + const readSidebarHead = async (response, label = "group") => { + if (!response.body) + throw new Error(`Sidebar ${label} response missing`); + const reader = response.body.getReader(); + const decoder = new TextDecoder("utf-8", { fatal: true }); + let bytes = 0, + text = ""; + try { + while (true) { + const { value, done } = await reader.read(); + if (done) return JSON.parse(text + decoder.decode()); + bytes += value.byteLength; + if (bytes > SIDEBAR_HEAD_BYTES) + throw new Error(`Sidebar ${label} response exceeds capacity`); + text += decoder.decode(value, { stream: true }); + } + } finally { + await reader.cancel().catch(() => {}); + reader.releaseLock(); + } + }; // Discovery is lazy and independent for each community; unavailable relays never block startup. const registered = new Map(Object.entries(aliases)); const authorities = new Map(); @@ -342,6 +370,7 @@ export function relayBrokerPlugin({ let inflight = 0; let sidebarUploads = 0; let libraryRead; + const sidebarMutations = new Map(); const streams = new Map(); const admissions = createHostAdmission(); server.httpServer?.once("close", () => { @@ -528,6 +557,149 @@ export function relayBrokerPlugin({ sidebarUploads--; } } + if ( + [ + "/api/relay/sidebar-assignment", + "/api/relay/sidebar-star", + ].includes(route) && + req.method === "POST" + ) { + const starring = route === "/api/relay/sidebar-star"; + let raw = ""; + for await (const part of req) { + raw += part; + if (Buffer.byteLength(raw) > 2048) + return json(res, 413, { + error: `Sidebar preference intent is too large`, + }); + } + let intent; + try { + intent = JSON.parse(raw); + if (starring) assertSidebarStarIntent(intent); + else assertSidebarAssignmentIntent(intent); + } catch { + return json(res, 400, { + error: `Invalid sidebar preference intent`, + }); + } + const request = new AbortController(); + const close = () => request.abort(); + res.once("close", close); + const previous = sidebarMutations.get(relay) ?? Promise.resolve(); + const mutation = previous + .catch(() => {}) + .then(async () => { + request.signal.throwIfAborted(); + const filter = [ + { + kinds: [30078], + authors: [viewer], + "#d": [starring ? "channel-stars" : "channel-sections"], + limit: 1, + }, + ]; + const lane = admissions(relay, viewer).api; + const requestSignal = AbortSignal.any([ + request.signal, + AbortSignal.timeout(UPSTREAM_TIMEOUT_MS), + ]); + const dispatch = (path, body) => + admittedApiRequest( + lane, + () => { + requestSignal.throwIfAborted(); + const value = JSON.stringify(body); + const auth = finalizeEvent( + { + kind: 27235, + created_at: Math.floor(Date.now() / 1000), + content: "", + tags: [ + ["u", `${relay}${path}`], + ["method", "POST"], + [ + "payload", + createHash("sha256").update(value).digest("hex"), + ], + ["nonce", randomBytes(16).toString("hex")], + ], + }, + key, + ); + return fetchUpstream(`${relay}${path}`, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: + "Nostr " + + Buffer.from(JSON.stringify(auth)).toString( + "base64", + ), + }, + body: value, + redirect: "error", + signal: requestSignal, + }); + }, + requestSignal, + ); + const readHead = async () => { + const response = await dispatch("/query", filter); + if (!response.ok) + throw new Error( + `Sidebar preference query failed (${response.status})`, + ); + return readSidebarHead(response); + }; + const publishEvent = async (event) => { + const response = await dispatch("/events", event); + if (!response.ok) + throw new Error( + `Sidebar preference publish failed (${response.status})`, + ); + const receipt = await readSidebarHead( + response, + "publication", + ); + if ( + receipt.event_id !== event.id || + receipt.accepted !== true + ) + throw new Error( + "Sidebar preference publication was not accepted", + ); + }; + return (starring ? mutateSidebarStar : mutateSidebarAssignment)( + intent, + key, + readHead, + publishEvent, + ); + }); + sidebarMutations.set(relay, mutation); + try { + return json(res, 200, await mutation); + } catch (error) { + if (error instanceof ApiPaused) + return json(res, 429, { + error: error.message, + sent: false, + paused: true, + retryAfterMs: error.retryAfterMs, + }); + return json(res, 502, { + error: + error instanceof Error + ? error.message + : `Sidebar preference failed`, + }); + } finally { + res.off("close", close); + if (sidebarMutations.get(relay) === mutation) + sidebarMutations.delete(relay); + } + } if (route === "/api/relay/agent-library" && req.method === "GET") { try { // Share concurrent reads, never retain the local snapshot after completion. @@ -553,6 +725,8 @@ export function relayBrokerPlugin({ workflowReads: true, sidebarPreferences: true, readState: true, + sidebarPreferenceWrites: true, + sidebarStarWrites: true, agentLibrary: true, live: true, agentActivity: true, diff --git a/dev/sidebar-preference-writes.test.mjs b/dev/sidebar-preference-writes.test.mjs new file mode 100644 index 00000000..232ecc2f --- /dev/null +++ b/dev/sidebar-preference-writes.test.mjs @@ -0,0 +1,179 @@ +import { createServer } from "node:http"; +import { createHash } from "node:crypto"; +import { afterEach, expect, it } from "vitest"; +import { generateSecretKey, getPublicKey, verifyEvent } from "nostr-tools"; +import { relayBrokerPlugin } from "./relay-broker.mjs"; +import { prepareSidebarStar } from "./sidebar-stars.mjs"; +import { connectBrokerTransport } from "../src/features/relay/transport.ts"; +import { fixtureRelayUrl, fixtureAliases } from "../tests/relay-config.ts"; + +const disposals = []; +afterEach(async () => { + for (const dispose of disposals.splice(0)) await dispose(); +}); +async function harness() { + const key = generateSecretKey(), + viewer = getPublicKey(key); + let handler, queryFailure, publicationFailure; + let conflict = false; + const heads = new Map(), + calls = []; + const server = createServer((req, res) => { + req.headers.origin ??= `http://${req.headers.host}`; + handler(req, res); + }); + await relayBrokerPlugin({ + relayUrl: fixtureRelayUrl, + communityAliases: fixtureAliases, + identity: () => key, + upstreamFetch: async (url, init) => { + if (!init?.body) return Response.json({ self: viewer }); + const body = JSON.parse(init.body); + const auth = JSON.parse( + Buffer.from(init.headers.Authorization.slice(6), "base64").toString(), + ); + expect(verifyEvent(auth)).toBe(true); + expect(auth.pubkey).toBe(viewer); + expect(auth.tags).toContainEqual(["u", String(url)]); + expect(auth.tags).toContainEqual(["method", "POST"]); + expect(auth.tags).toContainEqual([ + "payload", + createHash("sha256").update(init.body).digest("hex"), + ]); + expect(init.redirect).toBe("error"); + calls.push({ url: String(url), body }); + if (String(url).endsWith("/events")) { + expect(verifyEvent(body)).toBe(true); + if (publicationFailure) return publicationFailure; + if (!conflict) + heads.set(body.tags.find(([name]) => name === "d")[1], body); + return Response.json({ accepted: true, event_id: body.id }); + } + if (queryFailure) return queryFailure; + const head = heads.get(body[0]["#d"][0]); + return Response.json(head ? [head] : []); + }, + }).configureServer({ + httpServer: server, + config: { logger: { info() {}, error() {} } }, + middlewares: { + use(callback) { + handler = callback; + }, + }, + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + disposals.push(async () => { + server.closeAllConnections(); + await new Promise((resolve) => server.close(resolve)); + }); + const base = `http://127.0.0.1:${server.address().port}`; + const transport = await connectBrokerTransport(base); + return { + key, + viewer, + transport, + calls, + heads, + failQuery(value) { + queryFailure = value; + }, + failPublication(value) { + publicationFailure = value; + }, + conflict() { + conflict = true; + }, + post(value, origin) { + return fetch(`${base}/api/relay/sidebar-star`, { + method: "POST", + headers: { + "Content-Type": "application/json", + ...(origin ? { Origin: origin } : {}), + }, + body: JSON.stringify(value), + }); + }, + }; +} +it("real broker Star roundtrip signs scoped requests and confirms before projecting", async () => { + const h = await harness(), + signal = new AbortController().signal; + h.heads.set( + "channel-stars", + prepareSidebarStar([], { channelId: "other", starred: true }, h.key).event, + ); + expect( + await h.transport.writeSidebarStar( + { channelId: "alpha", starred: true }, + signal, + ), + ).toEqual(["other", "alpha"]); + expect(h.calls.map((call) => new URL(call.url).pathname)).toEqual([ + "/query", + "/events", + "/query", + ]); + expect(h.calls[0].body).toEqual([ + { kinds: [30078], authors: [h.viewer], "#d": ["channel-stars"], limit: 1 }, + ]); + expect( + await h.transport.writeSidebarStar( + { channelId: "alpha", starred: false }, + signal, + ), + ).toEqual(["other"]); + expect(h.calls.filter((call) => call.url.endsWith("/events"))).toHaveLength( + 2, + ); + await h.transport.writeSidebarStar( + { channelId: "alpha", starred: false }, + signal, + ); + expect(h.calls.filter((call) => call.url.endsWith("/events"))).toHaveLength( + 2, + ); +}); +it("refuses invalid intent and foreign origins without upstream requests", async () => { + const h = await harness(); + expect((await h.post({ channelId: "alpha", starred: "true" })).status).toBe( + 400, + ); + expect( + ( + await h.post( + { channelId: "alpha", starred: true }, + "https://foreign.invalid", + ) + ).status, + ).toBe(403); + expect( + (await h.post({ channelId: "x".repeat(2100), starred: true })).status, + ).toBe(413); + expect(h.calls).toEqual([]); +}); +it.each(["query", "oversized", "publication", "receipt", "conflict"])( + "does not claim a saved Star after %s failure", + async (failure) => { + const h = await harness(); + if (failure === "query") + h.failQuery(new Response("failed", { status: 503 })); + if (failure === "oversized") + h.failQuery(new Response(`[${" ".repeat(270000)}]`)); + if (failure === "publication") + h.failPublication(new Response("failed", { status: 503 })); + if (failure === "receipt") + h.failPublication(Response.json({ accepted: false, event_id: "wrong" })); + if (failure === "conflict") h.conflict(); + await expect( + h.transport.writeSidebarStar( + { channelId: "alpha", starred: true }, + new AbortController().signal, + ), + ).rejects.toThrow(); + if (["query", "oversized"].includes(failure)) + expect(h.calls.filter((call) => call.url.endsWith("/events"))).toEqual( + [], + ); + }, +); diff --git a/dev/sidebar-preferences.d.mts b/dev/sidebar-preferences.d.mts new file mode 100644 index 00000000..a17e3e0a --- /dev/null +++ b/dev/sidebar-preferences.d.mts @@ -0,0 +1,28 @@ +import type { SidebarGroups } from "../src/features/relay/sidebar-preferences"; +import type { RelayEvent } from "../src/features/relay/events"; + +export function decodeSidebarPreferences( + events: readonly RelayEvent[], + secret: Uint8Array, +): import("../src/features/relay/sidebar-preferences").SidebarPreferences; +export function assertSidebarAssignmentIntent( + intent: unknown, +): asserts intent is { + channelId: string; + sectionId?: string; +}; +export function prepareSidebarAssignment( + events: readonly RelayEvent[], + intent: { channelId: string; sectionId?: string }, + secret: Uint8Array, + now?: number, +): { groups: SidebarGroups; event?: RelayEvent }; +export function mutateSidebarAssignment( + intent: { channelId: string; sectionId?: string }, + secret: Uint8Array, + readHead: () => Promise, + publish: (event: RelayEvent) => Promise, +): Promise; +export const SIDEBAR_REQUEST_BYTES: number; +export const SIDEBAR_UPLOAD_SLOTS: number; +export const SIDEBAR_UPLOAD_MS: number; diff --git a/dev/sidebar-preferences.mjs b/dev/sidebar-preferences.mjs index 86e334bf..dfe6b9e9 100644 --- a/dev/sidebar-preferences.mjs +++ b/dev/sidebar-preferences.mjs @@ -1,4 +1,4 @@ -import { getPublicKey, nip44, verifyEvent } from "nostr-tools"; +import { finalizeEvent, getPublicKey, nip44, verifyEvent } from "nostr-tools"; import { projectSidebarPreferences, SIDEBAR_COORDINATES, @@ -54,3 +54,136 @@ export function decodeSidebarPreferences(events, secret) { key.fill(0); } } + +const SECTION_COORDINATE = "channel-sections"; +function validAssignmentIntent(intent) { + return ( + intent && + typeof intent === "object" && + !Array.isArray(intent) && + typeof intent.channelId === "string" && + intent.channelId.trim().length > 0 && + intent.channelId.length <= 256 && + (intent.sectionId === undefined || + (typeof intent.sectionId === "string" && + intent.sectionId.trim().length > 0 && + intent.sectionId.length <= 256)) && + Object.keys(intent).every((key) => ["channelId", "sectionId"].includes(key)) + ); +} +export function assertSidebarAssignmentIntent(intent) { + if (!validAssignmentIntent(intent)) + throw new Error("Invalid sidebar assignment intent"); +} +function parseSectionsEvent(events, secret) { + if ( + !Array.isArray(events) || + events.length > 1 || + Buffer.byteLength(JSON.stringify(events)) > SIDEBAR_REQUEST_BYTES + ) + throw new Error("Invalid sidebar group head"); + if (!events.length) + return { + blob: { version: 1, sections: [], assignments: {} }, + createdAt: 0, + }; + const [event] = events; + const viewer = getPublicKey(secret); + const tags = event?.tags?.filter?.( + (tag) => Array.isArray(tag) && tag[0] === "d", + ); + if ( + event?.kind !== 30078 || + event.pubkey !== viewer || + tags?.length !== 1 || + tags[0]?.[1] !== SECTION_COORDINATE || + typeof event.content !== "string" || + !verifyEvent(event) + ) + throw new Error("Invalid sidebar group head"); + const key = nip44.v2.utils.getConversationKey(secret, viewer); + try { + const plaintext = nip44.v2.decrypt(event.content, key); + if (Buffer.byteLength(plaintext) > 128 * 1024) + throw new Error("Sidebar plaintext budget exceeded"); + const blob = JSON.parse(plaintext); + projectSidebarPreferences(blob, undefined); + return { blob, createdAt: event.created_at }; + } finally { + key.fill(0); + } +} +/** Narrow host command: mutate one assignment against the latest encrypted head. */ +export function prepareSidebarAssignment( + events, + intent, + secret, + now = Date.now(), +) { + assertSidebarAssignmentIntent(intent); + const viewer = getPublicKey(secret); + const current = parseSectionsEvent(events, secret); + if ( + intent.sectionId !== undefined && + !current.blob.sections.some((section) => section.id === intent.sectionId) + ) + throw new Error("Sidebar group no longer exists"); + const assignments = { + ...current.blob.assignments, + ...(intent.sectionId === undefined + ? {} + : { [intent.channelId]: intent.sectionId }), + }; + if (intent.sectionId === undefined) delete assignments[intent.channelId]; + const blob = { ...current.blob, assignments }; + const groups = projectSidebarPreferences(blob, undefined); + const previous = Object.hasOwn(current.blob.assignments, intent.channelId) + ? current.blob.assignments[intent.channelId] + : undefined; + if (previous === intent.sectionId) return { groups }; + const key = nip44.v2.utils.getConversationKey(secret, viewer); + let content; + try { + content = nip44.v2.encrypt(JSON.stringify(blob), key); + } finally { + key.fill(0); + } + return { + groups, + event: finalizeEvent( + { + kind: 30078, + content, + created_at: Math.max(Math.floor(now / 1000), current.createdAt + 1), + tags: [ + ["d", SECTION_COORDINATE], + ["t", SECTION_COORDINATE], + ], + }, + secret, + ), + }; +} + +/** Publish one assignment, then re-read the coordinate before reporting saved state. */ +export async function mutateSidebarAssignment( + intent, + secret, + readHead, + publish, +) { + assertSidebarAssignmentIntent(intent); + const draft = prepareSidebarAssignment(await readHead(), intent, secret); + if (!draft.event) return draft.groups; + await publish(draft.event); + const confirmation = prepareSidebarAssignment( + await readHead(), + intent, + secret, + ); + if (confirmation.event) + throw new Error( + "Sidebar groups changed on another device; reload and try again", + ); + return confirmation.groups; +} diff --git a/dev/sidebar-stars.mjs b/dev/sidebar-stars.mjs new file mode 100644 index 00000000..50be9b06 --- /dev/null +++ b/dev/sidebar-stars.mjs @@ -0,0 +1,90 @@ +import { finalizeEvent, getPublicKey, nip44 } from "nostr-tools"; +import { decodeSidebarPreferences } from "./sidebar-preferences.mjs"; + +const COORDINATE = "channel-stars"; +export function assertSidebarStarIntent(intent) { + if ( + !intent || + typeof intent !== "object" || + Array.isArray(intent) || + typeof intent.channelId !== "string" || + !intent.channelId.trim() || + intent.channelId.length > 256 || + typeof intent.starred !== "boolean" || + Object.keys(intent).some((key) => !["channelId", "starred"].includes(key)) + ) + throw new Error("Invalid sidebar star intent"); +} + +/** One explicit star intent against a fresh signed head; keep unstar tombstones. */ +export function prepareSidebarStar(events, intent, secret, now = Date.now()) { + assertSidebarStarIntent(intent); + // The shared bounded decoder verifies signature, own author, schema and budgets. + decodeSidebarPreferences(events, secret); + if ( + events.length > 1 || + events.some( + (event) => + !event.tags.some( + ([name, value]) => name === "d" && value === COORDINATE, + ), + ) + ) + throw new Error("Invalid sidebar star head"); + const viewer = getPublicKey(secret); + const key = nip44.v2.utils.getConversationKey(secret, viewer); + try { + const head = events[0]; + const current = head + ? JSON.parse(nip44.v2.decrypt(head.content, key)) + : { version: 1, channels: {} }; + const previous = Object.hasOwn(current.channels, intent.channelId) + ? current.channels[intent.channelId] + : undefined; + if (previous?.starred === intent.starred) return { stars: current }; + const stars = { + ...current, + channels: { + ...current.channels, + [intent.channelId]: { + ...previous, + starred: intent.starred, + updatedAt: Math.max(now, (previous?.updatedAt ?? 0) + 1), + }, + }, + }; + const event = finalizeEvent( + { + kind: 30078, + content: nip44.v2.encrypt(JSON.stringify(stars), key), + created_at: Math.max( + Math.floor(now / 1000), + (head?.created_at ?? 0) + 1, + ), + tags: [ + ["d", COORDINATE], + ["t", COORDINATE], + ], + }, + secret, + ); + // Refuse over-budget changes rather than silently trimming other channels. + decodeSidebarPreferences([event], secret); + return { stars, event }; + } finally { + key.fill(0); + } +} + +export async function mutateSidebarStar(intent, secret, readHead, publish) { + assertSidebarStarIntent(intent); + const draft = prepareSidebarStar(await readHead(), intent, secret); + if (!draft.event) return draft.stars; + await publish(draft.event); + const confirmation = prepareSidebarStar(await readHead(), intent, secret); + if (confirmation.event) + throw new Error( + "Sidebar stars changed on another device; reload and try again", + ); + return confirmation.stars; +} diff --git a/dev/sidebar-stars.test.mjs b/dev/sidebar-stars.test.mjs new file mode 100644 index 00000000..f97508f9 --- /dev/null +++ b/dev/sidebar-stars.test.mjs @@ -0,0 +1,211 @@ +import { expect, it, vi } from "vitest"; +import { + finalizeEvent, + generateSecretKey, + getPublicKey, + nip44, + verifyEvent, +} from "nostr-tools"; +import { + assertSidebarStarIntent, + prepareSidebarStar, + mutateSidebarStar, +} from "./sidebar-stars.mjs"; +import { + decodeSidebarPreferences, + SIDEBAR_REQUEST_BYTES, +} from "./sidebar-preferences.mjs"; + +function harness() { + const secret = generateSecretKey(); + const viewer = getPublicKey(secret); + return { + secret, + viewer, + encrypt(channels, overrides = {}) { + const key = nip44.v2.utils.getConversationKey(secret, viewer); + try { + return finalizeEvent( + { + kind: 30078, + created_at: 100, + tags: [["d", "channel-stars"]], + content: nip44.v2.encrypt( + JSON.stringify({ version: 1, channels }), + key, + ), + ...overrides, + }, + secret, + ); + } finally { + key.fill(0); + } + }, + }; +} +it("rejects invalid intent shapes before relay reads", async () => { + const h = harness(); + for (const intent of [ + null, + [], + {}, + { channelId: "", starred: true }, + { channelId: "a" }, + { channelId: "a", starred: 1 }, + { channelId: "x".repeat(257), starred: true }, + { channelId: "a", starred: true, extra: 1 }, + ]) + expect(() => assertSidebarStarIntent(intent)).toThrow( + "Invalid sidebar star intent", + ); + const read = vi.fn(); + await expect(mutateSidebarStar({}, h.secret, read, vi.fn())).rejects.toThrow( + "Invalid sidebar star intent", + ); + expect(read).not.toHaveBeenCalled(); +}); +it("encrypts explicit Star/Unstar with monotonic timestamps and preserves unrelated tombstones", () => { + const h = harness(); + const channels = { + alpha: { starred: false, updatedAt: 60000 }, + beta: { starred: true, updatedAt: 2 }, + gone: { starred: false, updatedAt: 3 }, + }; + const added = prepareSidebarStar( + [h.encrypt(channels)], + { channelId: "alpha", starred: true }, + h.secret, + 50000, + ); + expect(verifyEvent(added.event)).toBe(true); + expect(added.event).toMatchObject({ + pubkey: h.viewer, + kind: 30078, + created_at: 101, + tags: [ + ["d", "channel-stars"], + ["t", "channel-stars"], + ], + }); + expect(added.event.content).not.toContain("alpha"); + expect(added.stars.channels).toEqual({ + ...channels, + alpha: { starred: true, updatedAt: 60001 }, + }); + expect(decodeSidebarPreferences([added.event], h.secret).starred).toEqual([ + "alpha", + "beta", + ]); + const removed = prepareSidebarStar( + [added.event], + { channelId: "alpha", starred: false }, + h.secret, + 50000, + ); + expect(removed.stars.channels).toEqual({ + ...channels, + alpha: { starred: false, updatedAt: 60002 }, + }); + expect(decodeSidebarPreferences([removed.event], h.secret).starred).toEqual([ + "beta", + ]); + expect( + prepareSidebarStar( + [removed.event], + { channelId: "alpha", starred: false }, + h.secret, + ).event, + ).toBeUndefined(); + expect( + prepareSidebarStar( + [], + { channelId: "new", starred: false }, + h.secret, + 50000, + ).stars.channels, + ).toEqual({ new: { starred: false, updatedAt: 50000 } }); +}); +it("refuses untrusted, ambiguous, malformed and over-budget heads rather than seeding", () => { + const h = harness(), + other = harness(); + const intent = { channelId: "alpha", starred: true }; + const valid = h.encrypt({}); + for (const events of [ + null, + [other.encrypt({})], + [valid, valid], + [{ ...JSON.parse(JSON.stringify(valid)), sig: "0".repeat(128) }], + [h.encrypt({}, { tags: [["d", "channel-sections"]] })], + [ + h.encrypt( + {}, + { + tags: [ + ["d", "channel-stars"], + ["d", "channel-stars"], + ], + }, + ), + ], + [h.encrypt({ alpha: { starred: true, updatedAt: -1 } })], + [h.encrypt({}, { content: "x".repeat(SIDEBAR_REQUEST_BYTES) })], + ]) + expect(() => prepareSidebarStar(events, intent, h.secret)).toThrow(); + const full = Object.fromEntries( + Array.from({ length: 500 }, (_, i) => [ + `id-${i}`, + { starred: false, updatedAt: 1 }, + ]), + ); + expect(() => prepareSidebarStar([h.encrypt(full)], intent, h.secret)).toThrow( + "budget exceeded", + ); +}); +it("confirms fresh retained state, including newer unrelated entries, and does not publish no-ops", async () => { + const h = harness(); + let heads = []; + const read = vi.fn(async () => heads); + const publish = vi.fn(async () => { + heads = [ + h.encrypt({ + alpha: { starred: true, updatedAt: 1 }, + beta: { starred: true, updatedAt: 2 }, + }), + ]; + }); + const intent = { channelId: "alpha", starred: true }; + expect( + (await mutateSidebarStar(intent, h.secret, read, publish)).channels, + ).toHaveProperty("beta"); + expect(read).toHaveBeenCalledTimes(2); + expect(publish).toHaveBeenCalledOnce(); + await mutateSidebarStar(intent, h.secret, read, publish); + expect(publish).toHaveBeenCalledOnce(); +}); +it("does not report success on read/publish failures or conflicting confirmation", async () => { + const h = harness(), + intent = { channelId: "alpha", starred: true }; + const publish = vi.fn(); + await expect( + mutateSidebarStar( + intent, + h.secret, + async () => { + throw new Error("read failed"); + }, + publish, + ), + ).rejects.toThrow("read failed"); + expect(publish).not.toHaveBeenCalled(); + const read = vi.fn(async () => []); + await expect( + mutateSidebarStar(intent, h.secret, read, async () => { + throw new Error("publish failed"); + }), + ).rejects.toThrow("publish failed"); + expect(read).toHaveBeenCalledOnce(); + await expect( + mutateSidebarStar(intent, h.secret, read, publish), + ).rejects.toThrow("changed on another device"); +}); diff --git a/docs/channels.md b/docs/channels.md index c969f6cb..574f538e 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -72,6 +72,29 @@ late completion cannot repopulate a retired snapshot. These are account-owned preferences, not channel access grants: sidebar sections still intersect the authorized roster. There is no new disk cache or automatic cross-device sync. +The browser/development host exposes narrow **assign/remove group** and +**Star/Unstar** commands. Each re-reads the viewer's signed encrypted coordinate, +changes only the requested entry, publishes through the shared relay admission +lane, then re-reads to confirm the requested state. Unrelated fields and explicit +unstar tombstones are retained. Invalid/unreadable/over-budget heads fail closed; +only a successful absent-head read can seed a coordinate. Same-host writes are +serialized per relay. This is confirmed whole-record replacement, not atomic +cross-device merging, a durable pending outbox, or automatic retry: simultaneous +writers on different hosts can still race. Failure leaves the last confirmed UI +state and offers an explicit retry; a failed confirmation may follow a publication +that reached the relay. + +The session preference owner serializes local commands and fences refreshes, +caller cancellation, cache clear and disposal. `session.ts` only composes host +capabilities with session lifetime and a bounded deadline. Cache clear cancels +pending work but cannot retract a publication already accepted by the relay. +Stream rows expose these actions by right-click/long-press, Shift+F10 or the +Context Menu key. Menus remain open during saving and failed-save retry; confirmed +relocation expands the destination and restores focus by channel identity. Starred +placement is exclusive, retaining the saved assignment so Unstar restores it. +Forums/DMs, group CRUD/reorder and independent sorting are outside this slice. +Hosts without the write capabilities retain the read-only projection. + Search, collapsed section keys and sidebar scroll remain separate, scoped view intent. They are saved on page exit and restored before paint when the roster and groups are available; navigation history does not own them. The saved-groups diff --git a/src/bundled/channels/Channels.module.css b/src/bundled/channels/Channels.module.css index b36a01f7..b15d621e 100644 --- a/src/bundled/channels/Channels.module.css +++ b/src/bundled/channels/Channels.module.css @@ -152,6 +152,29 @@ color: var(--text); font-weight: var(--type-weight-medium); } +.channelRow { + position: relative; + display: flex; + align-items: center; + border-radius: var(--radius-row); +} +.channelRow .channelLink { + min-width: 0; + flex: 1; +} +.channelRow:has(.channelLink:hover), +.channelRow:has(.channelLink[aria-current="page"]), +.channelRow[data-menu-open="true"] { + background: var(--surface-hover); +} +.channelRow .channelLink:hover, +.channelRow .channelLink[aria-current="page"] { + background: transparent; +} +.channelRow:has(.channelLink[aria-current="page"]) { + background: var(--surface-accent); +} + .preferenceNotice { padding: var(--space-2); color: var(--text-muted); diff --git a/src/bundled/channels/ChannelsPage.tsx b/src/bundled/channels/ChannelsPage.tsx index 97a6c53b..1bfb9b82 100644 --- a/src/bundled/channels/ChannelsPage.tsx +++ b/src/bundled/channels/ChannelsPage.tsx @@ -22,13 +22,14 @@ import { import { Hash, Search, - MoreHorizontal, PlugZap, MessageCircle, + MoreHorizontal, Users, } from "lucide-react"; import type { RelayData } from "../../features/relay/service"; import type { RelaySession } from "../../features/relay/session"; +import type { ChannelSummary } from "../../features/relay/contracts"; import { useChannelList, useChannelWindow, @@ -48,6 +49,18 @@ import { useChannelLabels } from "./useChannelLabels"; import { useSidebarPreferences } from "./useSidebarPreferences"; import { useSidebarView } from "./useSidebarView"; import { sidebarSections } from "./sidebar-sections"; +import { + ContextMenuRoot, + ContextMenuTrigger, + MenuGroup, + MenuGroupLabel, + MenuIcon, + MenuItem, + MenuPopup, + MenuRadioGroup, + MenuRadioItem, + MenuSeparator, +} from "../../shared/design-system/ui/Menu"; import styles from "./Channels.module.css"; export function ChannelsPage({ @@ -179,6 +192,21 @@ function ChannelWorkspace({ messageId: string; }>(); const threadTrigger = useRef(null); + const [rowMenu, setRowMenu] = useState<{ + channel: ChannelSummary; + sectionId?: string; + anchor?: HTMLElement; + }>(); + const rowMenuGeneration = useRef(0); + const [groupWrite, setGroupWrite] = useState<{ + channelId: string; + pending: boolean; + error?: string; + }>(); + const [rowFocus, setRowFocus] = useState<{ + channelId: string; + sectionKey: string; + }>(); const [sent, setSent] = useState<{ channelId: string; id: string }>(); const sidebar = useSidebarView( scope, @@ -474,6 +502,77 @@ function ChannelWorkspace({ ), [channels, search], ); + const closeRowMenu = useCallback(() => { + rowMenuGeneration.current++; + setRowMenu(undefined); + setGroupWrite(undefined); + }, []); + useLayoutEffect(() => { + if (!rowFocus) return; + const destination = sidebar.list.current?.querySelector( + `[data-sidebar-section="${CSS.escape(rowFocus.sectionKey)}"]`, + ); + const link = destination?.querySelector( + `[data-channel-id="${CSS.escape(rowFocus.channelId)}"]`, + ); + link?.focus({ preventScroll: true }); + setRowFocus(undefined); + }, [rowFocus, sidebar.list]); + const openRowMenu = useCallback( + (channel: ChannelSummary, sectionId?: string, anchor?: HTMLElement) => { + rowMenuGeneration.current++; + setGroupWrite(undefined); + setRowMenu({ + channel, + ...(sectionId ? { sectionId } : {}), + ...(anchor ? { anchor } : {}), + }); + }, + [], + ); + const assignGroup = async (channelId: string, sectionId?: string) => { + const generation = rowMenuGeneration.current; + setGroupWrite({ channelId, pending: true }); + try { + await preferences.assign(channelId, sectionId); + if (generation !== rowMenuGeneration.current) return; + const sectionKey = sectionId ? `group:${sectionId}` : "channels"; + sidebar.toggle(sectionKey, true); + setRowFocus({ channelId, sectionKey }); + closeRowMenu(); + } catch (error) { + if (generation !== rowMenuGeneration.current) return; + setGroupWrite({ + channelId, + pending: false, + error: error instanceof Error ? error.message : String(error), + }); + } + }; + const setChannelStar = async (channelId: string, starred: boolean) => { + const generation = rowMenuGeneration.current; + setGroupWrite({ channelId, pending: true }); + try { + await preferences.setStar(channelId, starred); + if (generation !== rowMenuGeneration.current) return; + const sectionId = preferences.data?.assignments[channelId]; + const sectionKey = starred + ? "starred" + : preferences.data?.sections.some((group) => group.id === sectionId) + ? `group:${sectionId}` + : "channels"; + sidebar.toggle(sectionKey, true); + setRowFocus({ channelId, sectionKey }); + closeRowMenu(); + } catch (error) { + if (generation !== rowMenuGeneration.current) return; + setGroupWrite({ + channelId, + pending: false, + error: error instanceof Error ? error.message : String(error), + }); + } + }; return (
sidebar.toggle(section.key, event.currentTarget.open) } > - {section.icon && ( - - )} - {section.title} + + {section.icon && ( + + )} + {section.title} + {section.rows.map((channel) => { const Icon = @@ -511,10 +613,29 @@ function ChannelWorkspace({ ? Users : MessageCircle : Hash; - return ( + const currentSectionId = section.key.startsWith("group:") + ? section.key.slice("group:".length) + : undefined; + const movable = + preferences.writable && + section.key !== "starred" && + channel.channelType !== "dm" && + channel.channelType !== "forum" && + !!preferences.data?.sections.length; + const starrable = + preferences.starWritable && + !!preferences.data && + channel.channelType !== "dm" && + channel.channelType !== "forum"; + const starred = section.key === "starred"; + const menuOpen = + (movable || starrable) && + rowMenu?.channel.id === channel.id && + rowMenu.sectionId === currentSectionId; + const channelButton = ( ); + if (!movable && !starrable) { + return ( +
+ {channelButton} +
+ ); + } + return ( + { + if (open) openRowMenu(channel, currentSectionId); + else if (menuOpen) closeRowMenu(); + }} + > + { + if ( + event.key === "ContextMenu" || + (event.shiftKey && event.key === "F10") + ) { + event.preventDefault(); + openRowMenu( + channel, + currentSectionId, + event.currentTarget, + ); + } + }} + render={ +
+ } + > + {channelButton} + + + sidebar.list.current?.querySelector( + `[data-channel-id="${CSS.escape(channel.id)}"]`, + ) ?? false + } + > + {starrable && ( + + void setChannelStar(channel.id, !starred) + } + > + {starred ? "Unstar" : "Star"} + + )} + {movable && ( + <> + {starrable && } + + Move to group + + + void assignGroup(channel.id, sectionId) + } + disabled={ + groupWrite?.channelId === channel.id && + groupWrite.pending + } + > + {preferences.data?.sections.map((group) => ( + + {group.icon && ( + {group.icon} + )} + {group.name} + + ))} + + {currentSectionId && ( + <> + + void assignGroup(channel.id)} + > + Remove from group + + + )} + + )} + {groupWrite?.channelId === channel.id && + groupWrite.pending &&

Saving…

} + {groupWrite?.channelId === channel.id && + groupWrite.error && ( +

{groupWrite.error}

+ )} +
+ + ); })} ))} diff --git a/src/bundled/channels/sidebar-sections.test.ts b/src/bundled/channels/sidebar-sections.test.ts index d06d1171..b2a92927 100644 --- a/src/bundled/channels/sidebar-sections.test.ts +++ b/src/bundled/channels/sidebar-sections.test.ts @@ -49,3 +49,22 @@ it("intersects groups/stars with active authorized streams, keeping forums and D ), ).toEqual(["star", "work", "other", "forum", "dm", "group-dm"]); }); +it("Star placement is exclusive and Unstar restores the saved assignment", () => { + const channels = [row("alpha"), row("beta")]; + const saved = { + sections: [{ id: "work", name: "Work", order: 0 }], + assignments: { beta: "work" }, + starred: ["alpha", "beta"], + }; + const placements = (starred: string[]) => + sidebarSections(channels, { ...saved, starred }).map((section) => [ + section.key, + section.rows.map((channel) => channel.id), + ]); + expect(placements(saved.starred)).toEqual([["starred", ["alpha", "beta"]]]); + expect(placements(["alpha"])).toEqual([ + ["starred", ["alpha"]], + ["group:work", ["beta"]], + ]); + expect(saved.assignments).toEqual({ beta: "work" }); +}); diff --git a/src/bundled/channels/useSidebarPreferences.ts b/src/bundled/channels/useSidebarPreferences.ts index 49577106..011463d6 100644 --- a/src/bundled/channels/useSidebarPreferences.ts +++ b/src/bundled/channels/useSidebarPreferences.ts @@ -16,6 +16,10 @@ export function useSidebarPreferences( return { ...snapshot, status: snapshot.status === "idle" ? ("loading" as const) : snapshot.status, + writable: queries.writable, + assign: queries.assign, + starWritable: queries.starWritable, + setStar: queries.setStar, reload: queries.refresh, }; } diff --git a/src/features/relay/session.ts b/src/features/relay/session.ts index e529ca20..1ce1c884 100644 --- a/src/features/relay/session.ts +++ b/src/features/relay/session.ts @@ -626,6 +626,34 @@ export function createRelaySession( ); }, !!transport?.decodeSidebarPreferences, + (() => { + const write = transport?.writeSidebarAssignment; + return write + ? (intent, signal) => + write( + intent, + AbortSignal.any([ + lifetime.signal, + AbortSignal.timeout(20_000), + signal, + ]), + ) + : undefined; + })(), + (() => { + const write = transport?.writeSidebarStar; + return write + ? (intent, signal) => + write( + intent, + AbortSignal.any([ + lifetime.signal, + AbortSignal.timeout(20_000), + signal, + ]), + ) + : undefined; + })(), notify, ); const session = Object.freeze({ diff --git a/src/features/relay/sidebar-preferences-store.test.ts b/src/features/relay/sidebar-preferences-store.test.ts index 0aedbb89..b827ba4b 100644 --- a/src/features/relay/sidebar-preferences-store.test.ts +++ b/src/features/relay/sidebar-preferences-store.test.ts @@ -1,22 +1,163 @@ import { expect, it, vi } from "vitest"; import { createRelaySession } from "./session"; import { flush, keypair, scriptedTransport } from "./testing"; -import type { SidebarPreferences } from "./sidebar-preferences"; +import type { + SidebarAssignmentMutator, + SidebarStarMutator, + SidebarPreferences, +} from "./sidebar-preferences"; + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((yes, no) => { + resolve = yes; + reject = no; + }); + return { promise, resolve, reject }; +} const data: SidebarPreferences = { sections: [{ id: "work", name: "Work", order: 0 }], assignments: { alpha: "work" }, starred: ["beta"], }; -function setup(decode = vi.fn(async (): Promise => data)) { +function setup( + decode = vi.fn(async (): Promise => data), + write?: SidebarAssignmentMutator, + writeStar?: SidebarStarMutator, +) { const wire = scriptedTransport(keypair().pubkey, keypair().pubkey); const owner = createRelaySession({ ...wire.transport, decodeSidebarPreferences: decode, + ...(write ? { writeSidebarAssignment: write } : {}), + ...(writeStar ? { writeSidebarStar: writeStar } : {}), }); return { wire, owner, preferences: owner.session.sidebarPreferences, decode }; } +it("applies a confirmed assignment to the retained session snapshot", async () => { + const write = vi.fn(async () => ({ + sections: [ + { id: "work", name: "Work", order: 0 }, + { id: "later", name: "Later", order: 1 }, + ], + assignments: { alpha: "later" }, + })); + const { wire, owner, preferences } = setup(undefined, write); + try { + const initial = preferences.ensure(); + await flush(); + wire.next().respond([]); + await initial; + const listener = vi.fn(); + preferences.subscribe(listener); + await expect(preferences.assign("alpha", "later")).resolves.toMatchObject({ + assignments: { alpha: "later" }, + }); + expect(write).toHaveBeenCalledWith( + { channelId: "alpha", sectionId: "later" }, + expect.any(AbortSignal), + ); + expect(preferences.snapshot()).toEqual({ + status: "ready", + data: { + sections: [ + { id: "work", name: "Work", order: 0 }, + { id: "later", name: "Later", order: 1 }, + ], + assignments: { alpha: "later" }, + starred: ["beta"], + }, + }); + expect(Object.isFrozen(preferences.snapshot().data?.assignments)).toBe( + true, + ); + expect(listener).toHaveBeenCalledOnce(); + } finally { + owner.dispose(); + } +}); + +it("keeps the last confirmed snapshot when an assignment fails", async () => { + const write = vi.fn(async () => { + throw new Error("relay rejected write"); + }); + const { wire, owner, preferences } = setup(undefined, write); + try { + const initial = preferences.ensure(); + await flush(); + wire.next().respond([]); + await initial; + const retained = preferences.snapshot(); + await expect(preferences.assign("alpha")).rejects.toThrow( + "relay rejected write", + ); + expect(preferences.snapshot()).toBe(retained); + } finally { + owner.dispose(); + } +}); + +it("does not let an older refresh overwrite a confirmed assignment", async () => { + let resolveDecode!: (value: SidebarPreferences) => void; + const decode = vi.fn( + async () => + new Promise((resolve) => { + resolveDecode = resolve; + }), + ); + const write = vi.fn(async () => ({ + sections: [{ id: "work", name: "Work", order: 0 }], + assignments: { alpha: "work" }, + })); + const { wire, owner, preferences } = setup(decode, write); + try { + decode.mockResolvedValueOnce(data); + const initial = preferences.ensure(); + await flush(); + wire.next().respond([]); + await initial; + const refresh = preferences.refresh(); + await flush(); + wire.next().respond([]); + await flush(); + await preferences.assign("alpha", "work"); + resolveDecode({ ...data, assignments: {} }); + await refresh; + expect(preferences.snapshot().data?.assignments).toEqual({ alpha: "work" }); + } finally { + owner.dispose(); + } +}); + +it("rejects queued assignment results after session cache clear", async () => { + let resolveWrite!: ( + value: Awaited>, + ) => void; + const write = vi.fn( + async () => + new Promise((resolve) => { + resolveWrite = resolve; + }), + ); + const { wire, owner, preferences } = setup(undefined, write); + const initial = preferences.ensure(); + await flush(); + wire.next().respond([]); + await initial; + const pending = preferences.assign("alpha", "work"); + await flush(); + await owner.clearCache(); + resolveWrite({ + sections: [{ id: "work", name: "Work", order: 0 }], + assignments: { alpha: "work" }, + }); + await expect(pending).rejects.toThrow("unavailable"); + expect(preferences.snapshot()).toEqual({ status: "idle" }); + owner.dispose(); +}); it("one session retains groups across observers and deduplicates initial reads", async () => { const { wire, owner, preferences, decode } = setup(); try { @@ -164,3 +305,170 @@ it.each(["clearCache", "dispose"] as const)( } }, ); + +it("serializes confirmed assignment and star writes without losing either projection", async () => { + const gate = deferred(); + const started = deferred(); + const star = vi.fn(async () => { + started.resolve(); + return gate.promise; + }); + const assign = vi.fn(async () => ({ + sections: data.sections, + assignments: { alpha: "work", beta: "work" }, + })); + const { wire, owner, preferences } = setup(undefined, assign, star); + try { + const initial = preferences.ensure(); + await flush(); + wire.next().respond([]); + await initial; + const pending = preferences.setStar("alpha", true); + await started.promise; + const queued = preferences.assign("beta", "work"); + expect(preferences.snapshot().data).toEqual(data); + expect(assign).not.toHaveBeenCalled(); + gate.resolve(["alpha", "beta"]); + await Promise.all([pending, queued]); + expect(preferences.snapshot()).toEqual({ + status: "ready", + data: { + ...data, + assignments: { alpha: "work", beta: "work" }, + starred: ["alpha", "beta"], + }, + }); + expect(Object.isFrozen(preferences.snapshot().data?.starred)).toBe(true); + expect(star).toHaveBeenCalledWith( + { channelId: "alpha", starred: true }, + expect.any(AbortSignal), + ); + } finally { + gate.resolve([]); + owner.dispose(); + } +}); + +it("failed Star retains the confirmed snapshot and a retry can unstar", async () => { + const star = vi + .fn() + .mockRejectedValueOnce(new Error("publish rejected")) + .mockResolvedValueOnce([]); + const { wire, owner, preferences } = setup(undefined, undefined, star); + try { + const initial = preferences.ensure(); + await flush(); + wire.next().respond([]); + await initial; + const retained = preferences.snapshot(); + await expect(preferences.setStar("beta", false)).rejects.toThrow( + "publish rejected", + ); + expect(preferences.snapshot()).toBe(retained); + await preferences.setStar("beta", false); + expect(preferences.snapshot().data).toEqual({ ...data, starred: [] }); + } finally { + owner.dispose(); + } +}); + +it.each(["success", "failure"])( + "a stale refresh %s cannot overwrite confirmed Star", + async (outcome) => { + const gate = deferred(); + const started = deferred(); + const decode = vi + .fn(async () => data) + .mockImplementationOnce(async () => data); + const { wire, owner, preferences } = setup(decode, undefined, async () => [ + "alpha", + "beta", + ]); + try { + const initial = preferences.ensure(); + await flush(); + wire.next().respond([]); + await initial; + decode.mockImplementationOnce(() => { + started.resolve(); + return gate.promise; + }); + const refresh = preferences.refresh(); + await flush(); + wire.next().respond([]); + await started.promise; + await preferences.setStar("alpha", true); + const retained = preferences.snapshot(); + if (outcome === "success") gate.resolve(data); + else gate.reject(new Error("old read failed")); + await refresh; + expect(preferences.snapshot()).toBe(retained); + expect(retained.status).toBe("ready"); + expect(retained.data?.starred).toEqual(["alpha", "beta"]); + } finally { + gate.resolve(data); + owner.dispose(); + } + }, +); + +it.each(["clearCache", "dispose", "cancel"] as const)( + "%s aborts Star and fences active and queued writes", + async (action) => { + const gate = deferred(); + const started = deferred(); + const star = vi.fn(async (_intent, signal) => { + started.resolve(signal); + return gate.promise; + }); + const { wire, owner, preferences } = setup(undefined, undefined, star); + const caller = new AbortController(); + try { + const initial = preferences.ensure(); + await flush(); + wire.next().respond([]); + await initial; + const pending = preferences.setStar("alpha", true, caller.signal); + const activeSignal = await started.promise; + const queued = preferences.setStar("beta", false, caller.signal); + const result = Promise.allSettled([pending, queued]); + if (action === "cancel") caller.abort(); + else await owner[action](); + expect(activeSignal.aborted).toBe(true); + gate.resolve(["alpha", "beta"]); + expect((await result).map((entry) => entry.status)).toEqual([ + "rejected", + "rejected", + ]); + expect(star).toHaveBeenCalledOnce(); + expect(preferences.snapshot().data).toEqual( + action === "cancel" ? data : undefined, + ); + } finally { + gate.resolve([]); + owner.dispose(); + } + }, +); + +it("does not mutate before a successful initial preference read or without host capability", async () => { + const star = vi.fn(async () => []); + const assign = vi.fn(async () => data); + const { owner, preferences } = setup(undefined, assign, star); + try { + await expect(preferences.setStar("alpha", true)).rejects.toThrow( + "unavailable", + ); + await expect(preferences.assign("alpha", "work")).rejects.toThrow(); + expect(star).not.toHaveBeenCalled(); + expect(assign).not.toHaveBeenCalled(); + } finally { + owner.dispose(); + } + const readonly = setup(); + try { + expect(readonly.preferences.starWritable).toBe(false); + } finally { + readonly.owner.dispose(); + } +}); diff --git a/src/features/relay/sidebar-preferences-store.ts b/src/features/relay/sidebar-preferences-store.ts index 5f4aad64..b6fe42c1 100644 --- a/src/features/relay/sidebar-preferences-store.ts +++ b/src/features/relay/sidebar-preferences-store.ts @@ -1,4 +1,8 @@ -import type { SidebarPreferences } from "./sidebar-preferences"; +import type { + SidebarAssignmentMutator, + SidebarStarMutator, + SidebarPreferences, +} from "./sidebar-preferences"; type Snapshot = Readonly<{ status: "idle" | "loading" | "ready" | "error" | "unsupported"; @@ -10,6 +14,8 @@ type Snapshot = Readonly<{ export function createSidebarPreferencesStore( read: (signal?: AbortSignal) => Promise, available: boolean, + write?: SidebarAssignmentMutator, + writeStar?: SidebarStarMutator, notify = (listener: () => void) => listener(), ) { const listeners = new Set<() => void>(); @@ -20,6 +26,18 @@ export function createSidebarPreferencesStore( let active: | { controller: AbortController; promise: Promise } | undefined; + let writeQueue = Promise.resolve(); + let writeLifetime = new AbortController(); + let mutation = 0; + let generation = 0; + const retained = (data: SidebarPreferences): SidebarPreferences => + Object.freeze({ + sections: Object.freeze( + data.sections.map((section) => Object.freeze({ ...section })), + ), + assignments: Object.freeze({ ...data.assignments }), + starred: Object.freeze([...data.starred]), + }); const publish = (next: Snapshot) => { snapshot = Object.freeze(next); for (const listener of listeners) notify(listener); @@ -28,25 +46,28 @@ export function createSidebarPreferencesStore( if (closed || !available) return Promise.resolve(); if (active) return active.promise; const controller = new AbortController(); + const refreshMutation = mutation; const job = { controller, promise: Promise.resolve() }; active = job; job.promise = Promise.resolve().then(async () => { if (closed || controller.signal.aborted) return; try { const data = await read(controller.signal); - if (closed || controller.signal.aborted || active !== job) return; - publish({ - status: "ready", - data: Object.freeze({ - sections: Object.freeze( - data.sections.map((section) => Object.freeze({ ...section })), - ), - assignments: Object.freeze({ ...data.assignments }), - starred: Object.freeze([...data.starred]), - }), - }); + if ( + closed || + controller.signal.aborted || + active !== job || + mutation !== refreshMutation + ) + return; + publish({ status: "ready", data: retained(data) }); } catch (error) { - if (!closed && !controller.signal.aborted && active === job) + if ( + !closed && + !controller.signal.aborted && + active === job && + mutation === refreshMutation + ) publish({ ...snapshot, status: "error", @@ -65,6 +86,84 @@ export function createSidebarPreferencesStore( return { queries: Object.freeze({ available, + writable: !!write, + assign(channelId: string, sectionId?: string, signal?: AbortSignal) { + if (closed || !write || !snapshot.data) + return Promise.reject( + new Error("Saved sidebar groups are read-only in this host"), + ); + const writeGeneration = generation; + const writeSignal = AbortSignal.any([ + writeLifetime.signal, + ...(signal ? [signal] : []), + ]); + const run = writeQueue + .catch(() => {}) + .then(async () => { + if (closed || generation !== writeGeneration) + throw new Error("Saved sidebar groups are unavailable"); + writeSignal.throwIfAborted(); + const groups = await write( + { channelId, ...(sectionId ? { sectionId } : {}) }, + writeSignal, + ); + if (closed || generation !== writeGeneration) + throw new Error("Saved sidebar groups are unavailable"); + writeSignal.throwIfAborted(); + mutation++; + const current = snapshot.data; + publish({ + status: "ready", + data: retained({ + sections: groups.sections, + assignments: groups.assignments, + starred: current?.starred ?? [], + }), + }); + return groups; + }); + writeQueue = run.then( + () => undefined, + () => undefined, + ); + return run; + }, + starWritable: !!writeStar, + setStar(channelId: string, starred: boolean, signal?: AbortSignal) { + if (closed || !writeStar || !snapshot.data) + return Promise.reject( + new Error("Sidebar stars are unavailable in this host"), + ); + const writeGeneration = generation; + const writeSignal = AbortSignal.any([ + writeLifetime.signal, + ...(signal ? [signal] : []), + ]); + const run = writeQueue + .catch(() => {}) + .then(async () => { + if (closed || generation !== writeGeneration) + throw new Error("Sidebar stars are unavailable"); + writeSignal.throwIfAborted(); + const stars = await writeStar({ channelId, starred }, writeSignal); + if (closed || generation !== writeGeneration) + throw new Error("Sidebar stars are unavailable"); + writeSignal.throwIfAborted(); + const current = snapshot.data; + if (!current) throw new Error("Sidebar stars are unavailable"); + mutation++; + publish({ + status: "ready", + data: retained({ ...current, starred: stars }), + }); + return stars; + }); + writeQueue = run.then( + () => undefined, + () => undefined, + ); + return run; + }, // Keep explicit one-shot reads compatible; views use the retained snapshot. read, snapshot: () => snapshot, @@ -83,12 +182,19 @@ export function createSidebarPreferencesStore( }), clear() { if (closed) return; + generation++; + mutation++; + writeLifetime.abort(); + writeLifetime = new AbortController(); active?.controller.abort(); active = undefined; publish(empty()); }, dispose() { closed = true; + writeLifetime.abort(); + generation++; + mutation++; active?.controller.abort(); active = undefined; snapshot = empty(); diff --git a/src/features/relay/sidebar-preferences.test.ts b/src/features/relay/sidebar-preferences.test.ts index dec5f2c1..96719f7e 100644 --- a/src/features/relay/sidebar-preferences.test.ts +++ b/src/features/relay/sidebar-preferences.test.ts @@ -15,6 +15,13 @@ import { projectSidebarPreferences, readSidebarPreferences, } from "./sidebar-preferences"; +import { + assertSidebarAssignmentIntent, + decodeSidebarPreferences, + mutateSidebarAssignment, + prepareSidebarAssignment, + SIDEBAR_REQUEST_BYTES, +} from "../../../dev/sidebar-preferences.mjs"; import { keypair, signed, scriptedTransport, flush, roster } from "./testing"; import type { LiveCallbacks } from "./live"; import { ReadError } from "./errors"; @@ -578,3 +585,170 @@ it.each([ } }, ); + +it("rejects invalid assignment intents before any relay work", () => { + for (const intent of [ + null, + [], + {}, + { channelId: "" }, + { channelId: "general", sectionId: "" }, + { channelId: "general", extra: true }, + ]) + expect(() => assertSidebarAssignmentIntent(intent)).toThrow( + "Invalid sidebar assignment intent", + ); +}); + +it("prepares one host-owned assignment without replacing unrelated groups", () => { + const viewer = keypair(); + const encrypt = (value: unknown, created_at = 100) => + signed(viewer, { + kind: 30078, + created_at, + tags: [["d", "channel-sections"]], + content: nip44.v2.encrypt( + JSON.stringify(value), + nip44.v2.utils.getConversationKey(viewer.secret, viewer.pubkey), + ), + }); + const head = encrypt({ + version: 1, + sections: [ + { id: "work", name: "Work", order: 0 }, + { id: "later", name: "Later", order: 1 }, + ], + assignments: { general: "work", random: "later" }, + }); + const moved = prepareSidebarAssignment( + [head], + { channelId: "general", sectionId: "later" }, + viewer.secret, + 50_000, + ); + expect(moved.groups.assignments).toEqual({ + general: "later", + random: "later", + }); + expect(moved.event).toBeDefined(); + if (!moved.event) throw new Error("Missing sidebar assignment event"); + expect(moved.event).toMatchObject({ + kind: 30078, + pubkey: viewer.pubkey, + created_at: 101, + tags: [ + ["d", "channel-sections"], + ["t", "channel-sections"], + ], + }); + expect(decodeSidebarPreferences([moved.event], viewer.secret)).toMatchObject({ + sections: [ + { id: "work", name: "Work", order: 0 }, + { id: "later", name: "Later", order: 1 }, + ], + assignments: { general: "later", random: "later" }, + }); + const removed = prepareSidebarAssignment( + [moved.event], + { channelId: "general" }, + viewer.secret, + 50_000, + ); + expect(removed.groups.assignments).toEqual({ random: "later" }); + expect(() => + prepareSidebarAssignment( + [head], + { channelId: "general", sectionId: "gone" }, + viewer.secret, + ), + ).toThrow("no longer exists"); + const same = prepareSidebarAssignment( + [head], + { channelId: "general", sectionId: "work" }, + viewer.secret, + ); + expect(same.event).toBeUndefined(); +}); + +it("applies decoder-parity bounds to the untrusted sidebar group head", () => { + const viewer = keypair(); + const event = signed(viewer, { + kind: 30078, + tags: [["d", "channel-sections"]], + content: "x".repeat(SIDEBAR_REQUEST_BYTES), + }); + expect(Buffer.byteLength(JSON.stringify([event]))).toBeGreaterThan( + SIDEBAR_REQUEST_BYTES, + ); + expect(() => + prepareSidebarAssignment([event], { channelId: "general" }, viewer.secret), + ).toThrow("Invalid sidebar group head"); +}); + +it("confirms the requested assignment while preserving newer unrelated assignments", async () => { + const viewer = keypair(); + const encrypt = (assignments: Record) => + signed(viewer, { + kind: 30078, + tags: [["d", "channel-sections"]], + content: nip44.v2.encrypt( + JSON.stringify({ + version: 1, + sections: [{ id: "work", name: "Work", order: 0 }], + assignments, + }), + nip44.v2.utils.getConversationKey(viewer.secret, viewer.pubkey), + ), + }); + const initial = encrypt({}); + let confirmation = [initial]; + let publishedAssignments: Readonly> = {}; + const result = await mutateSidebarAssignment( + { channelId: "general", sectionId: "work" }, + viewer.secret, + async () => confirmation, + async (event) => { + publishedAssignments = decodeSidebarPreferences( + [event], + viewer.secret, + ).assignments; + confirmation = [encrypt({ ...publishedAssignments, random: "work" })]; + }, + ); + expect(publishedAssignments).toEqual({ general: "work" }); + expect(result.assignments).toEqual({ general: "work", random: "work" }); +}); + +it("assignment writes retain unrelated raw fields and dangling assignments omitted from the view", () => { + const viewer = keypair(); + const key = nip44.v2.utils.getConversationKey(viewer.secret, viewer.pubkey); + try { + const blob = { + version: 1, + extra: "preserve", + sections: [{ id: "work", name: "Work", order: 0, color: "blue" }], + assignments: { old: "missing", random: "work" }, + }; + const head = signed(viewer, { + kind: 30078, + tags: [["d", "channel-sections"]], + content: nip44.v2.encrypt(JSON.stringify(blob), key), + }); + const result = prepareSidebarAssignment( + [head], + { channelId: "__proto__", sectionId: "work" }, + viewer.secret, + ); + if (!result.event) throw new Error("Expected assignment publication"); + expect(JSON.parse(nip44.v2.decrypt(result.event.content, key))).toEqual({ + ...blob, + assignments: { ...blob.assignments, ["__proto__"]: "work" }, + }); + expect(result.groups.assignments).toEqual({ + random: "work", + ["__proto__"]: "work", + }); + } finally { + key.fill(0); + } +}); diff --git a/src/features/relay/sidebar-preferences.ts b/src/features/relay/sidebar-preferences.ts index 1214d652..da1ec1f9 100644 --- a/src/features/relay/sidebar-preferences.ts +++ b/src/features/relay/sidebar-preferences.ts @@ -5,7 +5,7 @@ export const SIDEBAR_COORDINATES = [ "channel-sections", "channel-stars", ] as const; -export type SidebarPreferences = Readonly<{ +export type SidebarGroups = Readonly<{ sections: readonly Readonly<{ id: string; name: string; @@ -13,13 +13,27 @@ export type SidebarPreferences = Readonly<{ order: number; }>[]; assignments: Readonly>; - starred: readonly string[]; }>; +export type SidebarPreferences = SidebarGroups & + Readonly<{ + starred: readonly string[]; + }>; +export type SidebarAssignmentIntent = Readonly<{ + channelId: string; + sectionId?: string; +}>; +export type SidebarAssignmentMutator = ( + intent: SidebarAssignmentIntent, + signal: AbortSignal, +) => Promise; +export type SidebarStarMutator = ( + intent: Readonly<{ channelId: string; starred: boolean }>, + signal: AbortSignal, +) => Promise; export type SidebarDecoder = ( events: readonly RelayEvent[], signal: AbortSignal, ) => Promise; - function object(value: unknown): Record { if (!value || typeof value !== "object" || Array.isArray(value)) throw new Error("Invalid sidebar preferences"); diff --git a/src/features/relay/transport.ts b/src/features/relay/transport.ts index 42c9b321..d2736bb6 100644 --- a/src/features/relay/transport.ts +++ b/src/features/relay/transport.ts @@ -8,7 +8,13 @@ import { readSnapshotText, } from "./read-state-snapshot"; import type { AgentLibraryReader } from "../agents/library"; -import type { SidebarDecoder, SidebarPreferences } from "./sidebar-preferences"; +import { + projectSidebarPreferences, + type SidebarAssignmentMutator, + type SidebarStarMutator, + type SidebarDecoder, + type SidebarPreferences, +} from "./sidebar-preferences"; import { createHostAdmission } from "./host-admission"; import { relayOrigin } from "../communities/destination"; import { @@ -55,6 +61,9 @@ export interface ReadTransport { requestId: string, priority: "foreground" | "background", ): Promise; + /** Host-only, relay-scoped mutation of one existing sidebar group assignment. */ + readonly writeSidebarAssignment?: SidebarAssignmentMutator; + readonly writeSidebarStar?: SidebarStarMutator; readonly profiling?: RelayProfiler; /** Verified incoming traffic. The session owns this subscription and fences late delivery. */ subscribe?(callbacks: LiveCallbacks): LiveSubscription; @@ -162,6 +171,8 @@ export async function connectBrokerTransport( relayUrl?: string; live?: boolean; sidebarPreferences?: boolean; + sidebarPreferenceWrites?: boolean; + sidebarStarWrites?: boolean; agentLibrary?: boolean; agentActivity?: boolean; readState?: boolean; @@ -318,6 +329,53 @@ export async function connectBrokerTransport( }, } : {}), + ...(session.sidebarPreferenceWrites + ? { + async writeSidebarAssignment(intent, signal) { + const result = await fetch(`${endpoint}/sidebar-assignment`, { + method: "POST", + credentials: "same-origin", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(intent), + signal, + }); + if (!result.ok) { + const failure = await readApiFailure(result); + throw new Error(failure.error); + } + const value = (await result.json()) as SidebarPreferences; + const groups = projectSidebarPreferences( + { + version: 1, + sections: value.sections, + assignments: value.assignments, + }, + undefined, + ); + return { + sections: groups.sections, + assignments: groups.assignments, + }; + }, + } + : {}), + ...(session.sidebarStarWrites + ? { + async writeSidebarStar(intent, signal) { + const result = await fetch(`${endpoint}/sidebar-star`, { + method: "POST", + credentials: "same-origin", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(intent), + signal, + }); + if (!result.ok) + throw new Error((await readApiFailure(result)).error); + return projectSidebarPreferences(undefined, await result.json()) + .starred; + }, + } + : {}), ...(session.writeKinds ? { writer: { diff --git a/tests/browser/fixture.mjs b/tests/browser/fixture.mjs index 3a265bf2..a5f0dbf6 100644 --- a/tests/browser/fixture.mjs +++ b/tests/browser/fixture.mjs @@ -591,21 +591,48 @@ export const test = base.extend({ answer, report, pending, - ...(readState + ...(readState || savedSidebar ? { - discovery: (community) => ({ - self: getPublicKey(relayKey), - read_state_snapshot: { - version: 1, - community_id: communityIds[community], - max_events: 4096, - max_bytes: 8388608, - }, - }), + ...(readState + ? { + discovery: (community) => ({ + self: getPublicKey(relayKey), + read_state_snapshot: { + version: 1, + community_id: communityIds[community], + max_events: 4096, + max_bytes: 8388608, + }, + }), + } + : {}), acceptPublication: (community, event) => { expect(verifyEvent(event)).toBe(true); expect(event.pubkey).toBe(viewer); expect(event.kind).toBe(30078); + const coordinate = event.tags.find( + ([key]) => key === "d", + )?.[1]; + if ( + ["channel-sections", "channel-stars"].includes(coordinate) + ) { + expect(event.tags).toContainEqual(["t", coordinate]); + const blob = JSON.parse( + nip44.v2.decrypt( + event.content, + nip44.v2.utils.getConversationKey(userKey, viewer), + ), + ); + readEvents.get(community).set(coordinate, event); + report.sidebarPublications ??= []; + report.sidebarPublications.push({ + community, + coordinate, + event, + blob, + }); + return; + } expect(event.tags).toContainEqual(["t", "read-state"]); const blob = JSON.parse( nip44.v2.decrypt( @@ -613,9 +640,6 @@ export const test = base.extend({ nip44.v2.utils.getConversationKey(userKey, viewer), ), ); - const coordinate = event.tags.find( - ([key]) => key === "d", - )?.[1]; expect(coordinate).toMatch(/^read-state:[0-9a-f]{32}$/); const previous = readEvents.get(community).get(coordinate); if ( @@ -998,10 +1022,26 @@ export const test = base.extend({ observerFailures.splice(match, 1); return true; }; + // The Star retry journey injects one specific failed host request. Match + // that exact URL once, not every 502 or every console error in the test. + const starFailures = [...(report.sidebarStarFailures ?? [])]; + const injectedStarFailure = (message, index) => { + if ( + !/^Failed to load resource: the server responded with a status of 502/.test( + message, + ) + ) + return false; + const match = starFailures.indexOf(consoleLocations.get(index)); + if (match < 0) return false; + starFailures.splice(match, 1); + return true; + }; expect( report.consoleErrors.filter( (message, index) => !retiredConsole(message, index) && + !injectedStarFailure(message, index) && !( expectedPageFailure && message.includes("Fixture page render failure") diff --git a/tests/browser/navigation-groups.spec.mjs b/tests/browser/navigation-groups.spec.mjs index 7417b265..6655de42 100644 --- a/tests/browser/navigation-groups.spec.mjs +++ b/tests/browser/navigation-groups.spec.mjs @@ -1,6 +1,185 @@ import { test, expect } from "./fixture.mjs"; import { open } from "./timeline.mjs"; +test.use({ productionBroker: true, savedSidebar: true }); + +test("row menu moves and removes a channel through the confirmed saved-group writer", async ({ + page, + app, +}) => { + await open(page, app); + const sidebar = page.getByRole("navigation", { + name: "Subscribed channels", + }); + const work = sidebar + .locator("details") + .filter({ has: page.locator("summary", { hasText: /^Work$/ }) }); + const channels = sidebar + .locator("details") + .filter({ has: page.locator("summary", { hasText: /^Channels$/ }) }); + await expect( + work.getByRole("button", { name: "Beta", exact: true }), + ).toBeVisible(); + + await work + .getByRole("button", { name: "Beta", exact: true }) + .click({ button: "right" }); + const menu = page.getByRole("menu", { name: "Actions for Beta" }); + await expect(menu).toBeVisible(); + await expect( + menu.getByRole("menuitemradio", { name: "Work" }), + ).toHaveAttribute("aria-checked", "true"); + await page.keyboard.press("End"); + await expect( + page.getByRole("menuitem", { name: "Remove from group" }), + ).toBeFocused(); + await page.keyboard.press("Home"); + await expect( + menu.getByRole("menuitem", { name: "Star", exact: true }), + ).toBeFocused(); + await page.keyboard.press("ArrowDown"); + await expect(menu.getByRole("menuitemradio", { name: "Work" })).toBeFocused(); + await page.keyboard.press("ArrowDown"); + await expect( + page.getByRole("menuitem", { name: "Remove from group" }), + ).toBeFocused(); + await page.keyboard.press("Enter"); + await expect( + channels.getByRole("button", { name: "Beta", exact: true }), + ).toBeVisible(); + await expect( + work.getByRole("button", { name: "Beta", exact: true }), + ).toHaveCount(0); + await expect( + channels.getByRole("button", { name: "Beta", exact: true }), + ).toBeFocused(); + expect(app.report.sidebarPublications).toHaveLength(1); + expect(app.report.sidebarPublications[0].blob.assignments).toEqual({}); + + await channels + .getByRole("button", { name: "Beta", exact: true }) + .click({ button: "right" }); + await page.getByRole("menuitemradio", { name: "Work" }).click(); + await expect( + work.getByRole("button", { name: "Beta", exact: true }), + ).toBeVisible(); + await expect( + channels.getByRole("button", { name: "Beta", exact: true }), + ).toHaveCount(0); + await expect( + work.getByRole("button", { name: "Beta", exact: true }), + ).toBeFocused(); + expect(app.report.sidebarPublications).toHaveLength(2); + expect(app.report.sidebarPublications[1].blob.assignments).toEqual({ + beta: "work", + }); + expect(app.report.unexpected).toEqual([]); +}); + +// Real context-menu keyboard/focus, viewport placement and row relocation require +// a browser. Intent validation, concurrency and persistence matrices stay below. +test("Star and Unstar retain the assigned group, keep one row, and recover from a failed save", async ({ + page, + app, +}, testInfo) => { + await page.addInitScript(() => + localStorage.setItem("buzz-appearance.v1", "dark"), + ); + await open(page, app); + const sidebar = page.getByRole("navigation", { name: "Subscribed channels" }); + const work = sidebar.locator('[data-sidebar-section="group:work"]'); + const starred = sidebar.locator('[data-sidebar-section="starred"]'); + const beta = work.getByRole("button", { name: "Beta", exact: true }); + await expect(beta).toBeVisible(); + await starred.locator("summary").click(); + await expect(starred).not.toHaveAttribute("open", ""); + await beta.focus(); + await page.keyboard.press("Shift+F10"); + const menu = page.getByRole("menu", { name: "Actions for Beta" }); + await expect(menu).toBeVisible(); + await menu.screenshot({ path: testInfo.outputPath("group-star-menu.png") }); + await page.keyboard.press("Escape"); + await expect(beta).toBeFocused(); + await page.keyboard.press("ContextMenu"); + await expect(menu).toBeVisible(); + let release; + const held = new Promise((resolve) => { + release = resolve; + }); + let started; + const requestStarted = new Promise((resolve) => { + started = resolve; + }); + await page.route("**/sidebar-star", async (route) => { + started(); + await held; + app.report.sidebarStarFailures ??= []; + app.report.sidebarStarFailures.push(route.request().url()); + await route.fulfill({ + status: 502, + contentType: "application/json", + body: JSON.stringify({ error: "Star save failed; retry" }), + }); + }); + try { + await menu.getByRole("menuitem", { name: "Star", exact: true }).click(); + await requestStarted; + await expect(menu.getByRole("status")).toHaveText("Saving…"); + await expect( + menu.getByRole("menuitem", { name: "Star", exact: true }), + ).toHaveAttribute("aria-disabled", "true"); + await expect( + page.locator( + '[data-sidebar-section="group:work"] [data-channel-id="beta"]', + ), + ).toBeVisible(); + } finally { + release(); + } + await expect(menu.getByRole("alert")).toHaveText( + "Relay request failed (502)", + ); + await expect( + menu.getByRole("menuitem", { name: "Star", exact: true }), + ).toBeEnabled(); + await page.unroute("**/sidebar-star"); + await page.keyboard.press("Home"); + await expect( + menu.getByRole("menuitem", { name: "Star", exact: true }), + ).toBeFocused(); + await page.keyboard.press("Enter"); + const relocated = starred.getByRole("button", { name: "Beta", exact: true }); + await expect(relocated).toBeVisible(); + await expect(relocated).toBeFocused(); + await sidebar.screenshot({ path: testInfo.outputPath("group-starred.png") }); + await expect(sidebar.locator('[data-channel-id="beta"]')).toHaveCount(1); + expect(app.report.sidebarPublications).toHaveLength(1); + expect(app.report.sidebarPublications[0]).toMatchObject({ + coordinate: "channel-stars", + blob: { channels: { alpha: { starred: true }, beta: { starred: true } } }, + }); + + // Work is absent while its only row is starred. Unstar must restore it. + await relocated.click({ button: "right" }); + await expect(menu).toBeVisible(); + const bounds = await menu.boundingBox(); + const viewport = page.viewportSize(); + expect(bounds.x).toBeGreaterThanOrEqual(0); + expect(bounds.y).toBeGreaterThanOrEqual(0); + expect(bounds.x + bounds.width).toBeLessThanOrEqual(viewport.width); + expect(bounds.y + bounds.height).toBeLessThanOrEqual(viewport.height); + await menu.getByRole("menuitem", { name: "Unstar", exact: true }).click(); + await expect(beta).toBeVisible(); + await expect(beta).toBeFocused(); + await expect(sidebar.locator('[data-channel-id="beta"]')).toHaveCount(1); + expect(app.report.sidebarPublications).toHaveLength(2); + expect(app.report.sidebarPublications[1]).toMatchObject({ + coordinate: "channel-stars", + blob: { channels: { alpha: { starred: true }, beta: { starred: false } } }, + }); + expect(app.report.unexpected).toEqual([]); +}); + test.use({ productionBroker: true, savedSidebar: true, From 0cbb69ab8e33b5ac11b1e97d503b45f660b1fd09 Mon Sep 17 00:00:00 2001 From: Carl Date: Thu, 17 Sep 2026 09:11:03 -1000 Subject: [PATCH 2/2] fix(channels): treat starred as a sidebar group Offer Starred and saved groups in one destination chooser. Confirm the destination assignment before clearing Star so removal returns to Channels without restoring a hidden group after reload. Retain confirmed local placement through partial failures, fence overlapping refreshes, and cover ordered writes, cancellation, retry and browser focus. Signed-off-by: Carl --- dev/sidebar-group-moves.test.mjs | 269 ++++++++++++++++++ dev/sidebar-stars.mjs | 3 +- dev/sidebar-stars.test.mjs | 2 +- docs/channels.md | 20 +- src/bundled/channels/ChannelsPage.tsx | 119 ++++---- .../relay/sidebar-preferences-store.test.ts | 25 +- .../relay/sidebar-preferences-store.ts | 169 ++++++----- tests/browser/navigation-groups.spec.mjs | 68 ++++- 8 files changed, 513 insertions(+), 162 deletions(-) create mode 100644 dev/sidebar-group-moves.test.mjs diff --git a/dev/sidebar-group-moves.test.mjs b/dev/sidebar-group-moves.test.mjs new file mode 100644 index 00000000..3c27fd87 --- /dev/null +++ b/dev/sidebar-group-moves.test.mjs @@ -0,0 +1,269 @@ +import { expect, it, vi } from "vitest"; +import { + finalizeEvent, + generateSecretKey, + getPublicKey, + nip44, +} from "nostr-tools"; +import { createSidebarPreferencesStore } from "../src/features/relay/sidebar-preferences-store.ts"; +import { sidebarSections } from "../src/bundled/channels/sidebar-sections.ts"; +import { + decodeSidebarPreferences, + mutateSidebarAssignment, +} from "./sidebar-preferences.mjs"; +import { mutateSidebarStar } from "./sidebar-stars.mjs"; + +async function setup({ cachedAssignment = true } = {}) { + const secret = generateSecretKey(); + const key = nip44.v2.utils.getConversationKey(secret, getPublicKey(secret)); + const heads = new Map(); + for (const [coordinate, blob] of [ + [ + "channel-sections", + { + version: 1, + sections: [{ id: "work", name: "Work", order: 0 }], + assignments: { alpha: "work", beta: "work" }, + }, + ], + [ + "channel-stars", + { version: 1, channels: { alpha: { starred: true, updatedAt: 1 } } }, + ], + ]) + heads.set( + coordinate, + finalizeEvent( + { + kind: 30078, + created_at: 1, + tags: [["d", coordinate]], + content: nip44.v2.encrypt(JSON.stringify(blob), key), + }, + secret, + ), + ); + key.fill(0); + const read = vi.fn(async () => + decodeSidebarPreferences([...heads.values()], secret), + ); + if (!cachedAssignment) + read.mockResolvedValueOnce({ + ...(await read()), + assignments: { beta: "work" }, + }); + const publications = []; + const publish = vi.fn(async (event) => { + const coordinate = event.tags.find(([tag]) => tag === "d")[1]; + heads.set(coordinate, event); + publications.push(coordinate); + }); + const assignment = vi.fn((intent, signal) => + mutateSidebarAssignment( + intent, + secret, + async () => { + signal.throwIfAborted(); + return [heads.get("channel-sections")]; + }, + publish, + ), + ); + const star = vi.fn(async (intent, signal) => { + const result = await mutateSidebarStar( + intent, + secret, + async () => { + signal.throwIfAborted(); + return [heads.get("channel-stars")]; + }, + publish, + ); + return Object.entries(result.channels) + .filter(([, value]) => value.starred) + .map(([id]) => id); + }); + const owner = createSidebarPreferencesStore(read, true, assignment, star); + await owner.queries.ensure(); + return { + owner, + prefs: owner.queries, + read, + publish, + publications, + assignment, + star, + }; +} + +it.each([true, false])( + "removal clears the durable previous group even when cached assignment is %s", + async (cachedAssignment) => { + const h = await setup({ cachedAssignment }); + try { + await h.prefs.setStar("alpha", false); + expect(h.publications).toEqual(["channel-sections", "channel-stars"]); + const restored = await h.read(); + expect(restored.assignments).toEqual({ beta: "work" }); + expect(restored.starred).toEqual([]); + expect(h.prefs.snapshot().data).toEqual(restored); + expect( + sidebarSections([{ id: "alpha", name: "Alpha" }], restored).map( + ({ key }) => key, + ), + ).toEqual(["channels"]); + } finally { + h.owner.dispose(); + } + }, +); + +it("moves directly from Starred into a saved group and retains other assignments", async () => { + const h = await setup(); + try { + await h.prefs.assign("alpha", "work"); + const restored = await h.read(); + expect(restored.assignments).toEqual({ alpha: "work", beta: "work" }); + expect(restored.starred).toEqual([]); + expect( + sidebarSections([{ id: "alpha", name: "Alpha" }], restored).map( + ({ key }) => key, + ), + ).toEqual(["group:work"]); + } finally { + h.owner.dispose(); + } +}); + +it("does not clear Star when assignment publication fails", async () => { + const h = await setup(); + try { + const before = h.prefs.snapshot(); + h.publish.mockRejectedValueOnce(new Error("assignment rejected")); + await expect(h.prefs.setStar("alpha", false)).rejects.toThrow( + "assignment rejected", + ); + expect(h.star).not.toHaveBeenCalled(); + expect(h.prefs.snapshot()).toBe(before); + expect((await h.read()).starred).toEqual(["alpha"]); + await h.prefs.setStar("alpha", false); + expect((await h.read()).assignments).toEqual({ beta: "work" }); + } finally { + h.owner.dispose(); + } +}); + +it("keeps Starred after a partial failure and retry cannot resurrect the old group", async () => { + const h = await setup(); + try { + const before = h.prefs.snapshot(); + h.star.mockRejectedValueOnce(new Error("star rejected")); + await expect(h.prefs.setStar("alpha", false)).rejects.toThrow( + "star rejected", + ); + expect(h.prefs.snapshot()).toBe(before); + const partial = await h.read(); + expect(partial.assignments).toEqual({ beta: "work" }); + expect(partial.starred).toEqual(["alpha"]); + await h.prefs.refresh(); + await h.prefs.setStar("alpha", false); + expect(h.prefs.snapshot().data).toEqual({ ...partial, starred: [] }); + expect(h.publications).toEqual(["channel-sections", "channel-stars"]); + } finally { + h.owner.dispose(); + } +}); + +it("a refresh during the two-write move cannot expose the intermediate assignment", async () => { + const h = await setup(); + let release; + const held = new Promise((resolve) => { + release = resolve; + }); + let started; + const entered = new Promise((resolve) => { + started = resolve; + }); + const star = h.star.getMockImplementation(); + h.star.mockImplementationOnce(async (...args) => { + started(); + await held; + return star(...args); + }); + try { + const before = h.prefs.snapshot(); + const pending = h.prefs.setStar("alpha", false); + await entered; + const refresh = h.prefs.refresh(); + expect(h.prefs.snapshot()).toBe(before); + expect(h.read).toHaveBeenCalledOnce(); + release(); + await Promise.all([pending, refresh]); + expect(h.prefs.snapshot().data.assignments).toEqual({ beta: "work" }); + expect(h.prefs.snapshot().data.starred).toEqual([]); + } finally { + release(); + h.owner.dispose(); + } +}); + +it("failed move plus an older refresh cannot strand the preference status at loading", async () => { + const h = await setup(); + let release; + const held = new Promise((resolve) => { + release = resolve; + }); + let started; + const entered = new Promise((resolve) => { + started = resolve; + }); + try { + const before = h.prefs.snapshot().data; + h.read.mockImplementationOnce(async () => { + started(); + return held; + }); + const refresh = h.prefs.refresh(); + await entered; + h.assignment.mockRejectedValueOnce(new Error("assignment failed")); + await expect(h.prefs.setStar("alpha", false)).rejects.toThrow( + "assignment failed", + ); + release(before); + await refresh; + expect(h.prefs.snapshot()).toEqual({ + status: "error", + error: "assignment failed", + data: before, + }); + await h.prefs.refresh(); + expect(h.prefs.snapshot().status).toBe("ready"); + } finally { + release(h.prefs.snapshot().data); + h.owner.dispose(); + } +}); + +it("cancellation between records prevents clearing Star and retry finishes from durable state", async () => { + const h = await setup(); + const caller = new AbortController(); + const assign = h.assignment.getMockImplementation(); + h.assignment.mockImplementationOnce(async (...args) => { + const result = await assign(...args); + caller.abort(); + return result; + }); + try { + await expect( + h.prefs.setStar("alpha", false, caller.signal), + ).rejects.toThrow(); + expect(h.star).not.toHaveBeenCalled(); + expect((await h.read()).starred).toEqual(["alpha"]); + await h.prefs.setStar("alpha", false); + const restored = await h.read(); + expect(restored.assignments).toEqual({ beta: "work" }); + expect(restored.starred).toEqual([]); + } finally { + h.owner.dispose(); + } +}); diff --git a/dev/sidebar-stars.mjs b/dev/sidebar-stars.mjs index 50be9b06..9e8b25f7 100644 --- a/dev/sidebar-stars.mjs +++ b/dev/sidebar-stars.mjs @@ -41,7 +41,8 @@ export function prepareSidebarStar(events, intent, secret, now = Date.now()) { const previous = Object.hasOwn(current.channels, intent.channelId) ? current.channels[intent.channelId] : undefined; - if (previous?.starred === intent.starred) return { stars: current }; + if (previous?.starred === intent.starred || (!previous && !intent.starred)) + return { stars: current }; const stars = { ...current, channels: { diff --git a/dev/sidebar-stars.test.mjs b/dev/sidebar-stars.test.mjs index f97508f9..e4e48572 100644 --- a/dev/sidebar-stars.test.mjs +++ b/dev/sidebar-stars.test.mjs @@ -124,7 +124,7 @@ it("encrypts explicit Star/Unstar with monotonic timestamps and preserves unrela h.secret, 50000, ).stars.channels, - ).toEqual({ new: { starred: false, updatedAt: 50000 } }); + ).toEqual({}); }); it("refuses untrusted, ambiguous, malformed and over-budget heads rather than seeding", () => { const h = harness(), diff --git a/docs/channels.md b/docs/channels.md index 574f538e..ee3e07a3 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -80,9 +80,9 @@ unstar tombstones are retained. Invalid/unreadable/over-budget heads fail closed only a successful absent-head read can seed a coordinate. Same-host writes are serialized per relay. This is confirmed whole-record replacement, not atomic cross-device merging, a durable pending outbox, or automatic retry: simultaneous -writers on different hosts can still race. Failure leaves the last confirmed UI -state and offers an explicit retry; a failed confirmation may follow a publication -that reached the relay. +writers on different hosts can still race. Failure retains the last fully confirmed UI +placement and offers an explicit retry; a failed confirmation may follow a +publication that reached the relay. The session preference owner serializes local commands and fences refreshes, caller cancellation, cache clear and disposal. `session.ts` only composes host @@ -91,7 +91,19 @@ pending work but cannot retract a publication already accepted by the relay. Stream rows expose these actions by right-click/long-press, Shift+F10 or the Context Menu key. Menus remain open during saving and failed-save retry; confirmed relocation expands the destination and restores focus by channel identity. Starred -placement is exclusive, retaining the saved assignment so Unstar restores it. +is a built-in group pinned first, offered alongside saved groups in one "Move to…" +chooser. Placement is exclusive. "Remove from Starred" and "Remove from [group]" +return to Channels, never to a remembered group. Moving out of Starred directly +into a saved group is supported. + +The legacy format still stores stars and assignments separately. The preference +owner confirms the requested assignment (or its removal) **before** clearing Star; +removal always checks the fresh assignment head, not just the cached projection. +Only the complete move updates local placement, and refreshes cannot expose an +intermediate write. If the second write fails, the channel remains Starred and an +explicit retry finishes the move; a reload reflects whatever reached the relay. +This is ordered two-record persistence, not an atomic multi-device move. A prior +assignment may remain stored while starred but is never used as an Unstar target. Forums/DMs, group CRUD/reorder and independent sorting are outside this slice. Hosts without the write capabilities retain the read-only projection. diff --git a/src/bundled/channels/ChannelsPage.tsx b/src/bundled/channels/ChannelsPage.tsx index 1bfb9b82..3fe49794 100644 --- a/src/bundled/channels/ChannelsPage.tsx +++ b/src/bundled/channels/ChannelsPage.tsx @@ -555,12 +555,7 @@ function ChannelWorkspace({ try { await preferences.setStar(channelId, starred); if (generation !== rowMenuGeneration.current) return; - const sectionId = preferences.data?.assignments[channelId]; - const sectionKey = starred - ? "starred" - : preferences.data?.sections.some((group) => group.id === sectionId) - ? `group:${sectionId}` - : "channels"; + const sectionKey = starred ? "starred" : "channels"; sidebar.toggle(sectionKey, true); setRowFocus({ channelId, sectionKey }); closeRowMenu(); @@ -618,18 +613,13 @@ function ChannelWorkspace({ : undefined; const movable = preferences.writable && - section.key !== "starred" && - channel.channelType !== "dm" && - channel.channelType !== "forum" && - !!preferences.data?.sections.length; - const starrable = preferences.starWritable && !!preferences.data && channel.channelType !== "dm" && channel.channelType !== "forum"; const starred = section.key === "starred"; const menuOpen = - (movable || starrable) && + movable && rowMenu?.channel.id === channel.id && rowMenu.sectionId === currentSectionId; const channelButton = ( @@ -652,7 +642,7 @@ function ChannelWorkspace({ ); - if (!movable && !starrable) { + if (!movable) { return (
{channelButton} @@ -700,64 +690,63 @@ function ChannelWorkspace({ ) ?? false } > - {starrable && ( - - void setChannelStar(channel.id, !starred) - } - > - {starred ? "Unstar" : "Star"} - - )} - {movable && ( + + Move to… + + { + if (destination === "starred") + void setChannelStar(channel.id, true); + else + void assignGroup( + channel.id, + destination.slice("group:".length), + ); + }} + disabled={ + groupWrite?.channelId === channel.id && + groupWrite.pending + } + > + + + Starred + + {preferences.data?.sections.map((group) => ( + + {group.icon && {group.icon}} + {group.name} + + ))} + + {(starred || currentSectionId) && ( <> - {starrable && } - - Move to group - - - void assignGroup(channel.id, sectionId) - } + + { + if (starred) + void setChannelStar(channel.id, false); + else void assignGroup(channel.id); + }} > - {preferences.data?.sections.map((group) => ( - - {group.icon && ( - {group.icon} - )} - {group.name} - - ))} - - {currentSectionId && ( - <> - - void assignGroup(channel.id)} - > - Remove from group - - - )} + Remove from {section.title} + )} {groupWrite?.channelId === channel.id && diff --git a/src/features/relay/sidebar-preferences-store.test.ts b/src/features/relay/sidebar-preferences-store.test.ts index b827ba4b..84fcfe63 100644 --- a/src/features/relay/sidebar-preferences-store.test.ts +++ b/src/features/relay/sidebar-preferences-store.test.ts @@ -31,8 +31,24 @@ function setup( const owner = createRelaySession({ ...wire.transport, decodeSidebarPreferences: decode, - ...(write ? { writeSidebarAssignment: write } : {}), - ...(writeStar ? { writeSidebarStar: writeStar } : {}), + ...(write || writeStar + ? { + writeSidebarAssignment: + write ?? + (async ({ channelId, sectionId }) => { + const assignments = { ...data.assignments }; + if (sectionId) assignments[channelId] = sectionId; + else delete assignments[channelId]; + return { sections: data.sections, assignments }; + }), + writeSidebarStar: + writeStar ?? + (async ({ channelId, starred }) => [ + ...data.starred.filter((id) => id !== channelId), + ...(starred ? [channelId] : []), + ]), + } + : {}), }); return { wire, owner, preferences: owner.session.sidebarPreferences, decode }; } @@ -309,7 +325,8 @@ it.each(["clearCache", "dispose"] as const)( it("serializes confirmed assignment and star writes without losing either projection", async () => { const gate = deferred(); const started = deferred(); - const star = vi.fn(async () => { + const star = vi.fn(async ({ starred }) => { + if (!starred) return ["alpha"]; started.resolve(); return gate.promise; }); @@ -335,7 +352,7 @@ it("serializes confirmed assignment and star writes without losing either projec data: { ...data, assignments: { alpha: "work", beta: "work" }, - starred: ["alpha", "beta"], + starred: ["alpha"], }, }); expect(Object.isFrozen(preferences.snapshot().data?.starred)).toBe(true); diff --git a/src/features/relay/sidebar-preferences-store.ts b/src/features/relay/sidebar-preferences-store.ts index b6fe42c1..3036382d 100644 --- a/src/features/relay/sidebar-preferences-store.ts +++ b/src/features/relay/sidebar-preferences-store.ts @@ -29,6 +29,7 @@ export function createSidebarPreferencesStore( let writeQueue = Promise.resolve(); let writeLifetime = new AbortController(); let mutation = 0; + let writing = false; let generation = 0; const retained = (data: SidebarPreferences): SidebarPreferences => Object.freeze({ @@ -44,6 +45,7 @@ export function createSidebarPreferencesStore( }; function refresh(): Promise { if (closed || !available) return Promise.resolve(); + if (writing) return writeQueue; if (active) return active.promise; const controller = new AbortController(); const refreshMutation = mutation; @@ -83,86 +85,103 @@ export function createSidebarPreferencesStore( }); return job.promise; } + // The legacy format uses two coordinates. Keep a move in one session queue, + // confirm the destination assignment before clearing Star, and expose the new + // placement only after both writes succeed. Failure is explicitly retryable; + // this is not an atomic cross-host transaction. + function move( + channelId: string, + destination: { starred: true } | { sectionId?: string }, + signal?: AbortSignal, + ): Promise { + const starring = "starred" in destination; + if (closed || !snapshot.data || !writeStar || !write) + return Promise.reject( + new Error("Sidebar group moves are unavailable in this host"), + ); + const writeGeneration = generation; + const writeSignal = AbortSignal.any([ + writeLifetime.signal, + ...(signal ? [signal] : []), + ]); + const check = () => { + if (closed || generation !== writeGeneration) + throw new Error("Sidebar group moves are unavailable"); + writeSignal.throwIfAborted(); + }; + const run = writeQueue + .catch(() => {}) + .then(async () => { + check(); + mutation++; + writing = true; + try { + // Always re-read/write the assignment on removal, even if the cached + // projection has no assignment (another client may have added one). + const groups = starring + ? undefined + : await write( + { + channelId, + ...(destination.sectionId + ? { sectionId: destination.sectionId } + : {}), + }, + writeSignal, + ); + check(); + const stars = await writeStar( + { channelId, starred: starring }, + writeSignal, + ); + check(); + const current = snapshot.data; + if (!current) throw new Error("Sidebar group moves are unavailable"); + const data = retained({ ...current, ...groups, starred: stars }); + publish({ status: "ready", data }); + return data; + } catch (error) { + // A refresh fenced by this move must not leave a permanent loading + // state if the move fails too. Retain its last confirmed placement. + if ( + generation === writeGeneration && + !closed && + snapshot.status === "loading" + ) + publish({ + ...snapshot, + status: "error", + error: error instanceof Error ? error.message : String(error), + }); + throw error; + } finally { + if (generation === writeGeneration) { + mutation++; + writing = false; + } + } + }); + writeQueue = run.then( + () => undefined, + () => undefined, + ); + return run; + } return { queries: Object.freeze({ available, - writable: !!write, + writable: !!write && !!writeStar, assign(channelId: string, sectionId?: string, signal?: AbortSignal) { - if (closed || !write || !snapshot.data) - return Promise.reject( - new Error("Saved sidebar groups are read-only in this host"), - ); - const writeGeneration = generation; - const writeSignal = AbortSignal.any([ - writeLifetime.signal, - ...(signal ? [signal] : []), - ]); - const run = writeQueue - .catch(() => {}) - .then(async () => { - if (closed || generation !== writeGeneration) - throw new Error("Saved sidebar groups are unavailable"); - writeSignal.throwIfAborted(); - const groups = await write( - { channelId, ...(sectionId ? { sectionId } : {}) }, - writeSignal, - ); - if (closed || generation !== writeGeneration) - throw new Error("Saved sidebar groups are unavailable"); - writeSignal.throwIfAborted(); - mutation++; - const current = snapshot.data; - publish({ - status: "ready", - data: retained({ - sections: groups.sections, - assignments: groups.assignments, - starred: current?.starred ?? [], - }), - }); - return groups; - }); - writeQueue = run.then( - () => undefined, - () => undefined, - ); - return run; + return move(channelId, sectionId ? { sectionId } : {}, signal); }, - starWritable: !!writeStar, - setStar(channelId: string, starred: boolean, signal?: AbortSignal) { - if (closed || !writeStar || !snapshot.data) - return Promise.reject( - new Error("Sidebar stars are unavailable in this host"), - ); - const writeGeneration = generation; - const writeSignal = AbortSignal.any([ - writeLifetime.signal, - ...(signal ? [signal] : []), - ]); - const run = writeQueue - .catch(() => {}) - .then(async () => { - if (closed || generation !== writeGeneration) - throw new Error("Sidebar stars are unavailable"); - writeSignal.throwIfAborted(); - const stars = await writeStar({ channelId, starred }, writeSignal); - if (closed || generation !== writeGeneration) - throw new Error("Sidebar stars are unavailable"); - writeSignal.throwIfAborted(); - const current = snapshot.data; - if (!current) throw new Error("Sidebar stars are unavailable"); - mutation++; - publish({ - status: "ready", - data: retained({ ...current, starred: stars }), - }); - return stars; - }); - writeQueue = run.then( - () => undefined, - () => undefined, + starWritable: !!write && !!writeStar, + async setStar(channelId: string, starred: boolean, signal?: AbortSignal) { + const data = await move( + channelId, + starred ? { starred: true } : {}, + signal, ); - return run; + return data.starred; }, // Keep explicit one-shot reads compatible; views use the retained snapshot. read, @@ -183,6 +202,7 @@ export function createSidebarPreferencesStore( clear() { if (closed) return; generation++; + writing = false; mutation++; writeLifetime.abort(); writeLifetime = new AbortController(); @@ -194,6 +214,7 @@ export function createSidebarPreferencesStore( closed = true; writeLifetime.abort(); generation++; + writing = false; mutation++; active?.controller.abort(); active = undefined; diff --git a/tests/browser/navigation-groups.spec.mjs b/tests/browser/navigation-groups.spec.mjs index 6655de42..53d617c6 100644 --- a/tests/browser/navigation-groups.spec.mjs +++ b/tests/browser/navigation-groups.spec.mjs @@ -31,17 +31,17 @@ test("row menu moves and removes a channel through the confirmed saved-group wri ).toHaveAttribute("aria-checked", "true"); await page.keyboard.press("End"); await expect( - page.getByRole("menuitem", { name: "Remove from group" }), + page.getByRole("menuitem", { name: "Remove from Work" }), ).toBeFocused(); await page.keyboard.press("Home"); await expect( - menu.getByRole("menuitem", { name: "Star", exact: true }), + menu.getByRole("menuitemradio", { name: "Starred", exact: true }), ).toBeFocused(); await page.keyboard.press("ArrowDown"); await expect(menu.getByRole("menuitemradio", { name: "Work" })).toBeFocused(); await page.keyboard.press("ArrowDown"); await expect( - page.getByRole("menuitem", { name: "Remove from group" }), + page.getByRole("menuitem", { name: "Remove from Work" }), ).toBeFocused(); await page.keyboard.press("Enter"); await expect( @@ -78,7 +78,7 @@ test("row menu moves and removes a channel through the confirmed saved-group wri // Real context-menu keyboard/focus, viewport placement and row relocation require // a browser. Intent validation, concurrency and persistence matrices stay below. -test("Star and Unstar retain the assigned group, keep one row, and recover from a failed save", async ({ +test("group moves include Starred, remove to Channels, and recover from a failed save", async ({ page, app, }, testInfo) => { @@ -122,11 +122,13 @@ test("Star and Unstar retain the assigned group, keep one row, and recover from }); }); try { - await menu.getByRole("menuitem", { name: "Star", exact: true }).click(); + await menu + .getByRole("menuitemradio", { name: "Starred", exact: true }) + .click(); await requestStarted; await expect(menu.getByRole("status")).toHaveText("Saving…"); await expect( - menu.getByRole("menuitem", { name: "Star", exact: true }), + menu.getByRole("menuitemradio", { name: "Starred", exact: true }), ).toHaveAttribute("aria-disabled", "true"); await expect( page.locator( @@ -140,12 +142,12 @@ test("Star and Unstar retain the assigned group, keep one row, and recover from "Relay request failed (502)", ); await expect( - menu.getByRole("menuitem", { name: "Star", exact: true }), + menu.getByRole("menuitemradio", { name: "Starred", exact: true }), ).toBeEnabled(); await page.unroute("**/sidebar-star"); await page.keyboard.press("Home"); await expect( - menu.getByRole("menuitem", { name: "Star", exact: true }), + menu.getByRole("menuitemradio", { name: "Starred", exact: true }), ).toBeFocused(); await page.keyboard.press("Enter"); const relocated = starred.getByRole("button", { name: "Beta", exact: true }); @@ -159,7 +161,7 @@ test("Star and Unstar retain the assigned group, keep one row, and recover from blob: { channels: { alpha: { starred: true }, beta: { starred: true } } }, }); - // Work is absent while its only row is starred. Unstar must restore it. + // Work is absent while its only row is starred. Removal returns to Channels. await relocated.click({ button: "right" }); await expect(menu).toBeVisible(); const bounds = await menu.boundingBox(); @@ -168,15 +170,55 @@ test("Star and Unstar retain the assigned group, keep one row, and recover from expect(bounds.y).toBeGreaterThanOrEqual(0); expect(bounds.x + bounds.width).toBeLessThanOrEqual(viewport.width); expect(bounds.y + bounds.height).toBeLessThanOrEqual(viewport.height); - await menu.getByRole("menuitem", { name: "Unstar", exact: true }).click(); - await expect(beta).toBeVisible(); - await expect(beta).toBeFocused(); + await expect( + menu.getByRole("menuitemradio", { name: "Starred", exact: true }), + ).toHaveAttribute("aria-checked", "true"); + await menu + .getByRole("menuitem", { name: "Remove from Starred", exact: true }) + .click(); + const ungrouped = sidebar.locator( + '[data-sidebar-section="channels"] [data-channel-id="beta"]', + ); + await expect(ungrouped).toBeVisible(); + await expect(ungrouped).toBeFocused(); + await expect(beta).toHaveCount(0); await expect(sidebar.locator('[data-channel-id="beta"]')).toHaveCount(1); - expect(app.report.sidebarPublications).toHaveLength(2); + expect(app.report.sidebarPublications).toHaveLength(3); expect(app.report.sidebarPublications[1]).toMatchObject({ + coordinate: "channel-sections", + blob: { assignments: {} }, + }); + expect(app.report.sidebarPublications[2]).toMatchObject({ coordinate: "channel-stars", blob: { channels: { alpha: { starred: true }, beta: { starred: false } } }, }); + await page.reload(); + // Roster rows can render before preference decode reattaches their menus. + await expect( + starred.getByRole("button", { name: "Alpha", exact: true }), + ).toBeVisible(); + await expect(ungrouped).toBeVisible(); + await expect(beta).toHaveCount(0); + + // Direct moves out of Starred use the same chooser, without a separate Unstar. + await ungrouped.click({ button: "right" }); + await menu + .getByRole("menuitemradio", { name: "Starred", exact: true }) + .click(); + await expect(relocated).toBeFocused(); + await relocated.click({ button: "right" }); + await menu.getByRole("menuitemradio", { name: "Work", exact: true }).click(); + await expect(beta).toBeVisible(); + await expect(beta).toBeFocused(); + await expect(sidebar.locator('[data-channel-id="beta"]')).toHaveCount(1); + expect(app.report.sidebarPublications.at(-2)).toMatchObject({ + coordinate: "channel-sections", + blob: { assignments: { beta: "work" } }, + }); + expect(app.report.sidebarPublications.at(-1)).toMatchObject({ + coordinate: "channel-stars", + blob: { channels: { beta: { starred: false } } }, + }); expect(app.report.unexpected).toEqual([]); });