Repository navigation
feat!: result model, multi-tenancy, crypto-shredding, metrics and subscriptions - #60
Draft
kieronlanning wants to merge 41 commits into
Draft
kieronlanning wants to merge 41 commits into
kieronlanning wants to merge 41 commits into
Conversation
The skills CLI only discovers skills in a root `skills/` folder and delivers them from git, so the previous `Sdk/.agents/skills` location was never reachable by it. The pack-validation rule requiring it inside the NuGet package goes with it. All three existing skills declared `category`, `roles` and `tags` as top-level frontmatter. The specification defines none of them, and a strict client rejects the skill outright. Adds skills for the result model, multi-tenancy and crypto-shredding, and a Skills.UnitTests project so the catalogue is validated rather than assumed - ValidatePurviewSkills silently passed a deliberately malformed skill, so it is not sufficient on its own. AGENTS.md now requires feature work to update both `skills/` and `docs/wiki`, and records the tenant-isolation testing rules that the defects found in this work would have been caught by.
…viour Guarantees-and-Limitations.md is the authoritative summary, so it now covers the failure vocabulary, tenant isolation per store, crypto-shredding, metrics and the originating-user capture. States the limits explicitly rather than implying parity: Azure Storage blob snapshots are not tenant-filtered directly, snapshots are disabled while payload protection is enabled, and enabling tenancy over data written while tenant-less hides it from tenant-scoped reads.
BREAKING CHANGE: the store contract returns results rather than null, bool or provider exceptions. Reads return Result<T, AggregateReadFailure> and saves return Result<SaveOutcome<T>, AggregateSaveFailure>, so "absent" and "deleted" are distinguishable and a save that wrote nothing is a success rather than a failure. SaveResult<T>, LockHandlingMode, ValidationException, IsDeletedAsync and ~36 duplicated provider exceptions are removed; ExistsState becomes AggregateExistence without the implicit bool that flattened "exists in a deleted state" into "exists". Stream versions widen from int to long. Tenant isolation is enforced by every event store, the SQL Server, PostgreSQL, MongoDB and Cosmos DB snapshot stores, and the distributed cache - whose key was previously unqualified, letting one tenant read another's aggregate ahead of every storage filter. Unowned rows are visible only to TenantScope.System, so a missed stamp becomes an absence rather than a leak. Crypto-shredding encrypts the payload of any event implementing IEventDataSubject under a per-subject key; erasing the key makes it unreadable everywhere and replay reports the gap through SkippedEvents. Snapshots are disabled while protection is enabled, because a snapshot holds the same data in clear and cannot be keyed to one subject. Also fixes several defects found along the way: the EF source generator shipped in three provider packages, so referencing two of them failed a consumer build with CS0579; concurrency conflicts escaped as exceptions on some providers and as DbUpdateException on others; Azure Storage's batch FailedEntity was never assigned, making its conflict classification dead code; transactions discarded the save failure; and the admin health endpoint reported a hardcoded "Ready".
…enarios
Tenancy was proved per-provider, in four near-identical copies, so a new
provider could be added without any tenancy coverage at all. The tests now
live in the shared event-store contract suite behind an abstract seam:
protected abstract IEventStoreCore<TAggregate> CreateTenantScopedEventStore(
TenantScope scope, Guid runId);
Abstract rather than virtual, so a provider cannot be added without stating
how it is scoped. Both scopes in a test share a runId and therefore the same
physical storage - sharing storage is what the isolation protects, and a
provider returning an isolated store per scope would pass while proving
nothing.
Five shared tests replace the four per-provider copies, adding the cached-read
and unowned-aggregate cases that only one provider had.
Also adds EventSourcing.AcceptanceTests: the guarantees stated as BDTest
scenarios in domain language, over reusable [StepText] steps, against the
in-memory store so they need no container. The AcceptanceTests suffix gives
them the Acceptance category, so PR validation's unit filter excludes them.
The GA 4 plan existed only as the label "subscriptions/retention", so this writes the design down before any code. Marked clearly as a proposal: nothing on the page is implemented. The substance is that there is no global event order to subscribe to. Streams are per-aggregate on every provider, and Azure Table Storage cannot have a cross-partition sequence at all, so "subscribe to the event stream" is a choice about what ordering a subscriber is promised rather than one design. Three options are compared with their real costs; per-aggregate catch-up is recommended as the only one implementable everywhere and honest about its promise. Retention is the inverse of the framework's first invariant - the snapshot becomes load-bearing - so the page states that pruning without a covering snapshot must not be possible, and lists the interactions (crypto-shredding, upcasting, lagging subscriptions, tenancy) that constrain it. Ends with the decisions needed before implementation starts.
The previous draft claimed a global sequence would break provider parity because Azure Table Storage cannot hold one. That was too strong: a capability gated on configuration and reported through EventStoreCapabilities is how this repository already models capabilities that are not universal. Adds Option D - publish through the existing outbox to an ordered log, so a provider that cannot offer a sequence itself still gets ordered delivery. Distinguishes transports that qualify from those that do not: Kafka and Event Hubs have offsets and replay, Service Bus has FIFO sessions but no replay, so a subscriber added later sees nothing. That difference decides the design. Option D layers on Option A rather than replacing it, because a transport cannot be the source of history - it holds nothing published before it was enabled, and retaining from genesis makes it a second full copy of the stream. Catch-up reads the store; live tail reads the transport. Records the two costs this exposes: the outbox does not currently preserve order under retry, and no transport integration exists today.
Adds IEventSubscriber<T> and a catch-up processor that reads each visible aggregate stream from a recorded position. Unlike the change feed and the outbox, which only carry what happened while they were running, a subscription reads the stream itself - so a projection added today can be built from history. Delivery is at-least-once and ordered within an aggregate. There is no ordering between aggregates, because no provider holds a global sequence; docs/wiki/Subscriptions-And-Retention-Design.md records why. Tenancy is structural rather than remembered: the TenantScope is part of SubscriptionCheckpointKey, so a position cannot be written without stating its tenant. Two tenants sharing one would let whichever ran first advance it past the other's unread events, which would then never be delivered. The tenancy tests assert on what discovery returns, not on what the subscriber receives. Removing the discovery filter fails two of them while every acceptance scenario stays green - the event read is filtered too, and that second filter masks the first. A subscriber that throws stops its stream at the failing event and retries from there, so a poison event stalls one stream indefinitely rather than being skipped. Documented, because skipping loses data silently. Retention and the ordered live tail remain unimplemented.
Subscriptions previously replayed every stream after a restart, which made them unusable in a deployed application. AddSqlServerSubscriptionCheckpoints and AddPostgresSubscriptionCheckpoints persist the position instead. Also removes the stream-discovery duplication introduced with subscriptions. IEventStoreCore<T>.GetAggregateIdsAsync already existed on every store and is already tenant-filtered - by the EF global query filter on the SQL providers, the scoped table client on Azure, and IsVisible on InMemory. The default discovery now reads that, which deletes two implementations and makes subscriptions work on Azure Storage, where the previous snapshot-store default could never have run. The scope is encoded into one non-null column by a shared helper rather than per provider. A nullable tenant column would not behave the same on both: PostgreSQL treats NULLs as distinct in a unique index, so the system scope would silently accumulate duplicate rows. The concurrency test found a real defect: PostgreSQL's CREATE TABLE IF NOT EXISTS is not safe to run concurrently - it still races on the table's implicit row type and raises 23505. Both stores now serialise creation in process and tolerate the cross-process race, which no in-process lock covers. Azure Storage and MongoDB still ship no durable checkpoint store.
Completes durable checkpoints for every provider that persists an event stream. Cosmos DB has none, so there is nothing to checkpoint. The monotonic guarantee - a position never moves backwards - has to be expressed differently per engine. MongoDB uses a filtered upsert and treats the resulting duplicate-key error as "already further on", which is the documented consequence of an upsert whose filter excludes the existing document. Azure Table Storage has no conditional-on-value write at all, so it reads and then writes conditionally on the ETag, retrying on contention and giving up after MaxWriteAttempts - leaving the position behind, which redelivers rather than skips. Neither composes its key by concatenation: a subscription name, tenant or aggregate id containing the delimiter would make two different keys identical. Azure Base64Url-encodes each segment, Mongo uses a compound _id document. Adds ConnectionString accessors to the Mongo and Azure fixtures so a test can build its own client.
Moving purview-build.json to .config/ in 4d72eda also widened the test selection: TestProjects went from *UnitTests.csproj to *Tests.csproj and TestFilter dropped the [Category=Unit] predicate. That made PR validation run every test project with no category filter, including the provider integration suites and Samples.AppHost, which need a container runtime and an Aspire host. On a hosted runner those fail at "Connection refused", which is what red CI on this branch was. AGENTS.md already states the pipeline never runs provider integration tests or the performance harnesses; this restores that. Also corrects the config path in AGENTS.md and README.md, which still pointed at the repository root.
…plit The EF Core support library and its source generator were moved into their own package so three providers no longer each embed a copy of the analyzer, which was producing duplicate-attribute errors in consumer builds. The pack validation rules were not updated with it, so they still required Postgres, SqlServer and CosmosDb to contain Purview.EventSourcing.EntityFrameworkCore.dll and the analyzer - content those packages correctly no longer carry - and the new package had no RequiredContent rule at all, which RequireExplicitContent rejects. Rules now match the packages that are actually produced, verified by reading the nupkg contents rather than by assumption.
kieronlanning
marked this pull request as draft
October 26, 2026 08:32
Detailed designs for each gap from the enterprise-readiness assessment, all marked as proposals. Each states the problem with evidence from the code, a design, phases, the tests that would prove it, and the decisions still open. Crypto key management is the blocking one and carries the explicit requirement for local, in-memory and external stores. Three tiers over the existing IEventCryptoKeyStore, which does not change: in-memory for tests, SQL/Postgres for self-hosted, Key Vault for external custody - recommending envelope encryption so a vault holds one KEK rather than a secret per subject, which does not scale. Two findings the plan had to confront. ProtectedPayload records alg and sub but no key identifier, so rotation is impossible without a payload format change; adding an optional kid is backward compatible because absent means "the only key". And erasure is only as irreversible as the least-irreversible copy - Key Vault purge protection and database backups both silently defeat shredding, which makes them operational guidance rather than footnotes. Retention supersedes Part 2 of the subscriptions design and widens it to idempotency markers, which grow unboundedly today and are safe to delete, so that half can ship first. Observability parity records the measured state: tracing exists on SQL Server and Postgres only, not on MongoDB, Azure Storage or Cosmos DB, though the cross-provider metrics are uniform.
Crypto-shredding was a trap rather than a feature: InMemoryEventCryptoKeyStore was the only implementation, so a restart made every protected event permanently unreadable - indistinguishable from erasing every data subject, with no error and no way back. Nothing stopped it reaching production. Two changes close that. A start-up check refuses a key store that declares itself non-durable outside Development, overridable by an explicit option; it runs at start-up rather than first use, so a misconfiguration cannot reach the point where events have been written under keys about to be lost. The check tests for INonDurableEventCryptoKeyStore rather than a concrete type, so a store the framework has never seen is covered too. And SQL Server and PostgreSQL key stores, tenant-scoped: the same data subject id in two tenants is two different people, so they get different keys and neither tenant can read or erase the other's. Verified by sabotage - removing the tenant from the predicate fails three tests. The Postgres concurrency test found a real bug. ON CONFLICT DO NOTHING returns no row, and a fallback SELECT in the same statement runs against the same snapshot, so it cannot see a row a concurrent transaction just inserted: the losing caller got nothing and threw. A no-op DO UPDATE takes the lock and returns the stored row either way. Had that shipped, concurrent first use of a subject would have failed intermittently under load. Also consolidates the tenant column encoding. Subscriptions had its own copy; crypto keys would have been a second, and two encodings of one concept is how one tenant's row collides with another's. Key Vault, caching and rotation remain planned, not built.
Completes durable key storage for every provider that persists an event stream, so crypto-shredding is no longer viable on the SQL providers only. Cosmos DB has no event stream, so there is nothing to protect. Each engine expresses atomic get-or-create differently, and getting it wrong is not a performance problem: two callers each generating their own key means payloads written by the loser can never be decrypted. Azure adds the entity and re-reads on 409; MongoDB uses $setOnInsert with an upsert and ReturnDocument.After, which is atomic without a second round trip. Tenant scoping is structural rather than filtered - the tenant is part of Azure's partition key and of Mongo's compound _id - so a query cannot forget it. Verified by sabotage: collapsing the scope fails three tests per provider. Also lifts Azure's Base64Url key encoder out of the subscription checkpoint store into a shared helper, for the same reason the tenant column was shared: a second copy is how two side tables drift into colliding keys. Documents the erasure caveat per provider, since each has its own way of bringing a shredded key back - Azure soft delete and point-in-time restore, a Mongo replica lagging the delete, a SQL point-in-time restore.
Model A of the two described in the key-management plan: one vault secret per subject, so erasure is a vault operation inheriting its access control and audit trail. Ships as a new provider-neutral package, Purview.EventSourcing.AzureKeyVault. Purge protection is the part worth knowing about. A soft-delete vault keeps a deleted secret recoverable for its retention period, so deleting alone is not erasure - the store purges by default to complete it. Purge protection forbids purging outright, which is a reasonable safeguard and incompatible with erasing on request; rather than let a shred appear to have worked, the store raises and explains the conflict. Opting out is possible but has to be deliberate. Secret names are a SHA-256 of tenant and subject rather than either value in clear. A vault name appears in portal listings and diagnostics visible far more widely than the key itself, and the tenant and subject are recorded as tags for when an operator legitimately needs them. SecretClient could not be substituted in tests - its public methods route through a non-virtual overload and the stub was never called - so the store depends on a narrow IKeyVaultSecretOperations seam with a thin SecretClient adapter. That also keeps the decisions worth testing in the store rather than behind an SDK call. Pins Azure.Identity and the Key Vault packages to the versions Aspire already requires transitively; lower central versions were a downgrade error.
Model B: a per-subject key generated locally, stored wrapped in a durable store, with the wrapping key held in the vault and never leaving it. The vault holds one key rather than one per subject, so this scales to any subject population and a stolen copy of the durable store is unreadable. The trade against Model A is stated in the docs rather than left implicit: erasure here deletes a row, so it does not appear in the vault's audit trail. Storing a wrapped key needs the durable store to persist bytes it was given rather than generate its own, which the existing contract could not express. ISuppliedKeyEventCryptoKeyStore adds get-or-add; all four durable stores now implement it, their existing GetOrCreateAsync becoming a call to it with a freshly generated key. No behaviour change. The first version of the test for "the durable store never holds a usable key" did not work. The wrapping stub reversed bytes, which is its own inverse, so a store that skipped wrapping entirely and wrote the plaintext key still passed - the sabotage check caught it. The stub now prefixes a marker and unwrapping demands it, so unwrapping something never wrapped fails loudly; the same sabotage now fails four tests. Also replaces IsEquivalentTo with base64 comparison for key material, since it is order-insensitive and would have accepted a reversed key as equal.
An uncached key store makes every protect and unprotect a round trip. Against a database that is wasteful; against Key Vault, where reads are throttled and billed, it is impractical - which is what made caching the blocker for using a vault at volume. The rule that makes it safe is that a cached key must never outlive its erasure. ShredAsync evicts, deletes, then evicts again: a reader racing the delete can repopulate the entry between the first eviction and the delete landing, and only the second eviction removes it. Writing that test was itself the interesting part. The first version invoked the racing reader after the inner store had already removed the key, so there was nothing live to repopulate with and the test passed even with the second eviction deleted. Inverting the stub's ordering makes the race real, and the same sabotage now fails. Caching also broke the non-durable start-up guard, which resolves IEventCryptoKeyStore and would now have found the cache wrapper rather than the in-memory store underneath - silently disabling the guard that exists to stop exactly that configuration. IEventCryptoKeyStoreDecorator lets the check unwrap to the real store, and a test asserts it still bites through the cache. The cache is per process, so KeyCacheDuration is the upper bound on how long an erasure takes to take effect across a cluster. Documented as a compliance parameter rather than a tuning one. Key material is never written to a distributed cache.
Completes the key-management plan. A subject can hold several key generations; the highest is current and encrypts new payloads, and ProtectedPayload records which one was used, so rotating never makes existing events unreadable. Earlier generations are retained deliberately - rotation limits what a future compromise exposes, it does not re-encrypt history. Two deliberate narrowings against the plan. Rotation is an optional IRotatingEventCryptoKeyStore rather than a change to IEventCryptoKeyStore, so stores adopt it independently and nothing breaks for anyone who has implemented the interface; and it ships for the two SQL providers only, with the rest documented as not supporting it. The kid field is absent on every payload written so far, which a reader treats as "the subject's only key" - backward compatible, no upcaster. Rotation and erasure stay distinct: ShredAsync removes every generation, because one left behind would mean the subject's older events were still readable. There is a test for exactly that. The capability flag matters more than it looks. The cache decorates the key store, so a protector testing for the interface alone would have missed rotation behind it, or cast the decorator and thrown. SupportsRotation lets the decorator report what the store it wraps can do, and the cache passes rotation through - evicting the cached current generation around a rotate for the same reason a shred evicts twice.
Tracing existed on SQL Server and PostgreSQL only, while the documentation implied parity. Azure Storage, MongoDB and Cosmos DB now emit spans too, and Azure and Cosmos gained the meter and logger they were missing. Span names match the existing SQL Server contract deliberately: a trace query built against one provider has to keep working when a service moves to another, so span names are a compatibility contract in the same way instrument names already are. Cosmos DB gets snapshot and query spans rather than the aggregate ones. It persists no event stream and delegates aggregate reads and writes to an inner event store that already emits those, so repeating them would double-count every operation in a trace. Two things I set out to do and did not. I had recommended putting aggregate ids behind an option defaulting to off; they are already recorded by the two SQL providers, so suppressing them only on the new ones would have swapped one inconsistency for another. Making it optional should change every provider together, and is noted rather than half-done. And the span assertions do not exist. I added them to the shared contract suite, where a new provider could not omit them, and they failed everywhere - the fixtures inject mocked telemetry, so nothing real is ever started. Rather than leave assertions that cannot fail, they are removed and the gap is recorded in the plan and the guarantees page: the wiring is covered by compilation and by the provider suites exercising those paths, which is weaker than it should be.
The audit trail died with the process - InMemoryAdminAuditLogger was the only implementation - so the record of who read which payload or triggered which rebuild was lost on restart. In a regulated deployment the audit trail is the control, which made that unusable. Entries now carry the tenant the operation acted on and a correlation identifier. Both were missing from AdminAuditEntry: "who did this" is incomplete without "to which tenant" when one administrator may be entitled to several, and an entry that cannot be tied to the events it caused answers very little. They are stamped centrally so an endpoint added later cannot omit them. Audit writes default to refusing the operation when they fail. Both answers are defensible, so the choice is an explicit option rather than an assumption, but the default is that an action nobody can prove happened is worse than an action that did not happen. My first attempt at the stamping wired it by decorating whatever was registered at the time, which broke both ways and took 49 unit tests with it: called before the logger there was nothing to decorate, and a durable logger registered afterwards replaced the decorator outright, losing the stamping silently. A keyed sink registration removes the ordering question entirely. The decorator also resolves the tenant and correlation services leniently - an Admin-only host need not have registered the event store, and audit stamping must not be what stops it starting. Append-only is enforced by the absence of an update or delete path; the framework cannot grant database permissions, so that is documented as a deployment control. A PreviousHash column is reserved for hash chaining. No read side, no Azure or MongoDB logger, and no audit retention yet.
This work spans more sessions than fit in one context window, and an earlier session lost its original request to compaction entirely - it had to be recovered from the transcript. Keeping the programme state in conversation is therefore not safe. Production-Readiness-Status.md records what has shipped, what has not, and which decisions block work, with every claim checked against the code rather than recalled. The five design pages keep the reasoning; this page keeps only the state, so there is one place to look and one place to update. AGENTS.md points at it from the layout table and from workflow step 5, which is what makes it survive: that file is re-read after compaction, so a future session finds the pointer even having lost everything else. Also records the gaps found while doing other work and deliberately left - the unguarded CREATE TABLE race still in PostgresOutboxStore, Azure blob snapshots only transitively tenant-filtered, tracked .agents/skills, absent Changeset tooling, and the lack of any upgrade test on a branch whose headline change is breaking.
First phase of the retention plan, and deliberately the safe half: markers exist to deduplicate a retried save, so their usefulness expires with the retry window - minutes, not years. Nothing removed them before, so they accumulated for the life of a deployment. Deleting an expired marker cannot lose data, which is what separates this from event-stream retention. The worst case is a save retried after the window elapsed being applied twice, and optimistic concurrency already rejects that. A zero or negative retention is refused outright, because that would delete markers for saves still in flight and reintroduce the duplicate they exist to prevent. Cleanup honours the current tenant. For a bulk delete that matters more than for a read: forgetting the filter destroys another tenant's data rather than exposing it, and the shared contract suite asserts one tenant's cleanup leaves the other's markers behind. Two things found on the way. MongoDBClient had an unused DeleteManyAsync overload that did not apply the tenant scope, sitting beside scoped siblings - no callers, but exactly how the wrong overload gets picked later, so it is now scoped. And the Mongo event store lives in Purview.EventSourcing.MongoDB despite its Events folder, so a partial declared from the folder name silently compiled as a second class; the resulting error pointed at the interface rather than the cause. My first version of the contract test asserted nothing: markers are opt-in via EventStoreOperationContext.UseIdempotencyMarker, so a plain save writes none and cleanup correctly removed zero. The test now enables them. Nothing schedules the cleanup; it is a maintenance call.
Phase 2 of the retention plan, and deliberately only the dry run. A preview that reports blockers is how an operator learns a policy is wrong before it costs them, and pruning an event is the one operation that can destroy business state permanently - so the destructive path waits on decisions that should be stated rather than inferred. The preview turned out to be provider-neutral. Everything it needs already exists on contracts every store implements: stream discovery, the snapshot and current versions on the aggregate, and the event range for timestamps. No provider implements anything. A stream is blocked when there is no covering snapshot, when its events are newer than MinimumAge, when a subscription has not read past the prune point, or when the retained-version margin leaves nothing. The prune point is the most conservative version all the rules allow. Subscription lag was the part the plan assumed and I had to check: there is no registry of subscription names, and the checkpoint store only reads by exact key. Resolving the registered IEventSubscriber<T> instances supplies the names without any new storage capability - at the cost that a subscription running in another process is invisible, which is documented rather than left to be discovered. Both safety blockers are verified by sabotage: removing the snapshot guard fails six tests, removing the subscription-lag check fails one. RequireSnapshot exists so the preview can report what a deployment has configured, not so it can be switched off casually; its documentation says so.
CI failed with "Sequence contains no elements": a .Single() on an empty Eligible list. The age assertions compared a one-millisecond MinimumAge against events written microseconds earlier, so whether a stream came back eligible or TooRecent depended on how fast the machine was. Worse on Windows, where DateTimeOffset.UtcNow advances in roughly 15ms steps - the comparison was not close to reliable there either. It passed locally by luck and failed on the first CI run. Seeding now waits past the age floor with a margin the clock cannot close, and the floor is named rather than repeated as a literal.
Changesets were used once and are not any more: there is no .changeset folder, no @changesets/cli, and no CHANGELOG.md, but AGENTS.md, the README and the release flow all still instructed people to use them. Following those instructions would have meant looking for tooling that is not there. The version is bumped directly in package.json and the GitHub release generates its notes from the commits and pull requests it contains - which is the reason commit messages here carry the consumer-visible detail rather than a changelog file. Also drops the corresponding entry from the production-readiness status page: the absence of changeset tooling was recorded there as a gap, and it is not one - it is the current design.
Completes retention. Both invocation modes ship as independent flags, so a deployment can enable operator-invoked, scheduled, both, or neither - EventRetentionOptions.Mode defaults to None, because adding a registration must never start deleting history. RequireSnapshot = false is permitted, and logged as a warning naming the risk on every run that uses it: those events are removed with no snapshot behind them and the state they carried cannot be reconstructed. Permitted because a deployment may have accepted that; loud because nobody should be able to say afterwards that they were not told. Pruning is also logged per stream rather than per run, since a single total cannot answer what happened to one aggregate. A lagging subscription blocks. That answer has a consequence worth stating: there is no way to retire a subscription, so an abandoned one blocks retention indefinitely. Recorded as the remaining gap rather than worked around. The safety rules stay in one place. IEventStreamPruning is deliberately the dumbest contract that works - it deletes what it is told to - and every rule is evaluated once by the preview, so a prune cannot quietly do more than the dry run an operator approved. Azure deletes the blob behind an oversized event as well as its pointer row; otherwise pruning would leak storage nothing will ever reference again, and nothing would report it. Verified by sabotage: widening the SQL delete past the prune point fails two contract tests, including the one asserting later events survive.
… misbehaves The suite covered the happy path and expected failures. The failures that actually corrupt an event store are a different kind: a write that committed but was never acknowledged, a caller retrying an operation that already happened, a failure between two writes meant to be one. None of those paths were reachable. FaultInjectingEventStore wraps any IEventStoreCore and fails chosen calls. FailAfter is the fault worth having: the call reaches storage and succeeds, then throws, so storage changed and the caller believes it did not. That is what produces duplicates, because retrying is the natural response. FailBefore only proves nothing was written. Faults are scheduled by call number rather than randomly. A test that fails the third save reproduces; one that fails a random save turns a real defect into an intermittent annoyance nobody trusts. The invariants run against the in-memory store, so they are unit tests on every pull request with no infrastructure: events survive a lost response, a retry does not double-apply, a failed save leaves nothing behind, versions stay contiguous across a failure, and each holds under a tenant scope - because a failure path is exactly where a scope gets dropped. Authored in SharedTestingFramework so provider suites can reuse them, and linked into the unit project rather than referenced: that project drags in every provider and the container packages, which would make the unit suite slow and infrastructure-dependent for two files. Writing the prune invariant found that InMemory had no IEventStreamPruning while all four persistent providers did; it has one now. Verified by sabotage: collapsing FailAfter into FailBefore - so storage no longer changes before the throw - fails the two lost-response tests.
I recorded that an abandoned subscription blocks retention indefinitely. It does not. The lagging check reads the registered IEventSubscriber instances, so a subscription whose code has been removed contributes no name and cannot block anything. What actually lingers is its checkpoint rows, which nothing deletes - storage waste, not a blocked prune. A reader acting on the original wording would have gone looking for a retirement mechanism to fix a problem that is not there.
I started building an upgrade suite that reads 1.x-shaped data, which had already been ruled out: 2.0 targets new deployments and supports no migration path. The work is removed. Both plans now say so explicitly rather than listing the absent suite as a gap, because a gap entry invites exactly the mistake I just made - the next reader sees "no upgrade test" and starts writing one.
… endpoints The per-endpoint tests cover the endpoints that exist today. These cover the set: they enumerate EndpointDataSource and assert a property of all of it, so an endpoint added later is covered without anyone remembering to cover it. The assertion that bites is NamesAnAuthorizationPolicy. The route group applies a blanket RequireAuthorization(), which is authentication only - any signed-in caller passes it. An endpoint that forgets its own RequireAuthorization(policyName) stays "protected" as far as every other test can tell, while being readable by anyone who can log in. Verified by removing one: that test failed and nothing else did. Also fixes the unguarded CREATE TABLE IF NOT EXISTS in PostgresOutboxStore, the last copy of a race already fixed in the checkpoint and crypto key stores. PostgreSQL races on the table's implicit row type and raises 23505 regardless of IF NOT EXISTS.
Audit phases 3 and 4. Before this, two of the four providers had no durable audit at all - the record of who read which payload died with the process - and none of them had a read side, so the trail was evidence you could not produce. IAdminAuditEntryReader is separate from IAdminAuditLogger on purpose: writing is something every Admin request does, reading is a privileged investigation, and a deployment can grant the two to different database principals only if they are different types. It also keeps the logger implementable by a write-only sink. Tenancy first, and all three outcomes asserted on every provider: the owner reads it, another tenant does not, System does. Unowned entries fail closed - visible to System only, never to everyone, because one missed stamp must look like absent data rather than another tenant's trail. AdminAuditQuery deliberately carries no tenant; it comes from ICurrentTenant, so a caller cannot widen their own scope. On Azure Storage the tenant is the partition key, not a predicate - a partition a query never names cannot be returned by accident. Its row key is inverted ticks, so a tenant-scoped read is newest-first with no sort, plus a random suffix because two entries written inside one tick would otherwise collide and AddEntity would fail the second, losing an entry under the failure mode meant to be the safest. Verified by sabotage, not by the tests passing: disabling the SQL Server predicate failed 2 of 7, disabling the Azure partition clause failed 2 of 8. Both the denial and the unowned-row test, which is what should fail and nothing else. Stated plainly rather than implied: a system-scope read on Azure Storage spans partitions, is not ordered by the store, and sorts in memory - bound it with FromUtc. There is no Admin API endpoint for the trail, by decision. Nothing deletes an audit entry on any provider, ever; the retention floor is phase 5 and is not done.
A subscriber that throws stalls its stream at the failing event - and every event behind it - forever. That was deliberate, because skipping loses data silently, but it was not an answer. Dead-lettering needs two deliberate steps, and neither is a default: MaxDeliveryAttempts is 0, and the default sink accepts nothing. Set one without the other and behaviour is unchanged. An escape hatch that opens too easily turns a visible stall into silent data loss, which is strictly worse than the stall. The sink's return value is the contract: the checkpoint advances past the event only when DeadLetterAsync returns true. A sink that refuses, or throws, leaves the stream stalled - correct, because the alternative is losing the event with nothing anywhere recording that it happened. DeadLetteredEvent identifies the event rather than copying it. The stream is still the canonical record and is not altered, so everything needed to fetch and redeliver it is present - and a sink holding a second copy of the payload would outlive crypto-shredding. Attempts are tracked by a singleton, because the processor is scoped and rebuilt every cycle; a counter inside it would reset and never reach any budget. The default counts in memory, so a restart grants another MaxDeliveryAttempts. Stated rather than hidden: that errs toward retrying too often, not abandoning too early. The first version of the default-behaviour test passed with the default sabotaged - the fixture was supplying its own 0 rather than letting the option default through, so it asserted the fixture, not the framework. Fixed, and the option default now has its own test that does fail when changed.
A checkpoint row exists per subscription, per tenant, per aggregate, and nothing removed them. Retiring a subscription left one for every stream it had ever read, for the life of the deployment, on a table that is otherwise only written to. ISubscriptionCheckpointMaintenance is implemented by all five stores. Two properties are deliberate: Operator-invoked, never scheduled. Removing a position for a subscription that is still running restarts that stream from version 1 and redelivers all of it. Delivery is at-least-once so that is a cost, not a corruption - but it is a cost, and nothing should incur it on a timer. No wildcard. Each call names one subscription AND one TenantScope, matched exactly; System is a scope, not "every tenant". Clearing all tenants means a call per tenant. A delete-everything-for-this-subscription switch is how a deployment replays itself by accident. The contract is shared rather than written four times, because each provider expresses the key differently - two columns, a compound _id, a composed partition key - and a delete that is right in one shape is easy to get wrong in another. Azure gets this structurally: subscription and tenant are already the partition key, so a retirement is one whole-partition delete that cannot catch anything else. Verified by sabotage: dropping the tenant clause from the Postgres delete failed both isolation tests and the count test, and nothing else.
…as called Every provider fixture injects the telemetry mock. That records the store calling telemetry and returns no Activity, so an ActivityListener attached to one sees nothing and a span-name assertion written against it passes while proving nothing. Span names are a documented compatibility contract, and until now no test could tell whether one still reached a listener. SqlServerEventStoreFixture gains CreateEventStoreWithRealTelemetry, which wires the generated implementation instead. It is a separate method rather than another optional parameter on CreateEventStore: that signature is called from most of the suite and widening it pushed an unrelated outbox test over the class-coupling analyzer. Writing these found a real one. DeleteAsync emits DeleteAggregate only for a permanent delete - a soft delete appends a Deleted event through the ordinary save path and is traced as SaveAggregate. An alert counting deletions by span name would only ever have seen the permanent ones. Both cases are now asserted, and the fact is documented. Stated rather than implied: this covers SQL Server. PostgreSQL, MongoDB and Azure Storage still use the mock everywhere, so their spans rest on call-level assertions and compilation. That is a gap, not parity, and the feature matrix now says so per provider.
FaultInjectingEventStore fails at the store boundary, which covers the framework's own logic and cannot reach what lives below it: a connection reset part-way through a write, a reply that never arrives, a half-written request. Those decide whether the provider's SDK retried, and whether that retry duplicated anything. FaultingTcpProxy sits between the client and the container and severs the connection on command. Written here rather than adding Toxiproxy: toxics are probabilistic, and a test that fails a random write turns a real defect into an intermittent annoyance nobody trusts. This faults the nth connection or after an exact byte count, so a failure reproduces - the same reasoning behind FaultPlan scheduling by call number. It also avoids a container and a dependency the repository then does not need. Severing the reply rather than the request is the fault that matters: the server has the write and may have committed it, and the client is told only that the connection went away. The caller retries something that already happened. The invariant asserted is that the resulting stream has contiguous, unique versions - a duplicate shows as a repeat, a lost event as a gap, and asserting the count alone would miss both. The SDK's connect retries are disabled, or these measure the retry policy rather than the stream. Two of the tests accept either outcome (committed or not), so both now assert the proxy saw a connection - without that they pass when nothing was faulted, which is how a fault-injection test quietly stops injecting. Stated rather than implied: SQL Server only, and the proxy is not protocol-aware. It severs connections convincingly and cannot corrupt one in a protocol-aware way.
RemoveSubscriptionAsync_LeavesAnotherTenantsCheckpointsAlone failed in two of three full Postgres suite runs and has never failed with the class run alone. The assertion that fired was the post-delete read, which cannot distinguish "the write never landed" from "the delete took one row too many" - and those need completely different fixes. Both tenancy tests now assert the other scope's checkpoint is present before the delete. Three full-suite runs have been clean since, but the new assertion has not fired, so nothing has been diagnosed and nothing is claimed fixed. Recorded as an open intermittent in the readiness status with what is actually known: it is an assertion failure rather than an exception, and the delete predicate names both the subscription and the tenant, so it cannot match another tenant's row.
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.
Fifteen commits that had accumulated unreviewed on a branch named
ef-cosmos, which stopped describing the contents several features ago. Renamed before pushing.What is in it
Result and failure model — expected failures are returned as results rather than thrown; exceptions are reserved for genuine faults. Breaking (
feat!).Multi-tenancy —
TenantScope+ICurrentTenant, with an explicit system scope rather than a null tenant, so a resolver that fails to determine a tenant cannot be mistaken for permission to read every tenant. Enforced across every provider's read and write paths, the snapshot stores, the distributed cache and the Admin API.Crypto-shredding — per data subject, with a skipped-event replay path.
Metrics and principal capture — one meter, identical instruments across providers.
Tenant isolation as a provider contract — the tenancy tests moved out of four near-identical per-provider copies into the shared event-store contract suite, behind an abstract seam, so a provider cannot be added without stating how it is scoped.
Acceptance suite — guarantees stated as BDTest scenarios in domain language, excluded from PR validation by category.
Subscriptions — catch-up reads over the event stream, delivering events written before the subscriber existed, with durable checkpoints for every provider that persists a stream.
Defects found and fixed along the way
Each of these was live while the suite was green:
BatchOperation.FailedEntitynever assigned, which made Azure's failure classification dead code."Ready".CREATE TABLE IF NOT EXISTSis not concurrency-safe — it races on the table's implicit row type and raises23505. Found by a concurrency test on the new checkpoint store.Validation
Release build: 0 warnings, 0 errors.
csharpierclean across 899 files. Pack produces 19 packages with no warnings.Tenancy was verified by sabotage, not just by a green run: removing a tenant filter or collapsing the scope encoding fails specific tests, and those failures were observed before each filter was restored. This mattered — an early acceptance-level tenancy test passed with the discovery filter removed, because the event read is filtered too and masked it. The tests now assert on what discovery returns, not on what arrives.
Known gaps, stated deliberately
PostgresOutboxStorehas the same unguardedCREATE TABLE IF NOT EXISTSrace fixed in the checkpoint stores — a real latent bug, left out of scope here..agents/skills/is tracked (59 files, duplicated into.claude/skills/) even thoughAGENTS.mddocuments it as gitignored. Pre-existing, not touched.Note on review
706 files and ~33k added lines. The commits are individually coherent and are probably easier to review one at a time than as a single diff.