From 7ec5efdb40be380854ef9d1e43c9b753f70d8570 Mon Sep 17 00:00:00 2001 From: juprodh Date: Wed, 8 Jul 2026 17:07:22 +0800 Subject: [PATCH] Move to community lexicons bookmark Lichen bookmarks are now obsolete --- lexicons/wiki.lichen.bookmark.json | 27 ------- lexicons/wiki.lichen.permissions.json | 3 +- src/atproto/pds.ts | 8 +-- src/firehose/handlers.ts | 25 ++++--- src/lib/collections.ts | 7 +- src/lib/orchestrators/bookmark.ts | 25 ++++--- tests/atproto/pds.test.ts | 10 +-- tests/firehose/handlers.test.ts | 91 +++++++++++++++--------- tests/lib/orchestrators/bookmark.test.ts | 51 +++++++++---- tests/server/routes/helpers.ts | 2 +- 10 files changed, 141 insertions(+), 108 deletions(-) delete mode 100644 lexicons/wiki.lichen.bookmark.json diff --git a/lexicons/wiki.lichen.bookmark.json b/lexicons/wiki.lichen.bookmark.json deleted file mode 100644 index a39f9ee..0000000 --- a/lexicons/wiki.lichen.bookmark.json +++ /dev/null @@ -1,27 +0,0 @@ -{ - "lexicon": 1, - "id": "wiki.lichen.bookmark", - "defs": { - "main": { - "type": "record", - "description": "A bookmark for a wiki. Stored on the user's PDS, portable across appviews.", - "key": "tid", - "record": { - "type": "object", - "required": ["wikiRef", "createdAt"], - "properties": { - "wikiRef": { - "type": "string", - "format": "at-uri", - "description": "AT-URI of the bookmarked wiki." - }, - "createdAt": { - "type": "string", - "format": "datetime", - "description": "When the bookmark was created." - } - } - } - } - } -} diff --git a/lexicons/wiki.lichen.permissions.json b/lexicons/wiki.lichen.permissions.json index 247edc8..3d037f4 100644 --- a/lexicons/wiki.lichen.permissions.json +++ b/lexicons/wiki.lichen.permissions.json @@ -15,8 +15,7 @@ "wiki.lichen.note", "wiki.lichen.noteRevision", "wiki.lichen.membership", - "wiki.lichen.memberRequest", - "wiki.lichen.bookmark" + "wiki.lichen.memberRequest" ] } ] diff --git a/src/atproto/pds.ts b/src/atproto/pds.ts index 816ec52..e9058e0 100644 --- a/src/atproto/pds.ts +++ b/src/atproto/pds.ts @@ -128,15 +128,15 @@ export function writeMemberRequestRecord( }); } -export function writeBookmarkRecord( +export function writeCommunityBookmarkRecord( rpc: Client, did: string, tid: string, - wikiRef: string, + subject: string, createdAt: string, ): Promise { - return putRecord(rpc, did, COLLECTIONS.bookmark, tid, { - wikiRef, + return putRecord(rpc, did, COLLECTIONS.communityBookmark, tid, { + subject, createdAt, }); } diff --git a/src/firehose/handlers.ts b/src/firehose/handlers.ts index 47f87dc..954a10a 100644 --- a/src/firehose/handlers.ts +++ b/src/firehose/handlers.ts @@ -68,8 +68,8 @@ interface MemberRequestRecord { createdAt: string; } -interface BookmarkRecord { - wikiRef: string; +interface CommunityBookmarkRecord { + subject: string; createdAt: string; } @@ -192,8 +192,8 @@ function isMemberRequestRecord(r: Rec): r is Rec & MemberRequestRecord { return hasString(r, "wikiRef") && hasString(r, "createdAt"); } -function isBookmarkRecord(r: Rec): r is Rec & BookmarkRecord { - return hasString(r, "wikiRef") && hasString(r, "createdAt"); +function isCommunityBookmarkRecord(r: Rec): r is Rec & CommunityBookmarkRecord { + return hasString(r, "subject") && hasString(r, "createdAt"); } // Built from a jetstream message in prod; tests invoke handleCommitEvent directly. @@ -238,8 +238,9 @@ export function handleCommitEvent(evt: FirehoseCommit): void { case COLLECTIONS.memberRequest: if (isMemberRequestRecord(r)) handleMemberRequest(evt.did, atUri, r); break; - case COLLECTIONS.bookmark: - if (isBookmarkRecord(r)) handleBookmark(evt.did, atUri, r); + case COLLECTIONS.communityBookmark: + if (isCommunityBookmarkRecord(r)) + handleCommunityBookmark(evt.did, atUri, r); break; } } @@ -363,15 +364,17 @@ function handleMemberRequest( upsertRequest(wiki.at_uri, wiki.slug, did, atUri, record.createdAt); } -function handleBookmark( +function handleCommunityBookmark( did: string, atUri: string, - record: BookmarkRecord, + record: CommunityBookmarkRecord, ): void { - const wiki = getWikiByAtUri(record.wikiRef); + // subject is a freeform URI (web URL or any at://). Only ingest ones that + // resolve to a wiki we know; everything else is silently skipped. + const wiki = getWikiByAtUri(record.subject); if (!wiki) return; - upsertBookmark(did, record.wikiRef, atUri, record.createdAt); + upsertBookmark(did, record.subject, atUri, record.createdAt); } function handleDelete(atUri: string, collection: string): void { @@ -391,7 +394,7 @@ function handleDelete(atUri: string, collection: string): void { case COLLECTIONS.memberRequest: deleteRequestByUri(atUri); break; - case COLLECTIONS.bookmark: + case COLLECTIONS.communityBookmark: deleteBookmarkByUri(atUri); break; } diff --git a/src/lib/collections.ts b/src/lib/collections.ts index 9df2d83..66f40e4 100644 --- a/src/lib/collections.ts +++ b/src/lib/collections.ts @@ -4,10 +4,15 @@ export const COLLECTIONS = { noteRevision: "wiki.lichen.noteRevision", membership: "wiki.lichen.membership", memberRequest: "wiki.lichen.memberRequest", + // Legacy: no longer minted or ingested from the firehose. Already-materialized + // bookmark rows are kept; the constant remains only for backfill/exists refs. bookmark: "wiki.lichen.bookmark", + // External NSID we don't own or publish (lexicon-community). All new bookmarks + // are minted and ingested here. + communityBookmark: "community.lexicon.bookmarks.bookmark", } as const; -export const OAUTH_SCOPE = `atproto include:wiki.lichen.permissions blob:*/*`; +export const OAUTH_SCOPE = `atproto include:wiki.lichen.permissions include:community.lexicon.bookmarks.authManageBookmarks blob:*/*`; export type MemberRole = "admin" | "contributor" | "viewer"; diff --git a/src/lib/orchestrators/bookmark.ts b/src/lib/orchestrators/bookmark.ts index a462fa9..41b3503 100644 --- a/src/lib/orchestrators/bookmark.ts +++ b/src/lib/orchestrators/bookmark.ts @@ -1,5 +1,8 @@ import * as TID from "@atcute/tid"; -import { deleteRecord, writeBookmarkRecord } from "../../atproto/pds.ts"; +import { + deleteRecord, + writeCommunityBookmarkRecord, +} from "../../atproto/pds.ts"; import { getAgent, type Session } from "../../atproto/session.ts"; import { deleteBookmarkByWiki, @@ -20,12 +23,18 @@ export async function addBookmarkAction( const now = new Date().toISOString(); const tid = TID.now(); - const atUri = `at://${did}/${COLLECTIONS.bookmark}/${tid}`; + const atUri = `at://${did}/${COLLECTIONS.communityBookmark}/${tid}`; const agent = session ? getAgent(session) : null; if (agent && session) { await withPdsError("add bookmark", async () => { - await writeBookmarkRecord(agent, session.did, tid, wikiAtUri, now); + await writeCommunityBookmarkRecord( + agent, + session.did, + tid, + wikiAtUri, + now, + ); }); } @@ -43,14 +52,10 @@ export async function deleteBookmarkAction( const agent = session ? getAgent(session) : null; if (agent) { const parsed = parseAtUri(bookmarkAtUri); - if (parsed) { + // Legacy wiki.lichen.bookmark records are out of scope and abandoned — DB-only. + if (parsed && parsed.collection !== COLLECTIONS.bookmark) { await withPdsError("remove bookmark", async () => { - await deleteRecord( - agent, - parsed.did, - COLLECTIONS.bookmark, - parsed.rkey, - ); + await deleteRecord(agent, parsed.did, parsed.collection, parsed.rkey); }); } } diff --git a/tests/atproto/pds.test.ts b/tests/atproto/pds.test.ts index 4d75d0f..ee2dd6f 100644 --- a/tests/atproto/pds.test.ts +++ b/tests/atproto/pds.test.ts @@ -37,7 +37,7 @@ const { writeRevisionRecord, writeMembershipRecord, writeMemberRequestRecord, - writeBookmarkRecord, + writeCommunityBookmarkRecord, deleteRecord, } = await import("../../src/atproto/pds.ts"); @@ -225,10 +225,10 @@ describe("PDS write functions — contract tests", () => { ); }); - test("writeBookmarkRecord passes correct fields", async () => { + test("writeCommunityBookmarkRecord passes correct fields", async () => { const { client, calls } = createMockClient(); - await writeBookmarkRecord( + await writeCommunityBookmarkRecord( client, DID, "tid-b1", @@ -237,10 +237,10 @@ describe("PDS write functions — contract tests", () => { ); const input: Record = calls[0]?.input ?? {}; - expect(input["collection"]).toBe(COLLECTIONS.bookmark); + expect(input["collection"]).toBe(COLLECTIONS.communityBookmark); const record = input["record"] as Record; - expect(record["wikiRef"]).toBe( + expect(record["subject"]).toBe( "at://did:plc:test/wiki.lichen.wiki/my-wiki", ); }); diff --git a/tests/firehose/handlers.test.ts b/tests/firehose/handlers.test.ts index 982d10a..8d77e9e 100644 --- a/tests/firehose/handlers.test.ts +++ b/tests/firehose/handlers.test.ts @@ -9,6 +9,7 @@ import { getNoteBySlug, getWiki, isBookmarked, + upsertBookmark, } from "../../src/server/db/queries/index.ts"; const db = getDb(); @@ -48,6 +49,7 @@ const HANDLER_TEST_WIKIS = [ "theme-enforced", "theme-unknown", "theme-cleared", + "cbk-wiki", ]; function cleanupHandlerTestData() { @@ -680,88 +682,109 @@ describe("member request handler", () => { }); }); -describe("bookmark handler", () => { - test("creates bookmark from firehose event", () => { - // Ensure wiki exists first +describe("legacy bookmark events (ignored)", () => { + test("does not materialize a legacy wiki.lichen.bookmark create", () => { handleCommitEvent( makeCommitEvt({ event: "create", - collection: "wiki.lichen.wiki", - rkey: "test-wiki", - did: ALICE_DID, + collection: "wiki.lichen.bookmark", + rkey: "legacy-bk", + did: BOB_DID, record: { - name: "BK Test Wiki", - visibility: "public", - createdAt: "2026-01-01T00:00:00.000Z", + wikiRef: WIKI_AT_URI, + createdAt: "2026-01-02T00:00:00.000Z", }, }), ); + expect(isBookmarked(BOB_DID, WIKI_AT_URI)).toBe(false); + }); + + test("legacy delete event leaves an already-materialized row intact", () => { + const legacyUri = `at://${BOB_DID}/wiki.lichen.bookmark/kept`; + upsertBookmark(BOB_DID, WIKI_AT_URI, legacyUri, "2026-01-02T00:00:00.000Z"); + expect(isBookmarked(BOB_DID, WIKI_AT_URI)).toBe(true); + handleCommitEvent( makeCommitEvt({ - event: "create", + event: "delete", collection: "wiki.lichen.bookmark", - rkey: "bk1", + rkey: "kept", did: BOB_DID, - record: { - wikiRef: WIKI_AT_URI, - createdAt: "2026-01-02T00:00:00.000Z", - }, }), ); expect(isBookmarked(BOB_DID, WIKI_AT_URI)).toBe(true); }); +}); - test("skips bookmark with missing wikiRef", () => { - const bkBadUri = `at://${BOB_DID}/wiki.lichen.bookmark/bk-bad`; +describe("community bookmark handler", () => { + const cWikiUri = `at://${ALICE_DID}/wiki.lichen.wiki/cbk-wiki`; + + test("creates bookmark from community.lexicon commit", () => { handleCommitEvent( makeCommitEvt({ event: "create", - collection: "wiki.lichen.bookmark", - rkey: "bk-bad", + collection: "wiki.lichen.wiki", + rkey: "cbk-wiki", + did: ALICE_DID, + record: { + name: "Community BK Wiki", + visibility: "public", + createdAt: "2026-01-01T00:00:00.000Z", + }, + }), + ); + + handleCommitEvent( + makeCommitEvt({ + event: "create", + collection: "community.lexicon.bookmarks.bookmark", + rkey: "cbk1", did: BOB_DID, record: { + subject: cWikiUri, createdAt: "2026-01-02T00:00:00.000Z", }, }), ); - const row = db - .query("SELECT * FROM bookmarks WHERE at_uri = ?") - .get(bkBadUri); - expect(row).toBeNull(); + + expect(isBookmarked(BOB_DID, cWikiUri)).toBe(true); }); - test("skips bookmark for non-existent wiki", () => { - const fakeWikiUri = "at://did:plc:ghost/wiki.lichen.wiki/nope"; + test("skips community bookmark whose subject is not a wiki", () => { + const bkWebUri = `at://${BOB_DID}/community.lexicon.bookmarks.bookmark/cbk-web`; handleCommitEvent( makeCommitEvt({ event: "create", - collection: "wiki.lichen.bookmark", - rkey: "bk-ghost", + collection: "community.lexicon.bookmarks.bookmark", + rkey: "cbk-web", did: BOB_DID, record: { - wikiRef: fakeWikiUri, + subject: "https://example.com/some-article", createdAt: "2026-01-02T00:00:00.000Z", }, }), ); - expect(isBookmarked(BOB_DID, fakeWikiUri)).toBe(false); + const row = db + .query("SELECT * FROM bookmarks WHERE at_uri = ?") + .get(bkWebUri); + expect(row).toBeNull(); }); - test("deletes bookmark on delete event", () => { - expect(isBookmarked(BOB_DID, WIKI_AT_URI)).toBe(true); + test("deletes community bookmark on delete event", () => { + expect(isBookmarked(BOB_DID, cWikiUri)).toBe(true); handleCommitEvent( makeCommitEvt({ event: "delete", - collection: "wiki.lichen.bookmark", - rkey: "bk1", + collection: "community.lexicon.bookmarks.bookmark", + rkey: "cbk1", did: BOB_DID, }), ); - expect(isBookmarked(BOB_DID, WIKI_AT_URI)).toBe(false); + expect(isBookmarked(BOB_DID, cWikiUri)).toBe(false); }); }); diff --git a/tests/lib/orchestrators/bookmark.test.ts b/tests/lib/orchestrators/bookmark.test.ts index 751001d..78b0db0 100644 --- a/tests/lib/orchestrators/bookmark.test.ts +++ b/tests/lib/orchestrators/bookmark.test.ts @@ -4,8 +4,8 @@ import { getDb } from "../../../src/server/db/index.ts"; const realPds = await import("../../../src/atproto/pds.ts"); const realSession = await import("../../../src/atproto/session.ts"); -const mockWriteBookmarkRecord = mock(async () => ({ - uri: "at://did:plc:bkuser/wiki.lichen.bookmark/abc", +const mockWriteCommunityBookmarkRecord = mock(async () => ({ + uri: "at://did:plc:bkuser/community.lexicon.bookmarks.bookmark/abc", cid: "bafyrei123", })); const mockDeleteRecord = mock(async () => {}); @@ -13,7 +13,7 @@ const mockGetAgent = mock(() => ({}) as never); mock.module("../../../src/atproto/pds.ts", () => ({ ...realPds, - writeBookmarkRecord: mockWriteBookmarkRecord, + writeCommunityBookmarkRecord: mockWriteCommunityBookmarkRecord, deleteRecord: mockDeleteRecord, })); mock.module("../../../src/atproto/session.ts", () => ({ @@ -25,7 +25,7 @@ const { addBookmarkAction, deleteBookmarkAction } = await import( "../../../src/lib/orchestrators/bookmark.ts" ); const { PdsWriteError } = await import("../../../src/lib/errors.ts"); -const { isBookmarked, upsertBookmark } = await import( +const { getBookmarkAtUri, isBookmarked, upsertBookmark } = await import( "../../../src/server/db/queries/index.ts" ); @@ -43,28 +43,38 @@ afterAll(() => { describe("addBookmarkAction", () => { test("writes to PDS and DB with session", async () => { - mockWriteBookmarkRecord.mockClear(); + mockWriteCommunityBookmarkRecord.mockClear(); await addBookmarkAction(USER_DID, WIKI_AT_URI, session); - expect(mockWriteBookmarkRecord).toHaveBeenCalledTimes(1); + expect(mockWriteCommunityBookmarkRecord).toHaveBeenCalledTimes(1); expect(isBookmarked(USER_DID, WIKI_AT_URI)).toBe(true); }); + test("mints a community.lexicon record, not wiki.lichen.bookmark", async () => { + const db = getDb(); + db.run("DELETE FROM bookmarks WHERE did = ?", [USER_DID]); + + await addBookmarkAction(USER_DID, WIKI_AT_URI, session); + + const atUri = getBookmarkAtUri(USER_DID, WIKI_AT_URI); + expect(atUri).toContain("/community.lexicon.bookmarks.bookmark/"); + }); + test("writes to DB only without session (dev mode)", async () => { const db = getDb(); db.run("DELETE FROM bookmarks WHERE did = ?", [USER_DID]); - mockWriteBookmarkRecord.mockClear(); + mockWriteCommunityBookmarkRecord.mockClear(); await addBookmarkAction(USER_DID, WIKI_AT_URI, null); - expect(mockWriteBookmarkRecord).not.toHaveBeenCalled(); + expect(mockWriteCommunityBookmarkRecord).not.toHaveBeenCalled(); expect(isBookmarked(USER_DID, WIKI_AT_URI)).toBe(true); }); test("throws PdsWriteError and does not write to DB on PDS failure", async () => { const db = getDb(); db.run("DELETE FROM bookmarks WHERE did = ?", [USER_DID]); - mockWriteBookmarkRecord.mockImplementationOnce(async () => { + mockWriteCommunityBookmarkRecord.mockImplementationOnce(async () => { throw new Error("PDS down"); }); @@ -77,12 +87,12 @@ describe("addBookmarkAction", () => { test("re-adding an already-bookmarked wiki writes no second PDS record", async () => { const db = getDb(); db.run("DELETE FROM bookmarks WHERE did = ?", [USER_DID]); - mockWriteBookmarkRecord.mockClear(); + mockWriteCommunityBookmarkRecord.mockClear(); await addBookmarkAction(USER_DID, WIKI_AT_URI, session); await addBookmarkAction(USER_DID, WIKI_AT_URI, session); - expect(mockWriteBookmarkRecord).toHaveBeenCalledTimes(1); + expect(mockWriteCommunityBookmarkRecord).toHaveBeenCalledTimes(1); expect(isBookmarked(USER_DID, WIKI_AT_URI)).toBe(true); }); }); @@ -92,7 +102,7 @@ describe("deleteBookmarkAction", () => { upsertBookmark( USER_DID, WIKI_AT_URI, - `at://${USER_DID}/wiki.lichen.bookmark/rm1`, + `at://${USER_DID}/community.lexicon.bookmarks.bookmark/rm1`, "2026-01-01T00:00:00.000Z", ); mockDeleteRecord.mockClear(); @@ -103,6 +113,21 @@ describe("deleteBookmarkAction", () => { expect(isBookmarked(USER_DID, WIKI_AT_URI)).toBe(false); }); + test("removes a legacy wiki.lichen.bookmark row without a PDS delete", async () => { + upsertBookmark( + USER_DID, + WIKI_AT_URI, + `at://${USER_DID}/wiki.lichen.bookmark/legacy`, + "2026-01-01T00:00:00.000Z", + ); + mockDeleteRecord.mockClear(); + + await deleteBookmarkAction(USER_DID, WIKI_AT_URI, session); + + expect(mockDeleteRecord).not.toHaveBeenCalled(); + expect(isBookmarked(USER_DID, WIKI_AT_URI)).toBe(false); + }); + test("deletes from DB only without session (dev mode)", async () => { upsertBookmark( USER_DID, @@ -132,7 +157,7 @@ describe("deleteBookmarkAction", () => { upsertBookmark( USER_DID, WIKI_AT_URI, - `at://${USER_DID}/wiki.lichen.bookmark/pdsfail`, + `at://${USER_DID}/community.lexicon.bookmarks.bookmark/pdsfail`, "2026-01-01T00:00:00.000Z", ); mockDeleteRecord.mockImplementationOnce(async () => { diff --git a/tests/server/routes/helpers.ts b/tests/server/routes/helpers.ts index c14fec5..9f2964a 100644 --- a/tests/server/routes/helpers.ts +++ b/tests/server/routes/helpers.ts @@ -44,7 +44,7 @@ mock.module("../../../src/atproto/pds.ts", () => ({ writeRevisionRecord: async () => mockPdsResult, writeMembershipRecord: async () => mockPdsResult, writeMemberRequestRecord: async () => mockPdsResult, - writeBookmarkRecord: async () => mockPdsResult, + writeCommunityBookmarkRecord: async () => mockPdsResult, deleteRecord: async () => {}, })); -- 2.51.2