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): add $type to reply ref so indexer resolves rootPostId/parentPostId (#61)

* fix(appview): add $type to reply ref so indexer resolves rootPostId/parentPostId

Post.isReplyRef() uses AT Protocol's is$typed() runtime guard which requires
$type: "space.atbb.post#replyRef" to be present. Without it the guard returned
false, leaving rootPostId/parentPostId null in the database and breaking reply
threading. Also adds an assertion to the happy-path test that verifies the
record written to the PDS includes the correct $type on the reply ref.

* fix(appview): address PR review feedback on reply ref $type fix

- Use Post.isReplyRef() in route tests instead of $type string literal,
so the actual indexer contract is tested (a typo in the string would
still break isReplyRef() but pass a literal comparison)
- Add isReplyRef() assertion to nested-reply test (creates reply to reply)
- Add regex URI assertions for stronger AT-URI shape validation
- Add indexer happy-path test: correctly-typed reply ref resolves
rootPostId/parentPostId to non-null values
- Upgrade logger.warn → logger.error for missing $type (data corruption,
not a warning — post is silently unreachable in thread navigation)
- Add errorId field to missing $type log entry for operator filtering
- Split outer try block in POST /api/posts: DB lookup and PDS write now
have separate catch blocks with accurate error messages (per CLAUDE.md
Try Block Granularity pattern)

authored by

Malpercio and committed by
GitHub
5d357ebb 9a0b4ec7

+105 -27
+51
apps/appview/src/lib/__tests__/indexer.test.ts
··· 251 expect(mockDb.insert).toHaveBeenCalled(); 252 }); 253 254 it("strips title when indexing a reply on create (title: null regardless of record)", async () => { 255 const { db: trackingDb, getInsertedValues } = createTrackingDb(); 256 const replyIndexer = new Indexer(trackingDb, mockLogger); ··· 291 expect(vals.parentPostId).toBeNull(); 292 expect(vals.rootUri).toBe(rootParentUri); 293 expect(vals.parentUri).toBe(rootParentUri); 294 }); 295 296 it("strips title when indexing a reply on update (title: null regardless of record)", async () => {
··· 251 expect(mockDb.insert).toHaveBeenCalled(); 252 }); 253 254 + it("resolves rootPostId and parentPostId when reply ref has correct $type", async () => { 255 + const mockPostId = 99n; 256 + const { db: trackingDb, getInsertedValues } = createTrackingDb([{ id: mockPostId }]); 257 + const replyIndexer = new Indexer(trackingDb, mockLogger); 258 + const rootUri = "at://did:plc:user1/space.atbb.post/topic1"; 259 + const parentUri = "at://did:plc:user1/space.atbb.post/reply1"; 260 + 261 + const event: CommitCreateEvent<"space.atbb.post"> = { 262 + did: "did:plc:test123", 263 + time_us: 1234567890, 264 + kind: "commit", 265 + commit: { 266 + rev: "abc", 267 + operation: "create", 268 + collection: "space.atbb.post", 269 + rkey: "reply2", 270 + cid: "cid789", 271 + record: { 272 + $type: "space.atbb.post", 273 + text: "A properly-typed reply", 274 + reply: { 275 + $type: "space.atbb.post#replyRef", 276 + root: { uri: rootUri, cid: "cidRoot" }, 277 + parent: { uri: parentUri, cid: "cidParent" }, 278 + }, 279 + createdAt: "2024-01-01T02:00:00Z", 280 + } as any, 281 + }, 282 + }; 283 + 284 + await replyIndexer.handlePostCreate(event); 285 + 286 + const vals = getInsertedValues(); 287 + expect(vals).toBeDefined(); 288 + // Correctly-typed reply ref: IDs must be resolved to non-null values 289 + expect(vals.rootPostId).toBe(mockPostId); 290 + expect(vals.parentPostId).toBe(mockPostId); 291 + expect(vals.rootUri).toBe(rootUri); 292 + expect(vals.parentUri).toBe(parentUri); 293 + // No error should be logged for a well-formed reply 294 + expect(mockLogger.error).not.toHaveBeenCalledWith( 295 + expect.stringContaining("reply ref missing $type"), 296 + expect.any(Object) 297 + ); 298 + }); 299 + 300 it("strips title when indexing a reply on create (title: null regardless of record)", async () => { 301 const { db: trackingDb, getInsertedValues } = createTrackingDb(); 302 const replyIndexer = new Indexer(trackingDb, mockLogger); ··· 337 expect(vals.parentPostId).toBeNull(); 338 expect(vals.rootUri).toBe(rootParentUri); 339 expect(vals.parentUri).toBe(rootParentUri); 340 + // Operators must be alerted — a post with null thread IDs is silently unreachable 341 + expect(mockLogger.error).toHaveBeenCalledWith( 342 + expect.stringContaining("reply ref missing $type"), 343 + expect.objectContaining({ errorId: "POST_REPLY_REF_MISSING_TYPE" }) 344 + ); 345 }); 346 347 it("strips title when indexing a reply on update (title: null regardless of record)", async () => {
+4 -2
apps/appview/src/lib/indexer.ts
··· 92 rootId = await this.getPostIdByUri(record.reply.root.uri, tx); 93 parentId = await this.getPostIdByUri(record.reply.parent.uri, tx); 94 } else if (record.reply) { 95 - // reply ref present but $type omitted — rootPostId/parentPostId will be null 96 - this.logger.warn("Post reply ref missing $type — rootPostId/parentPostId not resolved", { 97 operation: "Post CREATE", 98 postDid: event.did, 99 postRkey: event.commit.rkey, 100 }); 101 } 102
··· 92 rootId = await this.getPostIdByUri(record.reply.root.uri, tx); 93 parentId = await this.getPostIdByUri(record.reply.parent.uri, tx); 94 } else if (record.reply) { 95 + // reply ref present but $type omitted — rootPostId/parentPostId will be null, 96 + // making this reply unreachable in thread navigation (data corruption). 97 + this.logger.error("Post reply ref missing $type — rootPostId/parentPostId not resolved", { 98 operation: "Post CREATE", 99 postDid: event.did, 100 postRkey: event.commit.rkey, 101 + errorId: "POST_REPLY_REF_MISSING_TYPE", 102 }); 103 } 104
+14
apps/appview/src/routes/__tests__/posts.test.ts
··· 4 import type { Variables } from "../../types.js"; 5 import { posts, users, modActions } from "@atbb/db"; 6 import { TID } from "@atproto/common-web"; 7 8 // Mock auth and permission middleware at the module level 9 let mockPutRecord: ReturnType<typeof vi.fn>; ··· 131 expect(data.uri).toBeTruthy(); 132 expect(data.cid).toBeTruthy(); 133 expect(data.rkey).toBeTruthy(); 134 }); 135 136 it("creates reply to reply", async () => { ··· 145 }); 146 147 expect(res.status).toBe(201); 148 }); 149 150 it("returns 400 for invalid parent ID format", async () => {
··· 4 import type { Variables } from "../../types.js"; 5 import { posts, users, modActions } from "@atbb/db"; 6 import { TID } from "@atproto/common-web"; 7 + import { SpaceAtbbPost as Post } from "@atbb/lexicon"; 8 9 // Mock auth and permission middleware at the module level 10 let mockPutRecord: ReturnType<typeof vi.fn>; ··· 132 expect(data.uri).toBeTruthy(); 133 expect(data.cid).toBeTruthy(); 134 expect(data.rkey).toBeTruthy(); 135 + 136 + // Verify the reply ref written to PDS passes Post.isReplyRef() — the actual 137 + // contract the indexer uses. A string literal check on $type would pass even 138 + // with a typo that still breaks the indexer's runtime guard. 139 + const putCall = mockPutRecord.mock.calls[0][0]; 140 + expect(Post.isReplyRef(putCall.record.reply)).toBe(true); 141 + expect(putCall.record.reply.root.uri).toMatch(/^at:\/\/did:plc:.*\/space\.atbb\.post\//); 142 + expect(putCall.record.reply.parent.uri).toMatch(/^at:\/\/did:plc:.*\/space\.atbb\.post\//); 143 }); 144 145 it("creates reply to reply", async () => { ··· 154 }); 155 156 expect(res.status).toBe(201); 157 + 158 + // Same isReplyRef contract check as the direct-reply case — nested replies 159 + // use the same construction path and must also include $type. 160 + const putCall = mockPutRecord.mock.calls[0][0]; 161 + expect(Post.isReplyRef(putCall.record.reply)).toBe(true); 162 }); 163 164 it("returns 400 for invalid parent ID format", async () => {
+36 -25
apps/appview/src/routes/posts.ts
··· 60 }); 61 } 62 63 try { 64 - // Look up root and parent posts 65 - const postsMap = await getPostsByIds(ctx.db, [rootId, parentId]); 66 67 - const root = postsMap.get(rootId); 68 - const parent = postsMap.get(parentId); 69 70 - if (!root) { 71 - return c.json({ error: "Root post not found" }, 404); 72 - } 73 74 - if (!parent) { 75 - return c.json({ error: "Parent post not found" }, 404); 76 - } 77 78 - // Validate parent belongs to same thread 79 - const parentValidation = validateReplyParent(root, parent, rootId); 80 - if (!parentValidation.valid) { 81 - return c.json({ error: parentValidation.error }, 400); 82 - } 83 84 - // Validate root post has forum reference 85 - if (!root.forumUri) { 86 - return c.json({ error: "Root post has no forum reference" }, 400); 87 - } 88 89 - // Construct AT-URIs 90 - const rootUri = `at://${root.did}/space.atbb.post/${root.rkey}`; 91 - const parentUri = `at://${parent.did}/space.atbb.post/${parent.rkey}`; 92 93 - // Generate TID for rkey 94 - const rkey = TID.nextStr(); 95 96 - // Write to user's PDS 97 const result = await user.agent.com.atproto.repo.putRecord({ 98 repo: user.did, 99 collection: "space.atbb.post", ··· 105 forum: { uri: root.forumUri, cid: root.cid }, 106 }, 107 reply: { 108 root: { uri: rootUri, cid: root.cid }, 109 parent: { uri: parentUri, cid: parent.cid }, 110 },
··· 60 }); 61 } 62 63 + // Look up root and parent posts 64 + let postsMap: Awaited<ReturnType<typeof getPostsByIds>>; 65 try { 66 + postsMap = await getPostsByIds(ctx.db, [rootId, parentId]); 67 + } catch (error) { 68 + return handleWriteError(c, error, "Failed to look up posts for reply", { 69 + operation: "POST /api/posts - post lookup", 70 + logger: ctx.logger, 71 + userId: user.did, 72 + rootId: rootIdStr, 73 + parentId: parentIdStr, 74 + }); 75 + } 76 77 + const root = postsMap.get(rootId); 78 + const parent = postsMap.get(parentId); 79 80 + if (!root) { 81 + return c.json({ error: "Root post not found" }, 404); 82 + } 83 84 + if (!parent) { 85 + return c.json({ error: "Parent post not found" }, 404); 86 + } 87 88 + // Validate parent belongs to same thread 89 + const parentValidation = validateReplyParent(root, parent, rootId); 90 + if (!parentValidation.valid) { 91 + return c.json({ error: parentValidation.error }, 400); 92 + } 93 94 + // Validate root post has forum reference 95 + if (!root.forumUri) { 96 + return c.json({ error: "Root post has no forum reference" }, 400); 97 + } 98 99 + const rootUri = `at://${root.did}/space.atbb.post/${root.rkey}`; 100 + const parentUri = `at://${parent.did}/space.atbb.post/${parent.rkey}`; 101 102 + // Generate TID for rkey 103 + const rkey = TID.nextStr(); 104 105 + // Write to user's PDS 106 + try { 107 const result = await user.agent.com.atproto.repo.putRecord({ 108 repo: user.did, 109 collection: "space.atbb.post", ··· 115 forum: { uri: root.forumUri, cid: root.cid }, 116 }, 117 reply: { 118 + $type: "space.atbb.post#replyRef", 119 root: { uri: rootUri, cid: root.cid }, 120 parent: { uri: parentUri, cid: parent.cid }, 121 },