Park the admin surface, audit every CLI mutation, expire delivered outbox entries - #70
Merged
Merged
Conversation
Three things that had to land before 1.0 can freeze anything. **The semver gate was suppressed in four places.** PublicApiAnalyzers was referenced but silent: a root .editorconfig set every RS00xx to none, a publicapi-silence.globalconfig repeated it, a project-local src/Alberto.Dcb/.editorconfig silenced the core package specifically, and a <NoWarn> PropertyGroup in Directory.Build.props listed the rules too. The NoWarn never actually applied — it was conditioned on IsPackable, which the csproj bodies set after Directory.Build.props' PropertyGroups are evaluated, so it always saw an empty value. All four are gone, the rules are at error, and every src/ project now has a captured baseline (2870 entries across 11 packages). Arming it surfaced a repo-wide hygiene bug that has nothing to do with the gate: the SDK defaults IsPackable to true, so every app, test, tool and benchmark project was packable, carrying package metadata and SourceLink. Directory.Build.props now defaults it to false and the packages opt in. Seven overload sets tripped RS0026/RS0027. Each was read individually and each is separated by a required parameter or by delegate arity, never by the optional tail, so none can bind ambiguously. They carry [SuppressMessage] with a justification naming the separating parameter rather than a pragma or a lowered severity. **Telemetry was exporting business data.** The event.appended span event carried event.tags as the full order:8f21,customer:4471 list — a DCB tag value is a domain identifier. It now lists the distinct concepts, which is what identifies the boundary the append was checked against; TelemetryOptions.RecordEventTagValues opts the ids back in. Both consume middlewares and the append interceptor set exception.message and exception.stacktrace as span attributes, where a collector has no unit to act on; they now call Activity.AddException, which is the span event the redaction processors expect to find. Npgsql's messages include the failing SQL, so this was not a hypothetical. **BatchedEfProjection is deleted.** Public, with no registration path — no AddBatchedEfProjection ever existed. Hand-registering it produced no RebuildableProjection, so a rebuild skipped the processor and left it diverged after the others promoted, and it handed the raw DbContext to the handler with no LastProcessedPosition guard, so a crash before the checkpoint write replayed the whole batch. AddEfProjection is idempotent per document, participates in rebuilds, and is itself an IBatchableProcessor. Also in this branch, from the same review pass: - ALB0026: AddAlberto refuses a module key it has already seen instead of overlaying the first module's options and starting a second set of control loops racing on its checkpoint. - ALB2001: a Roslyn analyzer in Alberto.Dcb.Commands that warns when a command pipeline is built and discarded, which appends nothing. [Pure] and [MustUseReturnValue] cannot produce a consumer-visible warning, so this had to be a real analyzer. - The in-memory backend accepted payloads Postgres rejects and returned them verbatim, so a suite could pass in memory and fail against a database. It now validates the JSON, rejects NUL in payload and metadata, and re-emits in canonical jsonb form. It also aliased the caller's tag and metadata collections into the stored envelope; both are copied now. - Backend conformance suite gains three requirements: EventType.Version must be derived from the stored _version tag on read, malformed JSON must be rejected, and NUL must be rejected. Payload round-trip is asserted for semantic JSON equality, not byte equality — Alberto should not promise one PostgreSQL version's exact jsonb output on behalf of every backend. - The Alberto.Dcb -> Alberto.Dcb.Testing.Xunit InternalsVisibleTo, the only grant to an assembly we publish, is down from three internal members to one (EventTag.ForVersion). 1530 tests pass, build is clean.
API-3 — three extension points had members no implementation outside this repo could honour. IDeadLetterStore's claim-and-fence trio moves to an optional IClaimableDeadLetterStore; ExtensionPointContractTests now freezes the abstract member set of every externally-implemented interface, so a member added after 1.0 must ship a default or move to its own interface. PG-1 — the append advisory lock hashed its key to 32 bits, so unrelated tenants could share one lock: ~50% chance of some colliding pair at 77k tenants, silently serializing appends that need not serialize. hashtextextended moves that bound to 2^64. CORE-3 — every Postgres conflict reported position -1 and query *, which is indistinguishable from a real conflict against an all-events query. The backend now reports the expected position and query it was given and parses the conflicting position out of the server's message. PIP-1 — a failed dead-letter write escaped the middleware chain, faulted the processor and held the checkpoint, re-delivering every healthy event in the window on every restart. The write is now retried three times and dropped with an error log; cancellation still propagates. EF-3 — inline EF projections neither filtered nor stamped RebuildVersion, so an entity registered both inline and async threw a duplicate-key ArgumentException out of AppendAsync for the whole rebuild window. EF-4 — an EF projection on a tenant-enabled module is only correct if two tenants cannot produce the same document id, and nothing can verify that. ALB0027 refuses the combination unless the call declares EfDocumentIdUniqueness.AcrossTenants. CORE-2 — ALB0018 caught an uncovered version gap only on the DI path. A hand-built EventSerializer read the older payload straight into the current shape, defaulting every member added since. Deserialize now refuses the gap, waivable at the declaration site with [EventType(UpcastingNotRequired = true)] which ALB0018 honours too.
MSG-1: delivered outbox entries now expire. WithOutbox gains deliveredRetention (default 7 days) and retentionSweepInterval, honoured by a separate OutboxRetentionService rather than a step on the relay's loop — a slow purge delays only the next purge, never publishing. Migration 034 adds the partial index on delivered_at that keeps the sweep off a sequential scan. `alberto ops outbox purge` does the same delete on demand and records an admin-outbox-purged audit event. Documents that outbox delivery is unordered, with RoutingHint as the per-entity ordering hook. CMD-1: a DcbConflictException that outlives Commit's retries no longer escapes OrThrow as an unhandled exception. DcbConflictException gains ProblemCode and ToProblem() so the thrown and TryCommit paths render one shape, and Problem.Details now reach GraphQL as error extensions. ADM: Alberto.Dcb.Admin and the new Alberto.Dcb.Postgres.Admin are parked (IsPackable=false), and every alberto ops mutation routes through IAdminOperator so it is audited in the same transaction as the change.
This was referenced Jul 31, 2026
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
1 participant
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.
Fourth hardening batch on the road to 1.0. Four findings: ADM (park), ADM-1 (audit), CMD-1 (conflict), MSG-1 (outbox retention + ordering).
ADM / ADM-1 — the admin surface
Alberto.Dcb.Adminis nowIsPackable=false, and the three PostgreSQL admin types moved out of the packableAlberto.Dcb.Postgresinto a new, also-unpublishedAlberto.Dcb.Postgres.Admin. ShippingIAdminReader/IAdminOperatorat 1.0 would freeze that abstraction under semver before the GraphQL API, MCP server, console and BFF parked onfeature/admin-surfaceexist. The namespace is unchanged (Alberto.Dcb.Postgres), so nousingmoves; the CLI references both by project and is unaffected.Every mutation in
alberto opsnow routes throughIAdminOperator, which appends an admin event toalberto_eventsin the same transaction as the change. Six commands previously reached past it to the underlying stores and left no trace.IAdminOperatorgainsRenameCheckpointAsyncandMarkDeadLettersForRetryAsync(additive, sofeature/admin-surfacestill merges).CMD-1 — a post-retry conflict is no longer an opaque 500
Commitretries aDcbConflictExceptionup to its attempt limit and then rethrows. The example slices'OrThrowawaited that bare, so a boundary that stayed contended surfaced as an unhandled exception — no code, no positions — on the documented happy path.OrThrownow catches it and raises the sameProblemaTryCommitwould have returned.DcbConflictExceptiongainsProblemCode("dcb.conflict") andToProblem()so both paths render one shape, pinned by a test.Problem.Detailsnow reach GraphQL as error extensions rather than being dropped — which is howexpectedPositionandconflictingPositionget to a client deciding whether a retry is worth it.MSG-1 — the outbox no longer grows forever, and its ordering is written down
WithOutboxgainsdeliveredRetention(default 7 days,Timeout.InfiniteTimeSpanto disable) andretentionSweepInterval(default 1 hour), honoured by a newOutboxRetentionService.It is deliberately a separate hosted service, not a step on the relay's loop — a purge that takes seconds would otherwise be seconds in which nothing is published, and the first sweep after this ships faces the whole accumulated backlog. It waits out a full interval before its first sweep, concurrent replicas are safe (the deletes are idempotent), and a failed sweep is logged and retried rather than faulting the host. Only
deliveredentries are ever eligible;pending,processingandfailedare work, not history.alberto ops outbox purge --before <ts>does the same delete on demand and recordsadmin-outbox-purged. Migration 034 (both tenancy sets) adds the partial index ondelivered_atthat keeps the sweep off a sequential scan —CREATE INDEX CONCURRENTLYunder the house-- alberto:no-transactionmarker.Separately, docs now state plainly that outbox delivery is unordered, for three independent reasons:
created_atis transaction-start time with no tiebreaker (idis a v4 UUID), retries re-deliver late, andFOR UPDATE SKIP LOCKEDhands concurrent relays disjoint batches.ExternalMessage.RoutingHintis the per-entity ordering hook for transports with partition keys, message-group ids or routing keys. No behaviour changed there — it was true before and undocumented.Breaking
The default-on 7-day purge deletes data that nothing removed before.
UPGRADING.mdgets OR-1 with three ways out: keep the old behaviour withTimeout.InfiniteTimeSpan, drain the backlog from the CLI first, or start wide and narrow it. Plus PK-1 for the admin parking.Verification
dotnet build Alberto.slnx→ 0 errors.dotnet test Alberto.slnx→ 1674 passed, 16 skipped, 0 failed (after merging the latestmain, including the repo/solution rename).New tests: 6 for
OutboxRetentionService(interval, cutoff, first-sweep delay, infinite retention, failure recovery, defaults), 2 Testcontainers tests forPurgeOutboxAsync(status/age selectivity + audit event, and no-op writes nothing), 6 forMutationResults.OrThrow.One note on the retention tests:
BackgroundServicestartsExecuteAsyncon the thread pool, soStartAsyncreturning says nothing about the loop having begun — advancing aFakeTimeProviderat that point advances past a timer that does not exist yet. The service exposes aninternal Armedsignal for tests to wait on; nothing outside the tests needs it, because real clocks do not jump.