Let the identity directory supply appearance a provider lacks

src/identity/providers.ts

@@ -68,6 +68,33 @@ export class ConfiguredIdentityResolver implements IdentityResolver {
 	}
 }
 
+/**
+ * Fill a resolved identity's missing appearance from the configured directory.
+ *
+ * Who an ID belongs to and what they look like come from different places. A live provider knows
+ * the first; whether it knows the second depends on the account it signed in with - Relay carries
+ * a picture only when the OAuth provider supplied one, and a colour only for the local user. The
+ * configured directory is where someone records what a provider cannot tell them.
+ *
+ * So a provider wins on identity while the directory fills the gaps, rather than the first
+ * resolver with any answer at all taking the whole record. Otherwise directory entries are ignored
+ * for exactly the people a provider already recognises, which is normally all of them - and the
+ * setting appears to do nothing.
+ */
+export function withConfiguredDecoration(
+	identity: Identity,
+	configured: Identity | null,
+): Identity {
+	if (!configured) return identity;
+
+	return {
+		...identity,
+		picture: identity.picture ?? configured.picture,
+		color: identity.color ?? configured.color,
+		colorLight: identity.colorLight ?? configured.colorLight,
+	};
+}
+
 export type RelayIdentitySupportStatus =
 	| "not-installed"
 	| "unsupported"

src/main.ts

@@ -72,6 +72,7 @@ import {
 	getRelayIdentitySupportStatus,
 	providerById,
 	selectIdentityProvider,
+	withConfiguredDecoration,
 } from "./identity/providers";
 import type {
 	Identity,
@@ -1353,7 +1354,8 @@ export default class RelayCommentsPlugin
 			if (!resolver.isAvailable()) continue;
 			try {
 				const identity = await resolver.resolveUser(author, path);
-				if (identity) return providerIdentity(identity, resolver.id);
+				if (identity)
+					return this.decorated(identity, resolver, author, path);
 			} catch {
 				// Resolver failures degrade to the unresolved author value
 				// instead of breaking review rendering.
@@ -1364,6 +1366,47 @@ export default class RelayCommentsPlugin
 		return null;
 	}
 
+	/**
+	 * One resolver's answer, with appearance the directory carries and it does not.
+	 *
+	 * Skipped when the directory is itself the answering resolver, which has nothing to fill in
+	 * from.
+	 */
+	private decorated(
+		identity: Identity,
+		resolver: IdentityResolver,
+		author: string,
+		path: string,
+	): ReviewerIdentity {
+		if (resolver.id === this.configuredIdentityResolver.id)
+			return providerIdentity(identity, resolver.id);
+
+		return providerIdentity(
+			withConfiguredDecoration(
+				identity,
+				this.resolveConfiguredIdentity(author, path),
+			),
+			resolver.id,
+		);
+	}
+
+	/** The configured directory's record for an author, for decoration only. */
+	private resolveConfiguredIdentity(
+		author: string,
+		path: string,
+	): Identity | null {
+		if (!this.configuredIdentityResolver.isAvailable()) return null;
+
+		try {
+			return (
+				this.configuredIdentityResolver.resolveUserSnapshot?.(author, path) ??
+				null
+			);
+		} catch {
+			return null;
+		}
+	}
+
 	private resolveAuthorIdentitySnapshot(
 		author: string,
 		path: string,
@@ -1378,7 +1421,8 @@ export default class RelayCommentsPlugin
 			if (!resolver.resolveUserSnapshot) return undefined;
 			try {
 				const identity = resolver.resolveUserSnapshot(author, path);
-				if (identity) return providerIdentity(identity, resolver.id);
+				if (identity)
+					return this.decorated(identity, resolver, author, path);
 			} catch {
 				return undefined;
 			}

tests/unit/identity/providers.test.ts

@@ -7,8 +7,69 @@ import {
 	ObsidianSyncIdentityProvider,
 	RelayIdentityProvider,
 	selectIdentityProvider,
+	withConfiguredDecoration,
 } from "src/identity/providers";
 
+describe("configured decoration", () => {
+	it("supplies the appearance a provider does not carry", () => {
+		expect(
+			withConfiguredDecoration(
+				{ id: "u1", name: "Sarah Rilling" },
+				{
+					id: "u1",
+					name: "whatever the directory calls them",
+					picture: "https://example.com/sarah.jpg",
+					color: "#30bced",
+					colorLight: "#30bced33",
+				},
+			),
+		).toEqual({
+			id: "u1",
+			// The provider stays authoritative about who this is.
+			name: "Sarah Rilling",
+			picture: "https://example.com/sarah.jpg",
+			color: "#30bced",
+			colorLight: "#30bced33",
+		});
+	});
+
+	it("never overrides appearance a provider does carry", () => {
+		expect(
+			withConfiguredDecoration(
+				{ id: "u1", name: "Sarah", picture: "provider.jpg", color: "#111111" },
+				{
+					id: "u1",
+					name: "Sarah",
+					picture: "configured.jpg",
+					color: "#222222",
+				},
+			),
+		).toMatchObject({ picture: "provider.jpg", color: "#111111" });
+	});
+
+	it("leaves an identity alone when the directory has no entry", () => {
+		const identity = { id: "u1", name: "Sarah" };
+
+		expect(withConfiguredDecoration(identity, null)).toEqual(identity);
+	});
+
+	it("fills each field independently", () => {
+		// A directory entry that only sets a colour must not blank out a picture, and vice versa.
+		expect(
+			withConfiguredDecoration(
+				{ id: "u1", name: "Sarah", picture: "provider.jpg" },
+				{ id: "u1", name: "Sarah", color: "#30bced" },
+			),
+		).toEqual({
+			id: "u1",
+			name: "Sarah",
+			picture: "provider.jpg",
+			color: "#30bced",
+			colorLight: undefined,
+		});
+	});
+});
+
 describe("configured identity directory", () => {
 	it("resolves other authors without acting as the current user", async () => {
 		const provider = new ConfiguredIdentityResolver(() => [