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

fix(appview): assign default Member role on first login (#58)

* fix(appview): assign default Member role when creating membership on first login

Membership PDS records were written without a role reference, causing the
firehose to index roleUri as null. The permission middleware fails closed on
null roleUri, so all newly-registered users got 403 on every post attempt.

Now looks up the seeded "Member" role and includes it as a strongRef in the
membership record at creation time. Falls back gracefully (no role field) if
the Member role is not yet in the DB.

* fix(appview): address code review feedback on default-member-role fix

- Log ctx.logger.error when Member role not found in DB (operator alert)
- Wrap role lookup in try-catch; log ctx.logger.warn and proceed without
role on transient DB errors, so membership is still created
- Add orderBy(asc(roles.indexedAt)) to make role selection deterministic
when duplicate Member roles exist
- Rename test DIDs to use did:plc:test-* prefix per established cleanup patterns
- Add test asserting logger.error fires when Member role is absent
- Add test asserting membership is created without role on DB error

* fix(appview): re-throw programming errors from role lookup catch block

Per CLAUDE.md error handling standards, TypeError/ReferenceError/SyntaxError
indicate code bugs and must not be silently swallowed. Adds re-throw guard
before the warn log so transient DB errors are handled gracefully while
programming errors surface during development.

Adds test verifying TypeError propagates rather than being caught.

authored by

Malpercio and committed by
GitHub
988af01b df9f1b81

+220 -13
+167 -1
apps/appview/src/lib/__tests__/membership.test.ts
··· 1 1 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; 2 2 import { createMembershipForUser } from "../membership.js"; 3 3 import { createTestContext, type TestContext } from "./test-context.js"; 4 - import { memberships, users } from "@atbb/db"; 4 + import { memberships, users, roles } from "@atbb/db"; 5 5 import { eq, and } from "drizzle-orm"; 6 6 7 7 describe("createMembershipForUser", () => { ··· 237 237 testDid 238 238 ); 239 239 expect(result2.created).toBe(false); 240 + }); 241 + 242 + it("includes Member role in new membership PDS record when Member role exists in DB", async () => { 243 + const memberRoleRkey = "memberrole123"; 244 + const memberRoleCid = "bafymemberrole456"; 245 + 246 + await ctx.db.insert(roles).values({ 247 + did: ctx.config.forumDid, 248 + rkey: memberRoleRkey, 249 + cid: memberRoleCid, 250 + name: "Member", 251 + description: "Regular forum member", 252 + permissions: ["space.atbb.permission.createTopics", "space.atbb.permission.createPosts"], 253 + priority: 30, 254 + createdAt: new Date(), 255 + indexedAt: new Date(), 256 + }); 257 + 258 + const mockAgent = { 259 + com: { 260 + atproto: { 261 + repo: { 262 + putRecord: vi.fn().mockResolvedValue({ 263 + data: { 264 + uri: "at://did:plc:test-new-member/space.atbb.membership/tid789", 265 + cid: "bafynewmember", 266 + }, 267 + }), 268 + }, 269 + }, 270 + }, 271 + } as any; 272 + 273 + await createMembershipForUser(ctx, mockAgent, "did:plc:test-new-member"); 274 + 275 + expect(mockAgent.com.atproto.repo.putRecord).toHaveBeenCalledWith( 276 + expect.objectContaining({ 277 + record: expect.objectContaining({ 278 + role: { 279 + role: { 280 + uri: `at://${ctx.config.forumDid}/space.atbb.forum.role/${memberRoleRkey}`, 281 + cid: memberRoleCid, 282 + }, 283 + }, 284 + }), 285 + }) 286 + ); 287 + }); 288 + 289 + it("logs error and creates membership without role when Member role not found in DB", async () => { 290 + // No roles seeded — Member role absent 291 + const errorSpy = vi.spyOn(ctx.logger, "error"); 292 + 293 + const mockAgent = { 294 + com: { 295 + atproto: { 296 + repo: { 297 + putRecord: vi.fn().mockResolvedValue({ 298 + data: { 299 + uri: "at://did:plc:test-no-role/space.atbb.membership/tid000", 300 + cid: "bafynorole", 301 + }, 302 + }), 303 + }, 304 + }, 305 + }, 306 + } as any; 307 + 308 + const result = await createMembershipForUser(ctx, mockAgent, "did:plc:test-no-role"); 309 + 310 + expect(result.created).toBe(true); 311 + const callArg = mockAgent.com.atproto.repo.putRecord.mock.calls[0][0]; 312 + expect(callArg.record.role).toBeUndefined(); 313 + expect(errorSpy).toHaveBeenCalledWith( 314 + expect.stringContaining("Member role not found"), 315 + expect.objectContaining({ operation: "createMembershipForUser" }) 316 + ); 317 + }); 318 + 319 + it("creates membership without role when role lookup DB error occurs", async () => { 320 + // Simulate a transient DB error on the roles query (3rd select call). 321 + // Forum and membership queries must succeed; only the role lookup fails. 322 + const origSelect = ctx.db.select.bind(ctx.db); 323 + vi.spyOn(ctx.db, "select") 324 + .mockImplementationOnce(() => origSelect() as any) // forums lookup 325 + .mockImplementationOnce(() => origSelect() as any) // memberships check 326 + .mockReturnValueOnce({ // roles query — DB error 327 + from: vi.fn().mockReturnValue({ 328 + where: vi.fn().mockReturnValue({ 329 + orderBy: vi.fn().mockReturnValue({ 330 + limit: vi.fn().mockRejectedValue(new Error("DB connection lost")), 331 + }), 332 + }), 333 + }), 334 + } as any); 335 + 336 + const warnSpy = vi.spyOn(ctx.logger, "warn"); 337 + 338 + const mockAgent = { 339 + com: { 340 + atproto: { 341 + repo: { 342 + putRecord: vi.fn().mockResolvedValue({ 343 + data: { 344 + uri: "at://did:plc:test-role-err/space.atbb.membership/tid999", 345 + cid: "bafyrole-err", 346 + }, 347 + }), 348 + }, 349 + }, 350 + }, 351 + } as any; 352 + 353 + const result = await createMembershipForUser(ctx, mockAgent, "did:plc:test-role-err"); 354 + 355 + expect(result.created).toBe(true); 356 + const callArg = mockAgent.com.atproto.repo.putRecord.mock.calls[0][0]; 357 + expect(callArg.record.role).toBeUndefined(); 358 + expect(warnSpy).toHaveBeenCalledWith( 359 + expect.stringContaining("role lookup"), 360 + expect.objectContaining({ operation: "createMembershipForUser" }) 361 + ); 362 + 363 + vi.restoreAllMocks(); 364 + }); 365 + 366 + it("re-throws TypeError from role lookup so programming errors are not silently swallowed", async () => { 367 + const origSelect = ctx.db.select.bind(ctx.db); 368 + vi.spyOn(ctx.db, "select") 369 + .mockImplementationOnce(() => origSelect() as any) // forums lookup 370 + .mockImplementationOnce(() => origSelect() as any) // memberships check 371 + .mockReturnValueOnce({ // roles query — TypeError 372 + from: vi.fn().mockReturnValue({ 373 + where: vi.fn().mockReturnValue({ 374 + orderBy: vi.fn().mockReturnValue({ 375 + limit: vi.fn().mockRejectedValue( 376 + new TypeError("Cannot read properties of undefined") 377 + ), 378 + }), 379 + }), 380 + }), 381 + } as any); 382 + 383 + // putRecord returns a valid response — the only TypeError in flight is the 384 + // one from the role lookup mock. If the catch block swallows it, the 385 + // function would return { created: true } instead of rejecting. 386 + const mockAgent = { 387 + com: { 388 + atproto: { 389 + repo: { 390 + putRecord: vi.fn().mockResolvedValue({ 391 + data: { 392 + uri: "at://did:plc:test-type-err/space.atbb.membership/tid111", 393 + cid: "bafytypeerr", 394 + }, 395 + }), 396 + }, 397 + }, 398 + }, 399 + } as any; 400 + 401 + await expect( 402 + createMembershipForUser(ctx, mockAgent, "did:plc:test-type-err") 403 + ).rejects.toThrow(TypeError); 404 + 405 + vi.restoreAllMocks(); 240 406 }); 241 407 242 408 it("upgrades bootstrap membership to real PDS record", async () => {
+53 -12
apps/appview/src/lib/membership.ts
··· 1 1 import type { AppContext } from "./app-context.js"; 2 2 import type { Agent } from "@atproto/api"; 3 - import { memberships, forums } from "@atbb/db"; 4 - import { eq, and } from "drizzle-orm"; 3 + import { memberships, forums, roles } from "@atbb/db"; 4 + import { eq, and, asc } from "drizzle-orm"; 5 5 import { TID } from "@atproto/common-web"; 6 6 7 7 export async function createMembershipForUser( ··· 42 42 return { created: false }; 43 43 } 44 44 45 - return writeMembershipRecord(agent, did, forumUri, forum.cid); 45 + // Look up the default "Member" role to assign on first login. 46 + // Wrapped in try-catch so a transient DB error does not prevent membership creation. 47 + let defaultRoleRef: { uri: string; cid: string } | null = null; 48 + try { 49 + const [memberRole] = await ctx.db 50 + .select({ rkey: roles.rkey, cid: roles.cid }) 51 + .from(roles) 52 + .where(and(eq(roles.did, ctx.config.forumDid), eq(roles.name, "Member"))) 53 + .orderBy(asc(roles.indexedAt)) 54 + .limit(1); 55 + 56 + if (memberRole) { 57 + defaultRoleRef = { 58 + uri: `at://${ctx.config.forumDid}/space.atbb.forum.role/${memberRole.rkey}`, 59 + cid: memberRole.cid, 60 + }; 61 + } else { 62 + ctx.logger.error("Member role not found in DB — creating membership without role. User will have no permissions. Run seedDefaultRoles to fix.", { 63 + operation: "createMembershipForUser", 64 + did, 65 + forumDid: ctx.config.forumDid, 66 + }); 67 + } 68 + } catch (error) { 69 + if (error instanceof TypeError || error instanceof ReferenceError || error instanceof SyntaxError) { 70 + throw error; 71 + } 72 + ctx.logger.warn("Member role lookup failed — creating membership without role", { 73 + operation: "createMembershipForUser", 74 + did, 75 + error: error instanceof Error ? error.message : String(error), 76 + }); 77 + } 78 + 79 + return writeMembershipRecord(agent, did, forumUri, forum.cid, defaultRoleRef); 46 80 } 47 81 48 82 async function writeMembershipRecord( 49 83 agent: Agent, 50 84 did: string, 51 85 forumUri: string, 52 - forumCid: string 86 + forumCid: string, 87 + defaultRoleRef: { uri: string; cid: string } | null = null 53 88 ): Promise<{ created: boolean; uri?: string; cid?: string }> { 54 89 const rkey = TID.nextStr(); 55 90 const now = new Date().toISOString(); 56 91 92 + const record: Record<string, unknown> = { 93 + $type: "space.atbb.membership", 94 + forum: { 95 + forum: { uri: forumUri, cid: forumCid }, 96 + }, 97 + createdAt: now, 98 + joinedAt: now, 99 + }; 100 + 101 + if (defaultRoleRef) { 102 + record.role = { role: { uri: defaultRoleRef.uri, cid: defaultRoleRef.cid } }; 103 + } 104 + 57 105 const result = await agent.com.atproto.repo.putRecord({ 58 106 repo: did, 59 107 collection: "space.atbb.membership", 60 108 rkey, 61 - record: { 62 - $type: "space.atbb.membership", 63 - forum: { 64 - forum: { uri: forumUri, cid: forumCid }, 65 - }, 66 - createdAt: now, 67 - joinedAt: now, 68 - }, 109 + record, 69 110 }); 70 111 71 112 return { created: true, uri: result.data.uri, cid: result.data.cid };