malpercio.dev / atbb
WIP! A BB-style forum, on the ATmosphere! We're still working... we'll be back soon when we have something to show off!
node typescript hono htmx atproto
fork atom

fix(atb-61): address PR review feedback on preset theme implementation

- Fix rkey regex to allow hyphens (/^[a-z0-9-]+$/i) — all 5 preset rkeys
contain hyphens (neobrutal-light, clean-dark, etc.) and were silently
falling back to the hardcoded theme
- Add live-ref resolveTheme test verifying CID check is skipped when
expectedCid is null (canonical atbb.space presets ship without CID)
- Add ThemePolicy indexer tests verifying flat .uri field access and
null themeCid for live refs
- Fix bare catch in publish-presets.ts to only swallow 404 (record not
found) and rethrow all other errors
- Add isRecordCurrent() helper including name/colorScheme in change
detection, not just tokens
- Replace non-null assertions on .find() in admin.ts with explicit guards
- Log existing themePolicy before overwriting in bootstrap-local-presets.ts
- Update Bruno Update Theme Policy docs: remove stale CID-lookup comment

malpercio.dev 6341d8b7 745a8511

+207 -9
+17
apps/appview/scripts/bootstrap-local-presets.ts
··· 93 93 const darkUri = localUris.find((t) => t.rkey === "neobrutal-dark")!.uri; 94 94 const available = localUris.map((t) => ({ uri: t.uri })); 95 95 96 + // Show existing themePolicy before overwriting so the operator can see what will change 97 + try { 98 + const existing = await agent.com.atproto.repo.getRecord({ 99 + repo: did, 100 + collection: "space.atbb.forum.themePolicy", 101 + rkey: "self", 102 + }); 103 + const rec = existing.data.value as Record<string, unknown>; 104 + const existingLight = (rec.defaultLightTheme as Record<string, string> | undefined)?.uri; 105 + const existingDark = (rec.defaultDarkTheme as Record<string, string> | undefined)?.uri; 106 + console.log("\nExisting themePolicy:"); 107 + console.log(` defaultLightTheme: ${existingLight ?? "(none)"}`); 108 + console.log(` defaultDarkTheme: ${existingDark ?? "(none)"}`); 109 + } catch { 110 + console.log("\nNo existing themePolicy found — will create."); 111 + } 112 + 96 113 console.log("\nWriting themePolicy with local refs..."); 97 114 98 115 if (isDryRun) {
+20 -4
apps/appview/scripts/publish-presets.ts
··· 39 39 process.exit(1); 40 40 } 41 41 42 - /** Stable JSON string for token comparison — sorted keys, ignores ordering differences. */ 42 + /** Stable JSON string for comparison — sorted token keys, ignores ordering differences. */ 43 43 function stableTokensJson(tokens: Record<string, string>): string { 44 44 return JSON.stringify(Object.fromEntries(Object.entries(tokens).sort())); 45 + } 46 + 47 + /** True if the existing PDS record has the same content as the local preset. */ 48 + function isRecordCurrent( 49 + existing: Record<string, unknown>, 50 + preset: { name: string; colorScheme: string }, 51 + tokens: Record<string, string> 52 + ): boolean { 53 + return ( 54 + existing.name === preset.name && 55 + existing.colorScheme === preset.colorScheme && 56 + stableTokensJson(existing.tokens as Record<string, string>) === stableTokensJson(tokens) 57 + ); 45 58 } 46 59 47 60 async function run() { ··· 78 91 const existing = res.data.value as Record<string, unknown>; 79 92 existingCreatedAt = (existing.createdAt as string) ?? null; 80 93 81 - if (stableTokensJson(existing.tokens as Record<string, string>) === stableTokensJson(tokens)) { 94 + if (isRecordCurrent(existing, preset, tokens)) { 82 95 alreadyCurrent = true; 83 96 } 84 - } catch { 85 - // Record doesn't exist yet — will create 97 + } catch (err: unknown) { 98 + // Only swallow 404 — record doesn't exist yet and will be created. 99 + // Re-throw anything else (network errors, auth failures, etc.). 100 + const status = (err as Record<string, unknown>).status; 101 + if (status !== 404) throw err; 86 102 } 87 103 88 104 if (alreadyCurrent) {
+134
apps/appview/src/lib/__tests__/indexer.test.ts
··· 1287 1287 }); 1288 1288 1289 1289 1290 + describe("ThemePolicy Handler", () => { 1291 + /** 1292 + * Creates a tracking DB for themePolicy tests. 1293 + * ThemePolicy's genericCreate path uses afterUpsert, which requires: 1294 + * 1st insert (themePolicies): .values().returning([{id}]) 1295 + * delete (themePolicyAvailableThemes): .where() 1296 + * 2nd insert (themePolicyAvailableThemes): .values() 1297 + */ 1298 + function createThemePolicyTrackingDb() { 1299 + let insertCallCount = 0; 1300 + let policyInsertValues: any = null; 1301 + let availableThemesInsertValues: any = null; 1302 + 1303 + const db = { 1304 + transaction: vi.fn().mockImplementation(async (callback: (tx: any) => Promise<any>) => { 1305 + const tx = { 1306 + insert: vi.fn().mockImplementation(() => ({ 1307 + values: vi.fn().mockImplementation((vals: any) => { 1308 + insertCallCount++; 1309 + if (insertCallCount === 1) { 1310 + policyInsertValues = vals; 1311 + return { returning: vi.fn().mockResolvedValue([{ id: 1n }]) }; 1312 + } 1313 + availableThemesInsertValues = vals; 1314 + return Promise.resolve(undefined); 1315 + }), 1316 + })), 1317 + delete: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) }), 1318 + update: vi.fn(), 1319 + select: vi.fn().mockReturnValue({ 1320 + from: vi.fn().mockReturnValue({ 1321 + where: vi.fn().mockReturnValue({ limit: vi.fn().mockResolvedValue([]) }), 1322 + }), 1323 + }), 1324 + }; 1325 + return await callback(tx); 1326 + }), 1327 + } as unknown as Database; 1328 + 1329 + return { 1330 + db, 1331 + getPolicyInsertValues: () => policyInsertValues, 1332 + getAvailableThemesInsertValues: () => availableThemesInsertValues, 1333 + }; 1334 + } 1335 + 1336 + it("indexes themePolicy with flat themeRef URIs — live refs (no CID)", async () => { 1337 + // This test verifies the field access uses .uri directly (not .theme.uri from old strongRef). 1338 + // If the old .defaultLightTheme.theme.uri path were used, this would throw TypeError. 1339 + const { db: trackingDb, getPolicyInsertValues, getAvailableThemesInsertValues } = 1340 + createThemePolicyTrackingDb(); 1341 + const themePolicyIndexer = new Indexer(trackingDb, mockLogger); 1342 + 1343 + const event: CommitCreateEvent<"space.atbb.forum.themePolicy"> = { 1344 + did: "did:plc:forum", 1345 + time_us: 1234567890, 1346 + kind: "commit", 1347 + commit: { 1348 + rev: "abc", 1349 + operation: "create", 1350 + collection: "space.atbb.forum.themePolicy", 1351 + rkey: "self", 1352 + cid: "cidPolicy", 1353 + record: { 1354 + $type: "space.atbb.forum.themePolicy", 1355 + defaultLightTheme: { uri: "at://did:web:atbb.space/space.atbb.forum.theme/neobrutal-light" }, 1356 + defaultDarkTheme: { uri: "at://did:web:atbb.space/space.atbb.forum.theme/neobrutal-dark" }, 1357 + availableThemes: [ 1358 + { uri: "at://did:web:atbb.space/space.atbb.forum.theme/neobrutal-light" }, 1359 + { uri: "at://did:web:atbb.space/space.atbb.forum.theme/neobrutal-dark" }, 1360 + ], 1361 + allowUserChoice: true, 1362 + updatedAt: "2026-01-01T00:00:00Z", 1363 + } as any, 1364 + }, 1365 + }; 1366 + 1367 + await expect(themePolicyIndexer.handleThemePolicyCreate(event)).resolves.not.toThrow(); 1368 + 1369 + const policyVals = getPolicyInsertValues(); 1370 + expect(policyVals).toBeDefined(); 1371 + expect(policyVals.defaultLightThemeUri).toBe( 1372 + "at://did:web:atbb.space/space.atbb.forum.theme/neobrutal-light" 1373 + ); 1374 + expect(policyVals.defaultDarkThemeUri).toBe( 1375 + "at://did:web:atbb.space/space.atbb.forum.theme/neobrutal-dark" 1376 + ); 1377 + 1378 + const availableVals = getAvailableThemesInsertValues(); 1379 + expect(availableVals).toBeDefined(); 1380 + expect(availableVals[0].themeUri).toBe( 1381 + "at://did:web:atbb.space/space.atbb.forum.theme/neobrutal-light" 1382 + ); 1383 + // Live refs: no CID in record → themeCid must be null in DB row 1384 + expect(availableVals[0].themeCid).toBeNull(); 1385 + }); 1386 + 1387 + it("indexes themePolicy with pinned themeRefs (CID present)", async () => { 1388 + const { db: trackingDb, getAvailableThemesInsertValues } = 1389 + createThemePolicyTrackingDb(); 1390 + const themePolicyIndexer = new Indexer(trackingDb, mockLogger); 1391 + 1392 + const event: CommitCreateEvent<"space.atbb.forum.themePolicy"> = { 1393 + did: "did:plc:forum", 1394 + time_us: 1234567890, 1395 + kind: "commit", 1396 + commit: { 1397 + rev: "abc", 1398 + operation: "create", 1399 + collection: "space.atbb.forum.themePolicy", 1400 + rkey: "self", 1401 + cid: "cidPolicy2", 1402 + record: { 1403 + $type: "space.atbb.forum.themePolicy", 1404 + defaultLightTheme: { uri: "at://did:plc:forum/space.atbb.forum.theme/light1", cid: "bafylight" }, 1405 + defaultDarkTheme: { uri: "at://did:plc:forum/space.atbb.forum.theme/dark1", cid: "bafydark" }, 1406 + availableThemes: [ 1407 + { uri: "at://did:plc:forum/space.atbb.forum.theme/light1", cid: "bafylight" }, 1408 + { uri: "at://did:plc:forum/space.atbb.forum.theme/dark1", cid: "bafydark" }, 1409 + ], 1410 + allowUserChoice: false, 1411 + updatedAt: "2026-01-01T00:00:00Z", 1412 + } as any, 1413 + }, 1414 + }; 1415 + 1416 + await themePolicyIndexer.handleThemePolicyCreate(event); 1417 + 1418 + const availableVals = getAvailableThemesInsertValues(); 1419 + expect(availableVals[0].themeCid).toBe("bafylight"); 1420 + expect(availableVals[1].themeCid).toBe("bafydark"); 1421 + }); 1422 + }); 1423 + 1290 1424 describe("Ban enforcement — handleModActionCreate", () => { 1291 1425 it("calls applyBan when a ban mod action is created", async () => { 1292 1426 const mockBanEnforcer = (indexer as any).banEnforcer;
+6 -2
apps/appview/src/routes/admin.ts
··· 1487 1487 cid: typeof t.cid === "string" && t.cid !== "" ? t.cid : undefined, 1488 1488 })); 1489 1489 1490 - const lightTheme = resolvedThemes.find((t) => t.uri === defaultLightThemeUri)!; 1491 - const darkTheme = resolvedThemes.find((t) => t.uri === defaultDarkThemeUri)!; 1490 + const lightTheme = resolvedThemes.find((t) => t.uri === defaultLightThemeUri); 1491 + const darkTheme = resolvedThemes.find((t) => t.uri === defaultDarkThemeUri); 1492 + if (!lightTheme || !darkTheme) { 1493 + // Both URIs were validated as present in availableThemes above — this is unreachable. 1494 + return c.json({ error: "Internal error: theme URIs not found in resolved themes" }, 500); 1495 + } 1492 1496 1493 1497 const { agent, error: agentError } = getForumAgentOrError(ctx, c, "PUT /api/admin/theme-policy"); 1494 1498 if (agentError) return agentError;
+27 -1
apps/web/src/lib/__tests__/theme-resolution.test.ts
··· 306 306 }); 307 307 308 308 it("returns FALLBACK_THEME when rkey contains path traversal characters", async () => { 309 - // rkey extracted from URI would be "..%2Fsecret" after split — fails /^[a-z0-9]+$/i 309 + // parseRkeyFromUri("at://did/col/../../secret") splits on "/" and returns parts[4] = ".." 310 + // ".." fails /^[a-z0-9-]+$/i, so we return FALLBACK_THEME without a theme fetch 310 311 mockFetch.mockResolvedValueOnce( 311 312 policyResponse({ 312 313 defaultLightThemeUri: "at://did/col/../../secret", ··· 316 317 expect(result.tokens).toEqual(FALLBACK_THEME.tokens); 317 318 // Only the policy fetch should have been made (no theme fetch) 318 319 expect(mockFetch).toHaveBeenCalledTimes(1); 320 + }); 321 + 322 + it("resolves theme from live ref (no CID in policy) without logging CID mismatch", async () => { 323 + // Live refs have no CID — canonical atbb.space presets ship this way. 324 + // The CID integrity check must be skipped when expectedCid is null. 325 + mockFetch 326 + .mockResolvedValueOnce( 327 + policyResponse({ 328 + availableThemes: [ 329 + { uri: "at://did:plc:forum/space.atbb.forum.theme/3lbllight" }, // no cid 330 + { uri: "at://did:plc:forum/space.atbb.forum.theme/3lbldark" }, // no cid 331 + ], 332 + }) 333 + ) 334 + .mockResolvedValueOnce(themeResponse("light", "bafylight")); 335 + 336 + const result = await resolveTheme(APPVIEW, undefined, undefined); 337 + 338 + // Theme resolved successfully — live ref does not trigger CID mismatch 339 + expect(result.tokens["color-bg"]).toBe("#fff"); 340 + expect(result.colorScheme).toBe("light"); 341 + expect(mockLogger.warn).not.toHaveBeenCalledWith( 342 + expect.stringContaining("CID mismatch"), 343 + expect.any(Object) 344 + ); 319 345 }); 320 346 });
+1 -1
apps/web/src/lib/theme-resolution.ts
··· 121 121 if (!defaultUri) return { ...FALLBACK_THEME, colorScheme }; 122 122 123 123 const rkey = parseRkeyFromUri(defaultUri); 124 - if (!rkey || !/^[a-z0-9]+$/i.test(rkey)) return { ...FALLBACK_THEME, colorScheme }; 124 + if (!rkey || !/^[a-z0-9-]+$/i.test(rkey)) return { ...FALLBACK_THEME, colorScheme }; 125 125 126 126 const matchingTheme = policy.availableThemes.find((t) => t.uri === defaultUri); 127 127 if (!matchingTheme) {
+2 -1
bruno/AppView API/Admin Themes/Update Theme Policy.bru
··· 34 34 35 35 Body: 36 36 - availableThemes (required): Non-empty array of { uri, cid? } theme references. 37 - cid is optional — if omitted, the AppView looks it up from the themes table by URI. 37 + cid is optional — if omitted, this is a live ref that always resolves to the current 38 + version of the theme record. Canonical atbb.space presets use live refs by default. 38 39 Both defaultLightThemeUri and defaultDarkThemeUri must be present in this list. 39 40 - defaultLightThemeUri (required): AT-URI of the default light-mode theme. 40 41 Must be in availableThemes.