diff --git a/src/firehose/handlers.ts b/src/firehose/handlers.ts index 5e2f64c..bf45c5e 100644 --- a/src/firehose/handlers.ts +++ b/src/firehose/handlers.ts @@ -199,51 +199,63 @@ function sanitizeMembership( type Rec = Record; -function hasString(r: Rec, key: string): boolean { +// For identifiers, references and timestamps, where "" is as malformed as absent. +function hasNonEmptyString(r: Rec, key: string): boolean { return typeof r[key] === "string" && r[key] !== ""; } +// Presence only. Separate from hasNonEmptyString because for a payload field "" +// is a value, not a missing field, and the two rules must be chosen per field +// rather than inherited from whichever helper was nearest. +function hasString(r: Rec, key: string): boolean { + return typeof r[key] === "string"; +} + function isWikiRecord(r: Rec): r is Rec & WikiRecord { return ( - hasString(r, "name") && + hasNonEmptyString(r, "name") && (r["visibility"] === "public" || r["visibility"] === "private") && - hasString(r, "createdAt") + hasNonEmptyString(r, "createdAt") ); } function isNoteRecord(r: Rec): r is Rec & NoteRecord { return ( - hasString(r, "slug") && - hasString(r, "title") && - hasString(r, "wikiRef") && - hasString(r, "createdAt") + hasNonEmptyString(r, "slug") && + hasNonEmptyString(r, "title") && + hasNonEmptyString(r, "wikiRef") && + hasNonEmptyString(r, "createdAt") ); } function isRevisionRecord(r: Rec): r is Rec & RevisionRecord { return ( - hasString(r, "noteRef") && + hasNonEmptyString(r, "noteRef") && + // A note created with no content diffs "" -> "", so an empty diff is a real + // first revision. The lexicon agrees: `diff` has no minLength. Requiring + // non-empty here dropped those revisions and left the note 404ing with no + // content, which is how this was found. hasString(r, "diff") && - hasString(r, "diffFormat") && - hasString(r, "createdAt") + hasNonEmptyString(r, "diffFormat") && + hasNonEmptyString(r, "createdAt") ); } function isMembershipRecord(r: Rec): r is Rec & MembershipRecord { return ( - hasString(r, "memberDid") && - hasString(r, "wikiRef") && - hasString(r, "role") && - hasString(r, "createdAt") + hasNonEmptyString(r, "memberDid") && + hasNonEmptyString(r, "wikiRef") && + hasNonEmptyString(r, "role") && + hasNonEmptyString(r, "createdAt") ); } function isMemberRequestRecord(r: Rec): r is Rec & MemberRequestRecord { - return hasString(r, "wikiRef") && hasString(r, "createdAt"); + return hasNonEmptyString(r, "wikiRef") && hasNonEmptyString(r, "createdAt"); } function isCommunityBookmarkRecord(r: Rec): r is Rec & CommunityBookmarkRecord { - return hasString(r, "subject") && hasString(r, "createdAt"); + return hasNonEmptyString(r, "subject") && hasNonEmptyString(r, "createdAt"); } // Built from a jetstream message in prod; tests invoke handleCommitEvent directly. @@ -266,8 +278,8 @@ export function handleCommitEvent(evt: FirehoseCommit): void { // src/firehose/index.ts), and the backfill path has always had to do them itself. // Both feed sinks that assume the atproto charset: `did` reaches authorization // and the member directory, `rkey` becomes a wiki slug in an href. Doing them - // here rather than per-path is what makes this function the chokepoint that - // INGESTION-AUDIT.md §2 asks for. + // here rather than per-path is what keeps this function the single chokepoint + // every remote record passes through. if (!isDid(evt.did)) { logDrop(atUri, "invalid did"); return; diff --git a/tests/firehose/handlers.test.ts b/tests/firehose/handlers.test.ts index eaebc4e..2ddc2ac 100644 --- a/tests/firehose/handlers.test.ts +++ b/tests/firehose/handlers.test.ts @@ -573,6 +573,62 @@ describe("revision handler", () => { }); }); +// A note created with no content has a first revision whose diff is "" — the +// lexicon puts no minLength on `diff`. Rejecting those left the note in the DB +// with no content row, which the note route serves as a 404 forever. +describe("empty first revision", () => { + const EMPTY_NOTE_TID = "362pbqd3teee1"; + const EMPTY_NOTE_AT_URI = `at://${ALICE_DID}/wiki.lichen.note/${EMPTY_NOTE_TID}`; + + beforeAll(() => { + handleCommitEvent( + makeCommitEvt({ + event: "create", + collection: "wiki.lichen.note", + rkey: EMPTY_NOTE_TID, + record: { + slug: "empty-note", + title: "Empty Note", + wikiRef: WIKI_AT_URI, + createdAt: "2026-01-01T00:00:00.000Z", + }, + }), + ); + }); + + function emptyRevision(rkey: string, overrides: Record) { + return makeCommitEvt({ + event: "create", + collection: "wiki.lichen.noteRevision", + rkey, + record: { + noteRef: EMPTY_NOTE_AT_URI, + diffFormat: "diff-match-patch", + createdAt: "2026-01-01T00:00:00.000Z", + ...overrides, + }, + }); + } + + test("ingests a revision whose diff is empty", () => { + handleCommitEvent(emptyRevision("362pbqd3teee2", { diff: "" })); + + const current = getCurrentNote(WIKI_AT_URI, "empty-note"); + expect(current).not.toBeNull(); + expect(current?.content).toBe(""); + }); + + // Presence, not truthiness: an absent diff is still a malformed record. + test("still drops a revision with no diff field at all", () => { + handleCommitEvent(emptyRevision("362pbqd3teee3", {})); + + const rev = db + .query("SELECT 1 FROM revisions WHERE at_uri = ?") + .get(`at://${ALICE_DID}/wiki.lichen.noteRevision/362pbqd3teee3`); + expect(rev).toBeNull(); + }); +}); + describe("membership handler", () => { test("creates membership when event DID is wiki owner", () => { const memberTid = "362pbqd3tgf1a"; diff --git a/tests/firehose/hostile-records.test.ts b/tests/firehose/hostile-records.test.ts index 4d20f20..76a2eab 100644 --- a/tests/firehose/hostile-records.test.ts +++ b/tests/firehose/hostile-records.test.ts @@ -65,8 +65,8 @@ afterAll(() => { // `did` and `rkey` used to be validated by @atcute/jetstream's schema. We turned // validateEvents off to stop it silently dropping identity events, which makes -// these checks ours — and the rkey one is the door INGESTION-AUDIT.md §4 left open -// on purpose while the library was still holding it shut. +// these checks ours — and the rkey one was deliberately left to the library back +// when the library was still holding it shut. describe("commit envelope shape", () => { test("drops an rkey that breaks out of an attribute", () => { const rkey = `x" onmouseover="alert(1)`; @@ -339,3 +339,30 @@ describe("coercion instead of rejection", () => { expect(getCurrentNote(WIKI_URI, "fmt-note")).toBeNull(); }); }); + +// `diff` is presence-checked so an empty first revision survives. The reference +// fields are not, and this is the test that holds that line. +// +// It has to assert the *reason*: an empty noteRef also fails to resolve to a note, +// so the record is dropped either way and "no row" proves nothing about which rule +// caught it. +describe("the empty-diff relaxation does not reach reference fields", () => { + test("rejects an empty noteRef at the type guard", () => { + const lines = captureWarn(() => { + handleCommitEvent({ + did: OWNER, + collection: COLLECTIONS.noteRevision, + rkey: "3emptynoteref", + operation: "create", + record: { + noteRef: "", + diff: "", + diffFormat: "diff-match-patch", + createdAt: ISO, + }, + }); + }); + + expect(lines.join("\n")).toContain("missing required fields"); + }); +});