Grandiras/ATProto.NET

Fork 0

feat!: typed identifiers for Spaces, the space server and streaming (4/4 of #119) #157

Merged

Grandiras merged 1 commit from issue-119-typed-spaces-streaming into main

2026-09-24 23:39:10 +00:00

Grandiras commented

2026-09-24 23:35:48 +00:00

Owner

Closes #119 (4/4 — the last part: Spaces, server and streaming).

The temporary PendingConversions allow-list in TypedIdentifierGuardTests is now empty and has been removed along with its test, so the guard enforces typed identifiers across the whole public surface with only the documented per-member exceptions.

Stacked on #154 (part 1), which sets the conventions this PR follows (CONTRIBUTING.md → Lexicon models and clients).

How to review this

Most of the 96 files are mechanical: a string becomes Did/Nsid/RecordKey/Tid/Cid/SpaceUri, and the call sites and tests follow. The parts worth reading closely:

src/ATProtoNet/Spaces/SpaceUri.cs, SpaceCommit.cs, SpaceRepoCar.cs — the typed space components, SpaceCommitContext(SpaceUri, Did, Tid), and the two places where typing adds validation (SpaceRepoCar.Verify refuses an index path that is not NSID/rkey or a link that is not a CID). The commit-context encoding still hashes the exact text, so the reference vector test is unchanged.
src/ATProtoNet/Lexicon/Com/AtProto/Sync/SyncModels.cs — the firehose event models and the new RepoOpAction enum.
src/ATProtoNet/Streaming/JetstreamEventParser.cs / JetstreamSegmentReader.cs — what the forward-tolerant parsers do with a value that does not parse (skip the commit for collection/rkey, null out rev/handle).
src/ATProtoNet.Server/Spaces/SpaceRequestValidation.cs and the endpoint files — query parameters and request bodies now bind through the identifier types, so the per-format string validators collapse to a presence check.
tests/ATProtoNet.Tests/Conventions/TypedIdentifierGuardTests.cs — my allow-list lines are gone; the new Exceptions entries are the members that genuinely carry something other than one format.
CHANGELOG.md.

Everything else — tests, samples, docs, the EF stores converting at the boundary — follows from those.

What changed

Spaces (`com.atproto.space.`, `com.atproto.simplespace.`, `ATProtoNet.Spaces`)

Client methods and models take and return SpaceUri (Lexicon space-ref), SpaceRecordUri (the at-uri of a space record), Did, Nsid, RecordKey, Tid, Cid; RegisterNotifyResponse.ExpiresAt is an AtDatetime; collections are IReadOnlyList<T>.
SpaceUri.Authority/SpaceType/Skey and SpaceRecordUri.Author/Collection/Rkey are typed; SpaceUri.Create / Record take typed components (so they can no longer fail).
SpaceCommitContext, SignedSpaceCommit.Rev, SpaceRepoOp, SpaceRepoRecord, SpaceRepoCommit, VerifiedSpaceRepo.Index, SpaceRepoCar.Verify, SpaceSyncer (resolver takes a Did), SpaceSyncResult, SpaceRepoCursor, ISpaceRepoStore, SpaceCredentialProvider (+ HostResolver), SpaceTypeDeclaration.Collections, SpaceAuthority.HostAudience/ParseServiceIdentifier, SpaceTokens.Verify(expectedSubject).
Convention items: every cursored response implements ICursorPage<T>; ListRecordsAsync and ListRepoOpsAsync take filters before limit/cursor; the string/SpaceUri overload pairs and the ToSpaceUri()/ToRecordUri() helpers are gone; client-only request bodies (CreateSpaceRecordRequest, PutSpaceRecordRequest, DeleteSpaceRecordRequest, ApplySpaceWritesRequest, NotifySpaceDeletedRequest) are internal. The other request bodies stay public because the public server endpoints accept them (#133 makes those internal).
Enumerators run on the shared paginator: EnumerateReposAsync, EnumerateRecordsAsync (+ reverse), EnumerateMembersAsync, and new EnumerateSpacesAsync / EnumerateBlobsAsync. New GetRecordAsync(SpaceRecordUri) / DeleteRecordAsync(SpaceRecordUri).

Space server (`ATProtoNet.Server.Spaces`)

ISpaceRepoHost, ISpaceAuthorityStore, ISimpleSpaceStore, SimpleSpaceRecord.Owner, SpaceAccessRequest, Verified* records, ISpaceCallerResolver, ISpaceDidDocumentResolver (+ CachingSpaceDidDocumentResolver.Invalidate), ISpaceServiceAuthVerifier (Did issuer, Nsid method), ISimpleSpaceManagingAppClient, SpaceWriteNotifier, the query-parameter classes and SpaceServerOptions.ServiceDid.
The binder validates identifier parameters (InvalidRequest naming the parameter), so since on listRepoOps/listBlobs and cid on getBlob, which were passed through unchecked, are now validated too.
EF Core: entities are unchanged; the stores convert between the typed values and the existing string columns, so consumers need no migration.

Streaming

subscribeRepos models: CommitEvent.Repo Did, Commit Cid, Rev/Since Tid/Tid?, PrevData Cid?, Blobs IReadOnlyList<Cid>?, Ops IReadOnlyList<RepoOp>?; RepoOp.Cid/Prev Cid? and Action a new RepoOpAction enum with the same members as JetstreamOperation; the account/identity/sync/handle/tombstone events carry Did/Handle/Tid; FirehoseMessage.Time AtDatetime?.
Jetstream: JetstreamCommitEvent.Collection Nsid, RKey → Rkey (RecordKey), Rev Tid?; JetstreamIdentityEvent.Handle Handle?; the events' Time AtDatetime?; WantedDids / JetstreamSnapshotRequest.Dids IReadOnlyList<Did>; JetstreamArchiveRow.RKey → Rkey; ListAllSegmentsAsync → EnumerateSegmentsAsync on the shared paginator; JetstreamSegmentPage : ICursorPage<T>.

`Auth/ServiceAuthGenerator`

Did service DID, Nsid? lxm; the audience stays a string (a DID with an optional #service fragment) and is now validated as one.

Guard test

Removed the spaces, streaming, firehose-event and ServiceAuthGenerator lines from PendingConversions. The mechanism stays: parts 2 and 3 still own their lines.
New documented Exceptions: SpaceToken.Issuer/Subject and SpaceTokens.Create(issuer/subject) (a DID or an OAuth client ID), ISpaceReplayStore.TryConsumeAsync(issuer) (also a DPoP thumbprint), the DPoP htu URL members, WantedCollections / JetstreamSnapshotRequest.Collections (accept app.bsky.graph.* wildcards), and JetstreamArchiveRow.Did/Collection/Rkey/Rev (raw archive columns; ToEvent() types them).

Breaking changes and migration

Spaces, server: pass the typed values the API returns (created.Uri, writer.Did); parse literals once (Nsid.Parse("com.example.bookmark"), Did.Parse(…), options.ServiceDid = Did.Parse(…)); name the reordered ListRecordsAsync/ListRepoOpsAsync arguments; read .Uri instead of ToSpaceUri()/ToRecordUri(); implement ISpaceRepoStore/ISpaceRepoHost/stores with the typed signatures.
Streaming: op.Action == RepoOpAction.Create instead of "create"; RKey → Rkey; ListAllSegmentsAsync → EnumerateSegmentsAsync; WantedDids = [Did.Parse(…)].
Service auth: new ServiceAuthGenerator(Did.Parse(…), key), CreateToken(aud, Nsid.Parse(…)).
Behaviour: a firehose frame with an unparseable identifier is dropped (the parser returns null, as for any malformed frame); a Jetstream commit with an unparseable collection/rkey is skipped; SpaceTypeDeclaration.FromLexicon throws JsonException for a non-NSID collection (e.g. *).

Fixed

The spaces and Jetstream archive enumerators stopped only on an empty page, so a host repeating its cursor with non-empty pages was asked for the same page forever. Regression tests: SpaceClientTests.Enumerators_HostRepeatingItsCursor_StopAfterTheRepeat (all five space enumerators) and JetstreamArchiveClientTests.EnumerateSegmentsAsync_ServerRepeatingItsCursor_StopsAfterTheRepeat; each fails against the old loop (it keeps requesting) and passes with exactly two requests.

Verification

dotnet build --no-incremental of the whole solution: 0 warnings, 0 errors (samples and integration tests included).
Unit tests: 3180 passed, 0 failed, 5 skipped (the 5 skips are pre-existing environment-gated tests).
TypedIdentifierGuardTests: no violations in my namespaces outside the documented exceptions; Exceptions_AllStillMatchAMember and PendingConversions_AllStillHaveViolations pass.
New tests: firehose frames with a malformed repo/commit/rev; Jetstream commits with a malformed collection/rkey/rev and an identity with a malformed handle; SpaceRepoCar.Verify with an index path that is not NSID/rkey; SpaceTypeDeclaration with a wildcard collection; delegation and service-auth tokens whose iss is not a DID; ServiceAuthGenerator audience validation; the enumerator loops above. The SpaceCommitContext reference vector (from @atproto/space) still passes unchanged, and the unambiguous-encoding test now uses two contexts that concatenate to the same text through a colon-bearing space key and DID.
Test data that used placeholder identifiers (bafyreiabc, 3kaa, rev123, …) now uses well-formed ones, since the models validate them.
Integration tests compile against the new API; they are gated on a spaces PDS and were not run.

Deliberately not done

Spaces hardening (#133): open unions and extension data for the Space models, making endpoint and query-parameter classes internal, DID-document strictness, caches. The typed index/path parsing in SpaceRepoCar.Verify and the NSID-only SpaceTypeDeclaration.Collections overlap two #133 items (each index key exactly one validated NSID/rkey; collections must not contain *); #133 can tick them after review.
Streaming consolidation (#126): TypedFirehoseConsumerOptions.CollectionFilter stays IReadOnlySet<string> (#126 reworks the filter to peek ops[].path before deserializing); no common IRecordEvent abstraction; JetstreamOperation and RepoOpAction stay separate enums with identical members rather than one shared type, to keep this a type conversion.
No EnumerateRepoOpsAsync: an oplog page carries the head commit, which an item enumerator would drop; SpaceSyncer is the consumer of that endpoint.
The docs/firehose.md "With Cursor" example still reads message.Seq off a raw FirehoseFrame, which has no such member; pre-existing and outside the APIs this PR changes.

🤖 Generated with Claude Code

Closes #119 (4/4 — the last part: Spaces, server and streaming). The temporary `PendingConversions` allow-list in `TypedIdentifierGuardTests` is now empty and has been removed along with its test, so the guard enforces typed identifiers across the whole public surface with only the documented per-member exceptions. Stacked on #154 (part 1), which sets the conventions this PR follows (`CONTRIBUTING.md` → *Lexicon models and clients*). ## How to review this Most of the 96 files are mechanical: a `string` becomes `Did`/`Nsid`/`RecordKey`/`Tid`/`Cid`/`SpaceUri`, and the call sites and tests follow. The parts worth reading closely: 1. **`src/ATProtoNet/Spaces/SpaceUri.cs`, `SpaceCommit.cs`, `SpaceRepoCar.cs`** — the typed space components, `SpaceCommitContext(SpaceUri, Did, Tid)`, and the two places where typing adds validation (`SpaceRepoCar.Verify` refuses an index path that is not `NSID/rkey` or a link that is not a CID). The commit-context encoding still hashes the exact text, so the reference vector test is unchanged. 2. **`src/ATProtoNet/Lexicon/Com/AtProto/Sync/SyncModels.cs`** — the firehose event models and the new `RepoOpAction` enum. 3. **`src/ATProtoNet/Streaming/JetstreamEventParser.cs` / `JetstreamSegmentReader.cs`** — what the forward-tolerant parsers do with a value that does not parse (skip the commit for `collection`/`rkey`, null out `rev`/`handle`). 4. **`src/ATProtoNet.Server/Spaces/SpaceRequestValidation.cs`** and the endpoint files — query parameters and request bodies now bind through the identifier types, so the per-format string validators collapse to a presence check. 5. **`tests/ATProtoNet.Tests/Conventions/TypedIdentifierGuardTests.cs`** — my allow-list lines are gone; the new `Exceptions` entries are the members that genuinely carry something other than one format. 6. **`CHANGELOG.md`**. Everything else — tests, samples, docs, the EF stores converting at the boundary — follows from those. ## What changed ### Spaces (`com.atproto.space.*`, `com.atproto.simplespace.*`, `ATProtoNet.Spaces`) - Client methods and models take and return `SpaceUri` (Lexicon `space-ref`), `SpaceRecordUri` (the `at-uri` of a space record), `Did`, `Nsid`, `RecordKey`, `Tid`, `Cid`; `RegisterNotifyResponse.ExpiresAt` is an `AtDatetime`; collections are `IReadOnlyList<T>`. - `SpaceUri.Authority`/`SpaceType`/`Skey` and `SpaceRecordUri.Author`/`Collection`/`Rkey` are typed; `SpaceUri.Create` / `Record` take typed components (so they can no longer fail). - `SpaceCommitContext`, `SignedSpaceCommit.Rev`, `SpaceRepoOp`, `SpaceRepoRecord`, `SpaceRepoCommit`, `VerifiedSpaceRepo.Index`, `SpaceRepoCar.Verify`, `SpaceSyncer` (resolver takes a `Did`), `SpaceSyncResult`, `SpaceRepoCursor`, `ISpaceRepoStore`, `SpaceCredentialProvider` (+ `HostResolver`), `SpaceTypeDeclaration.Collections`, `SpaceAuthority.HostAudience`/`ParseServiceIdentifier`, `SpaceTokens.Verify(expectedSubject)`. - **Convention items:** every cursored response implements `ICursorPage<T>`; `ListRecordsAsync` and `ListRepoOpsAsync` take filters before `limit`/`cursor`; the `string`/`SpaceUri` overload pairs and the `ToSpaceUri()`/`ToRecordUri()` helpers are gone; client-only request bodies (`CreateSpaceRecordRequest`, `PutSpaceRecordRequest`, `DeleteSpaceRecordRequest`, `ApplySpaceWritesRequest`, `NotifySpaceDeletedRequest`) are internal. The other request bodies stay public because the public server endpoints accept them (#133 makes those internal). - **Enumerators** run on the shared paginator: `EnumerateReposAsync`, `EnumerateRecordsAsync` (+ `reverse`), `EnumerateMembersAsync`, and new `EnumerateSpacesAsync` / `EnumerateBlobsAsync`. New `GetRecordAsync(SpaceRecordUri)` / `DeleteRecordAsync(SpaceRecordUri)`. ### Space server (`ATProtoNet.Server.Spaces`) - `ISpaceRepoHost`, `ISpaceAuthorityStore`, `ISimpleSpaceStore`, `SimpleSpaceRecord.Owner`, `SpaceAccessRequest`, `Verified*` records, `ISpaceCallerResolver`, `ISpaceDidDocumentResolver` (+ `CachingSpaceDidDocumentResolver.Invalidate`), `ISpaceServiceAuthVerifier` (`Did` issuer, `Nsid` method), `ISimpleSpaceManagingAppClient`, `SpaceWriteNotifier`, the query-parameter classes and `SpaceServerOptions.ServiceDid`. - The binder validates identifier parameters (`InvalidRequest` naming the parameter), so `since` on `listRepoOps`/`listBlobs` and `cid` on `getBlob`, which were passed through unchecked, are now validated too. - **EF Core:** entities are unchanged; the stores convert between the typed values and the existing string columns, so consumers need no migration. ### Streaming - `subscribeRepos` models: `CommitEvent.Repo` `Did`, `Commit` `Cid`, `Rev`/`Since` `Tid`/`Tid?`, `PrevData` `Cid?`, `Blobs` `IReadOnlyList<Cid>?`, `Ops` `IReadOnlyList<RepoOp>?`; `RepoOp.Cid`/`Prev` `Cid?` and `Action` a new `RepoOpAction` enum with the same members as `JetstreamOperation`; the account/identity/sync/handle/tombstone events carry `Did`/`Handle`/`Tid`; `FirehoseMessage.Time` `AtDatetime?`. - Jetstream: `JetstreamCommitEvent.Collection` `Nsid`, `RKey` → `Rkey` (`RecordKey`), `Rev` `Tid?`; `JetstreamIdentityEvent.Handle` `Handle?`; the events' `Time` `AtDatetime?`; `WantedDids` / `JetstreamSnapshotRequest.Dids` `IReadOnlyList<Did>`; `JetstreamArchiveRow.RKey` → `Rkey`; `ListAllSegmentsAsync` → `EnumerateSegmentsAsync` on the shared paginator; `JetstreamSegmentPage : ICursorPage<T>`. ### `Auth/ServiceAuthGenerator` - `Did` service DID, `Nsid?` lxm; the audience stays a string (a DID with an optional `#service` fragment) and is now validated as one. ### Guard test - Removed the spaces, streaming, firehose-event and `ServiceAuthGenerator` lines from `PendingConversions`. The mechanism stays: parts 2 and 3 still own their lines. - New documented `Exceptions`: `SpaceToken.Issuer`/`Subject` and `SpaceTokens.Create(issuer/subject)` (a DID *or* an OAuth client ID), `ISpaceReplayStore.TryConsumeAsync(issuer)` (also a DPoP thumbprint), the DPoP `htu` URL members, `WantedCollections` / `JetstreamSnapshotRequest.Collections` (accept `app.bsky.graph.*` wildcards), and `JetstreamArchiveRow.Did`/`Collection`/`Rkey`/`Rev` (raw archive columns; `ToEvent()` types them). ## Breaking changes and migration - **Spaces, server:** pass the typed values the API returns (`created.Uri`, `writer.Did`); parse literals once (`Nsid.Parse("com.example.bookmark")`, `Did.Parse(…)`, `options.ServiceDid = Did.Parse(…)`); name the reordered `ListRecordsAsync`/`ListRepoOpsAsync` arguments; read `.Uri` instead of `ToSpaceUri()`/`ToRecordUri()`; implement `ISpaceRepoStore`/`ISpaceRepoHost`/stores with the typed signatures. - **Streaming:** `op.Action == RepoOpAction.Create` instead of `"create"`; `RKey` → `Rkey`; `ListAllSegmentsAsync` → `EnumerateSegmentsAsync`; `WantedDids = [Did.Parse(…)]`. - **Service auth:** `new ServiceAuthGenerator(Did.Parse(…), key)`, `CreateToken(aud, Nsid.Parse(…))`. - **Behaviour:** a firehose frame with an unparseable identifier is dropped (the parser returns `null`, as for any malformed frame); a Jetstream commit with an unparseable `collection`/`rkey` is skipped; `SpaceTypeDeclaration.FromLexicon` throws `JsonException` for a non-NSID collection (e.g. `*`). ## Fixed - The spaces and Jetstream archive enumerators stopped only on an empty page, so a host repeating its cursor with non-empty pages was asked for the same page forever. Regression tests: `SpaceClientTests.Enumerators_HostRepeatingItsCursor_StopAfterTheRepeat` (all five space enumerators) and `JetstreamArchiveClientTests.EnumerateSegmentsAsync_ServerRepeatingItsCursor_StopsAfterTheRepeat`; each fails against the old loop (it keeps requesting) and passes with exactly two requests. ## Verification - `dotnet build --no-incremental` of the whole solution: **0 warnings, 0 errors** (samples and integration tests included). - Unit tests: **3180 passed, 0 failed, 5 skipped** (the 5 skips are pre-existing environment-gated tests). - `TypedIdentifierGuardTests`: no violations in my namespaces outside the documented exceptions; `Exceptions_AllStillMatchAMember` and `PendingConversions_AllStillHaveViolations` pass. - New tests: firehose frames with a malformed `repo`/`commit`/`rev`; Jetstream commits with a malformed `collection`/`rkey`/`rev` and an identity with a malformed handle; `SpaceRepoCar.Verify` with an index path that is not `NSID/rkey`; `SpaceTypeDeclaration` with a wildcard collection; delegation and service-auth tokens whose `iss` is not a DID; `ServiceAuthGenerator` audience validation; the enumerator loops above. The `SpaceCommitContext` reference vector (from `@atproto/space`) still passes unchanged, and the unambiguous-encoding test now uses two contexts that concatenate to the same text through a colon-bearing space key and DID. - Test data that used placeholder identifiers (`bafyreiabc`, `3kaa`, `rev123`, …) now uses well-formed ones, since the models validate them. - Integration tests compile against the new API; they are gated on a spaces PDS and were not run. ## Deliberately not done - **Spaces hardening (#133):** open unions and extension data for the Space models, making endpoint and query-parameter classes internal, DID-document strictness, caches. The typed index/path parsing in `SpaceRepoCar.Verify` and the NSID-only `SpaceTypeDeclaration.Collections` overlap two #133 items (each index key exactly one validated `NSID/rkey`; `collections` must not contain `*`); #133 can tick them after review. - **Streaming consolidation (#126):** `TypedFirehoseConsumerOptions.CollectionFilter` stays `IReadOnlySet<string>` (#126 reworks the filter to peek `ops[].path` before deserializing); no common `IRecordEvent` abstraction; `JetstreamOperation` and `RepoOpAction` stay separate enums with identical members rather than one shared type, to keep this a type conversion. - No `EnumerateRepoOpsAsync`: an oplog page carries the head commit, which an item enumerator would drop; `SpaceSyncer` is the consumer of that endpoint. - The `docs/firehose.md` "With Cursor" example still reads `message.Seq` off a raw `FirehoseFrame`, which has no such member; pre-existing and outside the APIs this PR changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit

2026-09-24 23:35:48 +00:00

feat!: typed identifiers for Spaces, the space server and streaming (part of #119 )

CI / pds-integration (pull_request) Successful in 27s

Details

CI / build-and-test (pull_request) Successful in 1m1s

Details

Sync Closures to GitHub / sync-closure (pull_request) Successful in 7s

Details

d78040766a

Part 4 of 4 of the typed public API, following the conventions part 1
set down in CONTRIBUTING.md:

- Spaces: com.atproto.space.* and com.atproto.simplespace.* clients and
  models, SpaceUri/SpaceRecordUri components, commits, the repo CAR, the
  syncer and the credential provider take and return SpaceUri,
  SpaceRecordUri, Did, Nsid, RecordKey, Tid, Cid and AtDatetime. The
  string space overloads and ToSpaceUri()/ToRecordUri() helpers are gone,
  filters come before limit and cursor, cursored responses implement
  ICursorPage<T>, and the client-only request bodies are internal.
- Space server: stores, repo host, verifiers, notifier, query parameters
  and SpaceServerOptions.ServiceDid are typed; the query binder and the
  body deserializer now validate identifiers, so the hand-written
  string validators collapse to a presence check. EF entities keep their
  string columns and the stores convert at the boundary, so no migration.
- Streaming: the subscribeRepos event models (Did repo, Cid commit, Tid
  rev, RepoOpAction for an op's action) and the Jetstream models (Nsid
  Collection, RecordKey Rkey, Tid? Rev, Handle?, AtDatetime? Time, Did
  filters) are typed; RKey is Rkey everywhere.
- ServiceAuthGenerator takes a Did issuer and an Nsid lxm, and checks
  that the audience is a DID with an optional service fragment.
- The spaces and Jetstream archive enumerators run on the shared
  paginator. They stopped only on an empty page, so a host repeating its
  cursor drove an endless request loop.

The guard test's allow-list loses the spaces, streaming and firehose
entries; the members that genuinely carry something other than one
identifier format (JWT iss/sub, DPoP URLs, wildcard collection filters,
raw archive columns) are documented exceptions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

opencode-bot commented

2026-09-24 23:35:49 +00:00

Collaborator

🤖 Reviewing this PR — the review will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-157/run-2026-09-24T23-35-48-704Z.md) — updating every ~5 minutes while the review runs.

Grandiras commented

2026-09-24 23:35:57 +00:00

Author

Owner

@claude /review — large and mostly mechanical. The description has How to review this. The non-mechanical parts are the server-side parameter binding (identifiers are now validated at bind time), RepoOpAction, and ServiceAuthGenerator's audience validation.

@claude /review — large and mostly mechanical. The description has *How to review this*. The non-mechanical parts are the server-side parameter binding (identifiers are now validated at bind time), `RepoOpAction`, and `ServiceAuthGenerator`'s audience validation.

Grandiras referenced this pull request

2026-09-24 23:35:58 +00:00

refactor(streaming)!: one streaming core, correct cursors, faster consumers #126

claude-bot commented

2026-09-24 23:36:02 +00:00

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/596); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-157/run-596.md) — updating every ~5 minutes while the task runs.

opencode-bot commented

2026-09-24 23:37:09 +00:00

Collaborator

⏳ Held on the OpenCode Go window — the 5-hour window is at 2% (the bot holds at 95%).
This continues automatically after the window resets (2026-09-25T03:54:33.092Z) — nothing to re-trigger.

⏳ **Held on the OpenCode Go window** — the 5-hour window is at 2% (the bot holds at 95%). This continues automatically after the window resets (2026-09-25T03:54:33.092Z) — nothing to re-trigger.

claude-reviewer approved these changes

2026-09-24 23:38:40 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Spot-checked the six areas the PR description calls out for close reading, plus a few of the mechanical conversions, against the actual branch content (not just the diff text):

SpaceRepoCar.Verify / SpaceRepoRecord / DecodeIndex — correctly refuses an index path that doesn't split into a valid NSID/rkey pair and an index link that isn't a CID, exactly as described. Cid.Value is used where DAG-CBOR encoding still needs the raw string.
SpaceCommit.cs — SpaceCommitContext.Encode still hashes .Value/.Value text for space/author/rev, so the reference vector is preserved; SpaceCommitVerifier compares commit.Rev != context.Rev using the new Tid equality instead of string.Equals, which is equivalent.
JetstreamEventParser — ParseCommitFields now skips the commit (returns null) when collection/rkey don't parse, and ParseTid/ParseDatetime null out rev/optional timestamps rather than failing the whole event, matching the documented forward-tolerant behavior.
SyncModels.cs — the firehose event models are typed consistently (Did, Cid, Tid, Handle, new RepoOpAction enum with the same members as JetstreamOperation); a malformed field on a required property will throw during JSON binding and get treated as a malformed frame upstream, consistent with the write-up.
SpaceRequestValidation — the collapse to a single generic Require<T> presence check is correct because the identifier types now do their own format validation during model binding; verified this is consistent with Did, Nsid, SpaceUri, etc. all being reference types (sealed record ... : IIdentifier<T>), so the where T : class constraint is safe.
TypedIdentifierGuardTests — the PendingConversions allow-list and its supporting test are fully removed, and the new Exceptions entries genuinely describe fields carrying more than one identifier format (space tokens' dual DID/OAuth-client-ID iss/sub, DPoP htu URLs, Jetstream wildcard collection filters, raw archive row columns pre-ToEvent()).
Verified RecordKey.NewTid() and SpaceUri.Create(Did, Nsid, RecordKey) referenced from CreateSimpleSpaceEndpoint actually exist in the branch and match how they're called.

No correctness issues found in the sampled surface. The typed-identifier conversions in the EF Core stores, in-memory stores, and endpoints follow the same patterns established in the prior three PRs in this series, and the equality/comparison rewrites (space.Owner == caller, rev.CompareTo(existing.Rev), etc.) are sound given these are sealed record types with value equality. I did not execute the build/test suite myself, but the diff is internally consistent with the PR's own verification claims.

⏱ 2m 24s · 18 turns · tokens: 107.0k in / 9.3k out (+2001.2k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Spot-checked the six areas the PR description calls out for close reading, plus a few of the mechanical conversions, against the actual branch content (not just the diff text): - `SpaceRepoCar.Verify` / `SpaceRepoRecord` / `DecodeIndex` — correctly refuses an index path that doesn't split into a valid NSID/rkey pair and an index link that isn't a CID, exactly as described. `Cid.Value` is used where DAG-CBOR encoding still needs the raw string. - `SpaceCommit.cs` — `SpaceCommitContext.Encode` still hashes `.Value`/`.Value` text for space/author/rev, so the reference vector is preserved; `SpaceCommitVerifier` compares `commit.Rev != context.Rev` using the new `Tid` equality instead of `string.Equals`, which is equivalent. - `JetstreamEventParser` — `ParseCommitFields` now skips the commit (returns null) when `collection`/`rkey` don't parse, and `ParseTid`/`ParseDatetime` null out `rev`/optional timestamps rather than failing the whole event, matching the documented forward-tolerant behavior. - `SyncModels.cs` — the firehose event models are typed consistently (`Did`, `Cid`, `Tid`, `Handle`, new `RepoOpAction` enum with the same members as `JetstreamOperation`); a malformed field on a `required` property will throw during JSON binding and get treated as a malformed frame upstream, consistent with the write-up. - `SpaceRequestValidation` — the collapse to a single generic `Require<T>` presence check is correct because the identifier types now do their own format validation during model binding; verified this is consistent with `Did`, `Nsid`, `SpaceUri`, etc. all being reference types (`sealed record ... : IIdentifier<T>`), so the `where T : class` constraint is safe. - `TypedIdentifierGuardTests` — the `PendingConversions` allow-list and its supporting test are fully removed, and the new `Exceptions` entries genuinely describe fields carrying more than one identifier format (space tokens' dual DID/OAuth-client-ID `iss`/`sub`, DPoP `htu` URLs, Jetstream wildcard collection filters, raw archive row columns pre-`ToEvent()`). - Verified `RecordKey.NewTid()` and `SpaceUri.Create(Did, Nsid, RecordKey)` referenced from `CreateSimpleSpaceEndpoint` actually exist in the branch and match how they're called. No correctness issues found in the sampled surface. The typed-identifier conversions in the EF Core stores, in-memory stores, and endpoints follow the same patterns established in the prior three PRs in this series, and the equality/comparison rewrites (`space.Owner == caller`, `rev.CompareTo(existing.Rev)`, etc.) are sound given these are `sealed record` types with value equality. I did not execute the build/test suite myself, but the diff is internally consistent with the PR's own verification claims. --- ⏱ 2m 24s · 18 turns · tokens: 107.0k in / 9.3k out (+2001.2k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-157/run-596.md)

Grandiras merged commit e06a01883f into main

2026-09-24 23:39:10 +00:00

Grandiras deleted branch issue-119-typed-spaces-streaming

2026-09-24 23:39:10 +00:00

Grandiras referenced this pull request from a commit

2026-09-24 23:39:11 +00:00

feat!: typed identifiers for Spaces, the space server and streaming (#157)

Grandiras referenced this pull request

2026-09-25 00:12:11 +00:00

fix(lexicon)!: wire fidelity against upstream lexicons, enforced by a drift test #159