From b1655e2148245508ce139b5a1d86a0959d143ee9 Mon Sep 17 00:00:00 2001 From: Tim Disney Date: Sun, 26 Jul 2026 11:22:48 -0700 Subject: [PATCH] Show an identity's Bluesky profile picture in its disc (#9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An actor disc has always been two derived letters on a colour derived from the DID. For an atproto identity that publishes a profile picture, the picture is a better answer to "who" — and it is the same face that identity already has everywhere else on the network. `resolveAvatarUrl` reads `app.bsky.actor.profile/self` from the DID's OWN PDS — the same place a handle comes from, never an aggregator that would have to be trusted about who somebody is — and turns the blob it names into a `cdn.bsky.app` thumbnail URL, so a 17px disc costs a thumbnail rather than a full-size blob. Only the URL goes through Bluesky. `Identities` caches the picture beside the handle and the PDS on a clock of its own: the read path asks that cache for a PDS on every poll and never for a face, so only `resolve` — the display path — spends a profile read. An entry cached before this existed gains a face on the next resolve without its handle being re-resolved. The derived disc is the fallback at every step, not an error path: no profile record (which is what most agents will always be), an unreachable PDS, an image that fails to load, or one still in flight all leave the row exactly as it was. Co-authored-by: claudebot.disnetdev.com (did:plc:n6ku5xddiuguwze3f356evla) --- DESIGN.md | 6 +- packages/atproto/src/client.ts | 77 +++++++++++++ packages/atproto/test/client.test.mjs | 71 ++++++++++++ packages/ui/src/app.css | 3 + packages/ui/src/lib/components/Account.svelte | 4 +- packages/ui/src/lib/components/Disc.svelte | 22 +++- packages/ui/src/lib/directory.ts | 14 ++- packages/ui/src/lib/fake-pds.ts | 20 ++++ packages/ui/src/lib/identity.test.ts | 80 ++++++++++++++ packages/ui/src/lib/identity.ts | 102 +++++++++++++++--- packages/ui/src/lib/session.svelte.ts | 4 +- packages/ui/src/lib/session.test.ts | 36 ++++++- 12 files changed, 417 insertions(+), 22 deletions(-) create mode 100644 packages/ui/src/lib/identity.test.ts diff --git a/DESIGN.md b/DESIGN.md index adff4ad..a000c05 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -231,7 +231,7 @@ three-colour semantic vocabulary that only verdicts and blocked states may spend - **Amber** — text `oklch(0.475 0.108 68)`, mark `oklch(0.660 0.145 72)`, wash `oklch(0.958 0.044 78)`: *stopped on you* — an unanswered question, a stale sync, the fixture chip. Never "warning" in the generic sense. ### Accents of last resort -- **Teal** (`oklch(0.560 0.100 200)`) and **Plum** (`oklch(0.520 0.130 330)`): the System glyph, and actor discs. Disc colour is derived from a DID, never authored. +- **Teal** (`oklch(0.560 0.100 200)`) and **Plum** (`oklch(0.520 0.130 330)`): the System glyph, and actor discs. Disc colour is derived from a DID, never authored — and it is what shows through when an identity publishes no profile picture. ### Named Rules **The One Voice Rule.** Ink indigo is the only brand hue, and it appears only on things that act or are @@ -339,7 +339,9 @@ state: a hollow ring (open), an accent ring with a wash centre (assigned), the s rotating (a live claim), a thick amber ring with a filled centre (awaiting you), a ring with a tick (landed), a solid ink disc (judged), a solid sage disc (merged), a struck-through ring (retracted). **The disc** is an actor: a circle for a human, a 5px-radius square for an agent, initials on a colour derived -from the DID. +from the DID. When that identity publishes a profile picture, the picture fills the disc — same box, +same shape, cropped to cover — and the derived fill stays behind it as the fallback for a picture that +is loading, transparent, or gone. Borders are hairlines, and a control's border is what changes on hover — not its fill, except where the fill *is* the state (a pressed version button, an "on" auto-review switch). diff --git a/packages/atproto/src/client.ts b/packages/atproto/src/client.ts index 2c5ec27..65bb985 100644 --- a/packages/atproto/src/client.ts +++ b/packages/atproto/src/client.ts @@ -408,6 +408,83 @@ export async function resolveDidHandle( } } +/** Where a Bluesky profile picture is published: one record per repo, at a fixed rkey. */ +export const BSKY_PROFILE_COLLECTION = 'app.bsky.actor.profile' +/** Bluesky's image service. It resizes and re-encodes, so a 17px disc costs a thumbnail, not a blob. */ +export const BSKY_CDN = 'https://cdn.bsky.app' + +export interface AvatarOptions { + fetcher?: FetchLike + /** The DID's PDS, when the caller already resolved one — this is the only place the record lives. */ + pds?: string | URL + /** A DID document already in hand, so the PDS is not resolved twice. */ + document?: DidDocument + /** The image service to serve the blob through. Overridable for a test or a private deployment. */ + cdn?: string | URL + signal?: AbortSignal +} + +/** The blob CID an `app.bsky.actor.profile` record points its avatar at, in either encoding. */ +function avatarBlobCid(record: unknown): string | undefined { + if (!object(record)) return undefined + const avatar = record.avatar + if (!object(avatar)) return undefined + // The current blob encoding, and the legacy one records written before the blob migration still use. + const ref = avatar.ref + if (object(ref) && typeof ref.$link === 'string' && ref.$link) return ref.$link + return typeof avatar.cid === 'string' && avatar.cid ? avatar.cid : undefined +} + +/** + * DID → the picture that identity publishes for itself, or `undefined`. + * + * A profile picture is not a Radial record and cannot be: it belongs to the person, not to the space, + * and it is the same picture every other atproto app shows them by. So it is read the way a handle + * is — from the identity's own repo (`app.bsky.actor.profile/self` on the DID's own PDS), never from + * an aggregator that would have to be trusted to answer honestly about who somebody is. + * + * Only the *URL* goes through Bluesky: the record names a blob, and `cdn.bsky.app` is the resizer + * that turns it into something a 17px disc can afford. Every failure here — no profile record, no + * avatar in it, an unreachable PDS — is `undefined`, which the caller renders as the derived disc. + */ +export async function resolveAvatarUrl( + did: string, + options: AvatarOptions = {}, +): Promise { + const fetcher = options.fetcher ?? fetch + let service: URL + try { + service = options.pds + ? new URL(options.pds) + : pdsEndpoint(options.document ?? (await resolveDidDocument(did, fetcher)), did) + } catch { + return undefined + } + const query = new URLSearchParams({ + repo: did, + collection: BSKY_PROFILE_COLLECTION, + rkey: 'self', + }) + let cid: string | undefined + try { + const value = await jsonResponse( + await fetcher( + new URL(`/xrpc/com.atproto.repo.getRecord?${query}`, service), + options.signal ? { signal: options.signal } : undefined, + ), + ) + cid = avatarBlobCid(value.value) + } catch { + // No profile record (the common case for an agent), or a PDS that would not answer. + return undefined + } + if (!cid) return undefined + return new URL( + `/img/avatar_thumbnail/plain/${encodeURIComponent(did)}/${encodeURIComponent(cid)}@jpeg`, + options.cdn ?? BSKY_CDN, + ).toString() +} + export type TxtResolver = (hostname: string) => Promise export interface IdentityResolution { diff --git a/packages/atproto/test/client.test.mjs b/packages/atproto/test/client.test.mjs index 309fe84..b103735 100644 --- a/packages/atproto/test/client.test.mjs +++ b/packages/atproto/test/client.test.mjs @@ -11,6 +11,7 @@ import { StrongRefResolver, XrpcError, createSession, + resolveAvatarUrl, resolveDidHandle, resolveHandleDid, resolveIdentityPds, @@ -434,6 +435,76 @@ describe('identity resolution', () => { ) }) + it('resolves an avatar from the identity own repo, and nothing when there is none', async () => { + // The picture is read from the DID's own PDS — the record is the identity's own assertion about + // itself — and only the resized *URL* goes through Bluesky's image service. + const asked = [] + const profile = (value) => async (url) => { + asked.push(String(url)) + return new URL(url).hostname === 'plc.directory' ? didDocument('https://pds.example') : value + } + + assert.equal( + await resolveAvatarUrl('did:plc:agent', { + fetcher: profile( + json({ + uri: 'at://did:plc:agent/app.bsky.actor.profile/self', + cid: 'cid-profile', + value: { $type: 'app.bsky.actor.profile', avatar: { ref: { $link: 'bafyblob' } } }, + }), + ), + }), + 'https://cdn.bsky.app/img/avatar_thumbnail/plain/did%3Aplc%3Aagent/bafyblob@jpeg', + ) + assert.deepEqual(asked, [ + 'https://plc.directory/did%3Aplc%3Aagent', + 'https://pds.example/xrpc/com.atproto.repo.getRecord?repo=did%3Aplc%3Aagent&collection=app.bsky.actor.profile&rkey=self', + ]) + + // A caller that already resolved the PDS does not resolve it again. + asked.length = 0 + assert.equal( + await resolveAvatarUrl('did:plc:agent', { + pds: 'https://pds.example', + fetcher: profile( + json({ + value: { avatar: { cid: 'legacyblob' } }, + }), + ), + }), + // The pre-migration blob encoding is still what some profile records hold. + 'https://cdn.bsky.app/img/avatar_thumbnail/plain/did%3Aplc%3Aagent/legacyblob@jpeg', + ) + assert.equal(asked.length, 1) + + // Every way of having no picture is the same answer: the caller draws its derived disc. + for (const response of [ + json({ error: 'RecordNotFound', message: 'not found' }, 400), + json({ value: { $type: 'app.bsky.actor.profile', description: 'no picture' } }), + json({ value: { avatar: {} } }), + ]) { + assert.equal( + await resolveAvatarUrl('did:plc:agent', { pds: 'https://pds.example', fetcher: profile(response) }), + undefined, + ) + } + + // …including a DID document with no PDS to ask, and a PDS that is simply not there. + assert.equal( + await resolveAvatarUrl('did:plc:agent', { fetcher: async () => json({ id: 'did:plc:agent', service: [] }) }), + undefined, + ) + assert.equal( + await resolveAvatarUrl('did:plc:agent', { + pds: 'https://pds.example', + fetcher: async () => { + throw new Error('offline') + }, + }), + undefined, + ) + }) + it('resolves a PDS from a handle and from a DID, skipping DNS for a DID', async () => { const fromHandle = await resolveIdentityPds('agent.example', { resolveTxt: async () => [['did=did:plc:agent']], diff --git a/packages/ui/src/app.css b/packages/ui/src/app.css index 76d971c..aef0221 100644 --- a/packages/ui/src/app.css +++ b/packages/ui/src/app.css @@ -404,6 +404,9 @@ a { color: var(--accent); text-underline-offset: 2px; } } .disc.human { border-radius: 999px; } .disc.agent { border-radius: 5px; } +/* A published profile picture, in the disc's own box and shape. The derived fill stays behind it, so + a transparent or slow image degrades to the colour it would have had. */ +.disc > img { width: 100%; height: 100%; object-fit: cover; border-radius: inherit; display: block; } /* ─── drawer ─────────────────────────────────────────────────────────────── */ .drawer { display: grid; grid-template-rows: 0fr; transition: grid-template-rows 400ms var(--ease-out); } diff --git a/packages/ui/src/lib/components/Account.svelte b/packages/ui/src/lib/components/Account.svelte index c3b208c..5f67ec8 100644 --- a/packages/ui/src/lib/components/Account.svelte +++ b/packages/ui/src/lib/components/Account.svelte @@ -27,7 +27,9 @@ // established under stands in. Either way the disc's colour comes from the DID, so it never moves. const me = $derived.by(() => { const known = space?.directory.get(account.did) - return known?.handle ? known : unknownActor(account.did, account.handle || undefined) + return known?.handle + ? known + : unknownActor(account.did, account.handle || undefined, known?.avatar) }) function toggle(): void { diff --git a/packages/ui/src/lib/components/Disc.svelte b/packages/ui/src/lib/components/Disc.svelte index e66c13e..743bbc9 100644 --- a/packages/ui/src/lib/components/Disc.svelte +++ b/packages/ui/src/lib/components/Disc.svelte @@ -3,15 +3,35 @@ // Round for a human, square-ish for an agent — the shape carries the distinction, so it survives // greyscale and the colour never has to. + // + // When the identity publishes a profile picture (`identity.ts`), that is what the disc shows, in + // the same box and the same shape: a face is a better answer to "who" than two derived letters. + // The derived disc is underneath it — literally, as the image's background — so a picture that is + // still loading, is transparent, or never arrives leaves the row exactly as it was before avatars + // existed. An image that errors is dropped for the life of this component rather than retried on + // every redraw, and it is requested with no referrer — the image host has no business learning + // which space, goal or artifact somebody is looking at. interface Props { actor: Actor size?: number } const { actor, size = 17 }: Props = $props() + + let broken = $state('') + const src = $derived(actor.avatar && actor.avatar !== broken ? actor.avatar : undefined) {actor.initials} +>{#if src} (broken = actor.avatar ?? '')} + />{:else}{actor.initials}{/if} diff --git a/packages/ui/src/lib/directory.ts b/packages/ui/src/lib/directory.ts index 4687dd1..147e632 100644 --- a/packages/ui/src/lib/directory.ts +++ b/packages/ui/src/lib/directory.ts @@ -10,6 +10,11 @@ // a new member must get a stable colour with nobody choosing one. The palette is the comp's, held // as literals rather than theme tokens — a disc is a saturated fill with white text, so it must not // lighten when the desk goes dark. +// +// A derived disc is what an identity looks like when nothing better is known. When one publishes a +// profile picture, `identity.ts` resolves it and overlays it here the same way a handle is overlaid, +// and `Disc.svelte` shows the face instead — same size, same shape, so the row does not move. The +// colour and initials are computed either way: they are the fallback the moment the image fails. import type { MaterializedIndex } from '@radial/core' @@ -23,6 +28,8 @@ export interface Actor { name: string initials: string color: string + /** The profile picture this identity publishes, when something has resolved one. */ + avatar?: string /** For an agent, the artifact types it accepts — what an assignee picker filters on. */ artifactTypes: string[] active: boolean @@ -60,7 +67,7 @@ const initialsFrom = (name: string): string => { * was later removed, or the human who just signed in to a space they are not a member of yet. Colour * and initials come out the same way they do for a member, so nobody changes appearance by joining. */ -export function unknownActor(did: string, handle?: string): Actor { +export function unknownActor(did: string, handle?: string, avatar?: string): Actor { return { did, kind: 'human', @@ -69,6 +76,7 @@ export function unknownActor(did: string, handle?: string): Actor { name: handle ?? shortDid(did), initials: initialsFrom(handle ?? did.slice(did.lastIndexOf(':') + 1)), color: colorFor(did), + ...(avatar ? { avatar } : {}), artifactTypes: [], active: false, } @@ -84,6 +92,7 @@ export interface Directory { export function buildDirectory( index: MaterializedIndex, handles: Record = {}, + avatars: Record = {}, ): Directory { const agentRecords = new Map() for (const agent of index.agents) { @@ -115,13 +124,14 @@ export function buildDirectory( name, initials: initialsFrom(handle ?? member.did.slice(member.did.lastIndexOf(':') + 1)), color: colorFor(member.did), + ...(avatars[member.did] ? { avatar: avatars[member.did] as string } : {}), artifactTypes: profile?.artifactTypes ?? [], active: member.active, }) } return { - get: (did) => actors.get(did) ?? unknownActor(did), + get: (did) => actors.get(did) ?? unknownActor(did, handles[did], avatars[did]), agents: () => [...actors.values()].filter((actor) => actor.kind === 'agent'), humans: () => [...actors.values()].filter((actor) => actor.kind === 'human'), } diff --git a/packages/ui/src/lib/fake-pds.ts b/packages/ui/src/lib/fake-pds.ts index ff6b13b..c98c5a0 100644 --- a/packages/ui/src/lib/fake-pds.ts +++ b/packages/ui/src/lib/fake-pds.ts @@ -60,6 +60,12 @@ export class FakePds { readonly repos = new Map() readonly heads = new Map() readonly calls: RecordedCall[] = [] + /** + * DID → the blob CID in that repo's `app.bsky.actor.profile`. A profile record is not a Radial + * record and lives outside `repos` for that reason: what the UI reads from it is one blob ref, and + * a DID absent here answers `RecordNotFound` exactly as a repo with no profile would. + */ + readonly profiles: Record = {} /** Every request throws while this is set: the member PDS, or the tab, has gone away. */ offline = false #minted = 0 @@ -67,7 +73,9 @@ export class FakePds { constructor( records: StoredRecord[], readonly handles: Record = {}, + profiles: Record = {}, ) { + Object.assign(this.profiles, profiles) for (const record of records) this.append(record) this.calls.length = 0 } @@ -142,6 +150,18 @@ export class FakePds { const collection = url.searchParams.get('collection') ?? '' const rkey = url.searchParams.get('rkey') ?? '' record('getRecord', did, `${collection}/${rkey}`) + if (collection === 'app.bsky.actor.profile') { + const blob = this.profiles[did] + if (!blob || rkey !== 'self') return json({ error: 'RecordNotFound', message: 'not found' }, 400) + return json({ + uri: `at://${did}/${collection}/${rkey}`, + cid: `cid-profile-${did}`, + value: { + $type: collection, + avatar: { $type: 'blob', ref: { $link: blob }, mimeType: 'image/jpeg', size: 1024 }, + }, + }) + } const found = this.#latest(did, collection, rkey) if (!found) return json({ error: 'RecordNotFound', message: 'not found' }, 400) return json({ uri: found.uri, cid: found.cid, value: found.value }) diff --git a/packages/ui/src/lib/identity.test.ts b/packages/ui/src/lib/identity.test.ts new file mode 100644 index 0000000..3bf4837 --- /dev/null +++ b/packages/ui/src/lib/identity.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from 'vitest' +import { FakePds, latestVersions, memoryStorage } from './fake-pds.js' +import { fixtureSpace, FIXTURE_DIDS } from '@radial/core/fixture' +import { Identities } from './identity.js' + +// The two facts this cache resolves that nothing else can, and the rule that keeps them apart: the +// PDS is what the read path needs on every tick, the face is what the view needs when it draws, and +// asking for one must not cost the other. + +const fixture = fixtureSpace() +const HANDLES = { [FIXTURE_DIDS.tim]: 'tim.test' } + +const harness = (profiles: Record = {}) => { + const network = new FakePds(latestVersions(fixture.records), HANDLES, profiles) + const storage = memoryStorage() + return { network, storage, identities: new Identities({ fetcher: network.fetch, storage }) } +} + +describe('resolving who a DID is', () => { + it('reads a profile record for the view, and never for the read path', async () => { + const { network, identities } = harness({ [FIXTURE_DIDS.tim]: 'bafyavatar' }) + + // What `FetchRepoTransport` asks for on every poll: a PDS, and nothing else. + expect(String(await identities.pdsResolver(FIXTURE_DIDS.tim))).toBe('https://pds.test/') + expect(network.counts('getRecord')).toBe(0) + expect(identities.avatars()).toEqual({}) + + // What a screen about to draw a row asks for. The document is already cached, so this costs the + // one request the picture itself needs. + expect(await identities.resolve([FIXTURE_DIDS.tim])).toBe(true) + expect(network.counts('didDocument')).toBe(1) + expect(identities.avatars()).toEqual({ + [FIXTURE_DIDS.tim]: `https://cdn.bsky.app/img/avatar_thumbnail/plain/${encodeURIComponent(FIXTURE_DIDS.tim)}/bafyavatar@jpeg`, + }) + + // Resolved is resolved: a second pass asks nothing and reports no redraw. + network.reset() + expect(await identities.resolve([FIXTURE_DIDS.tim])).toBe(false) + expect(network.calls).toHaveLength(0) + }) + + it('keeps a member with no profile record, and one whose PDS is down, on their derived disc', async () => { + const { network, identities } = harness() + expect(await identities.resolve([FIXTURE_DIDS.tim])).toBe(true) // the handle changed, if not a face + expect(identities.avatars()).toEqual({}) + + // A `RecordNotFound` is an answer, not a failure: it is not asked again until the entry expires. + network.reset() + await identities.resolve([FIXTURE_DIDS.tim]) + expect(network.calls).toHaveLength(0) + + const offline = harness() + offline.network.offline = true + expect(await offline.identities.resolve([FIXTURE_DIDS.ana])).toBe(false) + expect(offline.identities.avatars()).toEqual({}) + expect(offline.identities.handles()).toEqual({}) + }) + + it('gives a face to an entry cached before there were faces, without re-resolving the handle', async () => { + // The shape `radial:identities` held before this existed. It is still a good PDS and a good + // handle — only the picture is missing — so the upgrade costs one profile read, not a cold start. + const network = new FakePds(latestVersions(fixture.records), HANDLES, { + [FIXTURE_DIDS.tim]: 'bafyavatar', + }) + const storage = memoryStorage() + storage.setItem( + 'radial:identities', + JSON.stringify({ + [FIXTURE_DIDS.tim]: { pds: 'https://pds.test/', handle: 'tim.test', at: Date.now() }, + }), + ) + const identities = new Identities({ fetcher: network.fetch, storage }) + + expect(await identities.resolve([FIXTURE_DIDS.tim])).toBe(true) + expect(network.counts('didDocument')).toBe(0) + expect(network.counts('resolveHandle')).toBe(0) + expect(identities.handles()).toEqual({ [FIXTURE_DIDS.tim]: 'tim.test' }) + expect(identities.avatars()[FIXTURE_DIDS.tim]).toContain('bafyavatar@jpeg') + }) +}) diff --git a/packages/ui/src/lib/identity.ts b/packages/ui/src/lib/identity.ts index 0f379fb..6b692d1 100644 --- a/packages/ui/src/lib/identity.ts +++ b/packages/ui/src/lib/identity.ts @@ -1,18 +1,24 @@ // Who a DID is, resolved rather than read. // -// `directory.ts` turns the index's DIDs into rows; this turns a DID into the two facts the index -// cannot carry — the PDS that serves its repo, and the handle it goes by. Both come from the DID -// document, and both are cached, because the alternative is a `plc.directory` round trip per member -// per poll: `getLatestCommit` resolves a PDS every tick, and at ten seconds that is the difference -// between one request per member and three. +// `directory.ts` turns the index's DIDs into rows; this turns a DID into the three facts the index +// cannot carry — the PDS that serves its repo, the handle it goes by, and the face it publishes. +// All three are cached, because the alternative is a `plc.directory` round trip per member per poll: +// `getLatestCommit` resolves a PDS every tick, and at ten seconds that is the difference between one +// request per member and three. // // The handle is verified before it is shown (`resolveDidHandle` explains how, and why an unverified // one is dropped rather than displayed). A DID whose handle does not round-trip renders as a short // DID — never as a name that might belong to somebody else. +// +// The avatar is read from the identity's own repo (`resolveAvatarUrl`) once its PDS is known, which +// is one more request on a cold start and none on a warm one. A member with no profile record — an +// agent, usually — resolves to nothing at all and keeps the derived disc, so a face is an upgrade to +// a row that already worked rather than something a row waits for. import { declaredHandle, pdsEndpoint, + resolveAvatarUrl, resolveDidDocument, resolveDidHandle, type FetchLike, @@ -22,8 +28,16 @@ import { interface Identity { pds?: string handle?: string - /** When this was resolved, so a stale entry refreshes without a cold start. */ + /** The profile picture this DID publishes, when it publishes one. */ + avatar?: string + /** When the document facts above were resolved, so a stale entry refreshes without a cold start. */ at: number + /** + * When the profile record was last read — a second clock, because the picture is a second request + * against a second record. An entry cached before avatars existed has no `avatarAt` and so is due + * a face on the next `resolve`, without the handle and PDS beside it being thrown away. + */ + avatarAt?: number } const STORAGE_KEY = 'radial:identities' @@ -46,6 +60,7 @@ const browserStorage = (): IdentityOptions['storage'] => { export class Identities { readonly #entries = new Map() readonly #inflight = new Map>() + readonly #faces = new Map>() readonly #fetcher: FetchLike readonly #now: () => number readonly #storage: IdentityOptions['storage'] @@ -88,6 +103,20 @@ export class Identities { return resolved } + /** The profile pictures resolved so far, overlaid the same way. */ + avatars(): Record { + const resolved: Record = {} + for (const [did, identity] of this.#entries) { + if (identity.avatar) resolved[did] = identity.avatar + } + return resolved + } + + /** What a redraw would show, so `resolve` can answer whether anything actually changed. */ + #shown(): string { + return JSON.stringify([this.handles(), this.avatars()]) + } + /** A `PdsResolver` for `FetchRepoTransport`: cached, so a poll tick is one request per member. */ get pdsResolver(): PdsResolver { return async (did) => { @@ -103,13 +132,17 @@ export class Identities { * keeps showing as a DID, which is the same degradation as an unreachable PDS. */ async resolve(dids: Iterable): Promise { - const wanted = [...new Set(dids)].filter((did) => this.#stale(did)) + const wanted = [...new Set(dids)].filter((did) => this.#stale(did) || this.#faceStale(did)) if (wanted.length === 0) return false - const before = JSON.stringify(this.handles()) + const before = this.#shown() await Promise.all( - wanted.map((did) => this.#identity(did).catch(() => ({ at: this.#now() }) satisfies Identity)), + wanted.map((did) => + this.#identity(did) + .then(() => this.#face(did)) + .catch(() => undefined), + ), ) - return JSON.stringify(this.handles()) !== before + return this.#shown() !== before } #stale(did: string): boolean { @@ -117,6 +150,11 @@ export class Identities { return !identity || this.#now() - identity.at > TTL_MS } + #faceStale(did: string): boolean { + const identity = this.#entries.get(did) + return !identity?.avatarAt || this.#now() - identity.avatarAt > TTL_MS + } + #identity(did: string): Promise { const held = this.#entries.get(did) if (held && !this.#stale(did)) return Promise.resolve(held) @@ -124,9 +162,16 @@ export class Identities { if (running) return running const resolving = this.#resolve(did) .then((identity) => { - this.#entries.set(did, identity) + // The document facts are re-resolved; the face is on its own clock and rides across, so a + // handle refresh does not blank a picture that is still perfectly current. + const merged: Identity = { + ...(held?.avatar ? { avatar: held.avatar } : {}), + ...(held?.avatarAt ? { avatarAt: held.avatarAt } : {}), + ...identity, + } + this.#entries.set(did, merged) this.#save() - return identity + return merged }) .catch((error: unknown) => { // Keep a stale entry rather than losing a working PDS to one flaky lookup. @@ -152,4 +197,37 @@ export class Identities { } return identity } + + /** + * The picture, resolved separately from the document facts because it is wanted separately: the + * read path asks this cache for a PDS on every poll and never for a face, so reading a profile + * record there would be a request per repo per day spent on nothing anyone looks at. Only + * `resolve` — the display path, called with the members a screen is about to draw — asks for one. + */ + #face(did: string): Promise { + if (!this.#faceStale(did)) return Promise.resolve() + const running = this.#faces.get(did) + if (running) return running + const held = this.#entries.get(did) + const resolving = ( + held?.pds + ? resolveAvatarUrl(did, { fetcher: this.#fetcher, pds: held.pds }) + : Promise.resolve(undefined) + ) + .then((avatar) => { + // Re-read: the entry may have been replaced by a document refresh while this was in flight. + const current = this.#entries.get(did) ?? held + if (!current) return + const next: Identity = { ...current, avatarAt: this.#now() } + // A picture that has been taken down is a picture gone: the disc goes back to initials. + if (avatar) next.avatar = avatar + else delete next.avatar + this.#entries.set(did, next) + this.#save() + }) + .catch(() => undefined) + .finally(() => this.#faces.delete(did)) + this.#faces.set(did, resolving) + return resolving + } } diff --git a/packages/ui/src/lib/session.svelte.ts b/packages/ui/src/lib/session.svelte.ts index 81b4caf..9b32b6d 100644 --- a/packages/ui/src/lib/session.svelte.ts +++ b/packages/ui/src/lib/session.svelte.ts @@ -213,7 +213,7 @@ function publish(index: MaterializedIndex): void { session.space = { uri, index, - directory: buildDirectory(index, live?.identities.handles() ?? {}), + directory: buildDirectory(index, live?.identities.handles() ?? {}, live?.identities.avatars() ?? {}), // The instant the view is drawn at. Every "expires in 9 min" and "6m ago" on screen counts // against this one value, so a tick that changes nothing else still moves the clocks. asOf: new Date().toISOString(), @@ -222,7 +222,7 @@ function publish(index: MaterializedIndex): void { } } -/** One pass: poll every member repo, refold, resolve any handle we do not have yet. */ +/** One pass: poll every member repo, refold, resolve any identity we do not know yet. */ export async function sync(): Promise { const current = live if (!current || current.syncing || current.stopped) return diff --git a/packages/ui/src/lib/session.test.ts b/packages/ui/src/lib/session.test.ts index d57cf32..5957d69 100644 --- a/packages/ui/src/lib/session.test.ts +++ b/packages/ui/src/lib/session.test.ts @@ -45,8 +45,12 @@ const open = (harness: Harness): Promise => poll: false, }) -const harness = (records = currentVersions(), handles = HANDLES): Harness => ({ - network: new FakePds(records, handles), +const harness = ( + records = currentVersions(), + handles = HANDLES, + profiles: Record = {}, +): Harness => ({ + network: new FakePds(records, handles, profiles), database: memoryDatabase(), storage: memoryStorage(), }) @@ -107,6 +111,34 @@ describe('opening a live space', () => { expect(space.directory.get(FIXTURE_DIDS.ana).name).toBe('a1s2d3f4…') }) + it('shows the picture a member publishes, and the derived disc for one who does not', async () => { + // A profile picture is not a Radial record: it is read from the member's own repo, the same + // place their handle comes from, and it is an overlay on the directory rather than a field the + // index carries. An identity with no profile record keeps the colour-and-initials disc, so the + // fallback is not an error path — it is what most agents will always look like. + const live = harness(currentVersions(), HANDLES, { [FIXTURE_DIDS.tim]: 'bafyavatartim' }) + await open(live) + const space = session.space + if (!space) throw new Error('no space') + + expect(space.directory.get(FIXTURE_DIDS.tim).avatar).toBe( + `https://cdn.bsky.app/img/avatar_thumbnail/plain/${encodeURIComponent(FIXTURE_DIDS.tim)}/bafyavatartim@jpeg`, + ) + const planner = space.directory.get(FIXTURE_DIDS.planner) + expect(planner.avatar).toBeUndefined() + expect(planner.initials).toBe('PL') + + // One profile read per member on a cold scan, and none on the next tick: the answer is cached + // beside the handle and the PDS. + const profileReads = live.network.calls.filter((call) => + call.detail.startsWith('app.bsky.actor.profile'), + ) + expect(profileReads).toHaveLength(space.index.members.filter((member) => member.active).length) + live.network.reset() + await sync() + expect(live.network.counts('getRecord')).toBe(0) + }) + it('refuses a URI that does not name a space record', async () => { await openSpace('at://did:plc:qv7hjr2mzk4x/com.disnetdev.radial.goal/abc') expect(session.status).toBe('choosing') -- 2.51.2