fix(pds): include prevData in firehose #commit events#170
Closed
simnaut wants to merge 2 commits into
Closed
Conversation
prevData is the MST root CID of the previous commit (the data field at the since rev) and is effectively required for the inductive firehose. Without it, relays running strict commit validation (indigo with LenientSyncValidation off) fail with "missing prevData field" and reject every commit after the first, freezing the repo. Populated from the pre-write repo's commit.data at each write path (create/delete/put/applyWrites). Closes ascorbic#169 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds prevData to firehose #commit events so relays can verify commits inductively (avoiding “missing prevData field” failures).
Changes:
- Add
prevData(previous commit MST root CID) to sequencer commit event types and emitted events. - Populate
prevDatafrom the pre-write repo state across account write paths. - Add a regression test and update the firehose verification script/types.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/pds/test/firehose.test.ts | Adds a test asserting prevData matches the previous commit’s MST root. |
| packages/pds/src/sequencer.ts | Extends commit event/data types and forwards prevData into emitted events. |
| packages/pds/src/account-do.ts | Populates prevData on commit data objects built during repo writes. |
| packages/pds/scripts/verify-firehose.ts | Updates verification script’s commit event shape to include prevData. |
| .changeset/firehose-prevdata.md | Documents the patch change and rationale for relays/verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced May 23, 2026
Address review feedback: type prevData as CID | null in the sequencer (consistent with the nullable since field) and optional in the verify-firehose script (tolerates legacy pre-prevData / since=null streams). No behavior change — every write path still has a prior commit and sets it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Owner
|
Aah! I missed that you'd jumped on this. I just merged #171 that does the same thing and a few more compliance things that I found. I'll close this but thanks for your help. I'll merge the other one |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #169
Problem
Cirrus's firehose
#commitevents omit theprevDatafield — the MST root CID of the previous commit (thedatafield at thesincerev), which is "effectively required for the 'inductive' version of firehose" per the lexicon. Relays running strict commit validation (indigo withLenientSyncValidationoff) fail verification with"missing prevData field"and reject every commit after the first, freezing the repo on that relay.Demonstrated on two independent repos: a strict relay (relay.feeds.blue) and a lenient relay (bsky.network) read the firehose to the same cursor, yet the strict relay is frozen ~106 days while the lenient one is at head — the divergence is purely strict-vs-lenient validation of the missing field. The relay.feeds.blue operator confirmed they run standard indigo with
LenientSyncValidationoff.Change
The previous commit's MST root is already in scope at every write path in
account-do.ts(therepoobject captured beforeapplyWrites, alongside the existingprevRev). PopulateprevData = repo.commit.data:sequencer.ts: addprevData: CIDtoCommitDataandCommitEvent; set it insequenceCommit's payload (flows through the existing CBOR encoder).account-do.ts: setprevData: repo.commit.datain theCommitDataatrpcCreateRecord,rpcDeleteRecord,rpcPutRecord,rpcApplyWrites.test/firehose.test.ts: new test assertingprevDatais present and equals the prior commit'sdataCID.scripts/verify-firehose.ts: addprevDatato itsCommitEventinterface.Verification
pnpm --filter @getcirrus/pds build✅pnpm --filter @getcirrus/pds test:unit✅ (280 passed, incl. new prevData test)pnpm knip✅Related: #167 / #168 (
getLatestCommit) is a separate conformance gap, not the indexing fix.🤖 Generated with Claude Code