feat(tenancy): resolve tenants by host, provision them, seed two - #15
Merged
Conversation
The roadmap's Packet 7 block under-scoped the packet by roughly half: ADR-0036 additionally assigns host classification, TenantContextFactory, TenantContextOrigin, IOrganizationScopeValidator, DenyAllTenantMembership- Reader and eight architecture tests to it, none of which the roadmap named. An implementer working from the roadmap alone ships half a packet. Alongside that, six statements the implementer reads first were false. Two Accepted ADRs disagreed on whether the app.tenant_id setter set is closed at six. The one written CachedHostToTenantResolver body — the block an implementer copies — dropped the is_active term ADR-0036 requires and justified the omission with a premise that is false against the shipped migration, and it routed the unknown-host answer through a cache whose stored null never reads back as a hit while still consuming a globally evicted slot. The marker rule keyed on "has a TenantId property", which captures platform_host_to_tenant — the one table read to determine the tenant — and misses tenants, whose id is the tenant id. And TenantContextBehavior still claimed RLS is not enforced at runtime, which Packet 6 falsified. The five decisions Packet 7 was handed are settled here rather than discovered in review: the aggregate boundary resolves as promotion, the app.scope carrier is a forced deferral to Phase 02b, the tenant_locales single default is a partial unique index, an authority-ceiling refusal is byte-identical to an unresolvable host, and the request-level suite drives a test-only controller rather than claiming Phase 02d's first endpoint. Behaviour is unchanged: the code diff is comments only, and the suite is byte-for-byte the pre-pass baseline at 943 green with zero skips. ADR: 0042 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eight lenses over f8e0aa6, every finding independently verified. Three classes of defect were real. An erratum's own evidence was false. ADR-0036's new erratum and Amendment 3 both cited a grep whose result does not reproduce: the command returns twelve hits across three files, not "only hits inside this file". ADR-0041 makes "how it was shown wrong" mandatory and tells a reviewer to re-run it, so a false evidence line is a defect in the exact slot that rule polices. docs/modules/tenancy/README.md was missed entirely while both its siblings were edited, and still posed three settled decisions as Packet 7's open questions — the aggregate boundary, the app.scope carrier, and the tenant_locales single default. It is the per-module authority ADR-0042 links three times, and standards/README.md's edit had already removed the pointer into the third, orphaning it. Two skills would have made an implementer write the wrong thing: seed-tenant put the whole seed in one transaction, which is three aggregate roots against ADR-0042's allow-list of one; and add-architecture-test's migration scan classified one of the two shipped chains while its own Assert.NotEmpty could not see the gap — the silently-green test that skill exists to prevent. The rest is one claim fixed in one carrier and left standing in another: the marker rule's tenant-key clause, the IgnoreQueryFilters allow-list shape, the ITenantContext lifetime adjective, and the [PublicSurface] enumeration's home. ADR-0032 gains a dated Amendment for the Scoped to Transient registration change Packet 5 made, which its body still describes as scoped — true when written, so an amendment and not an erratum. Behaviour unchanged: the code diff is comments only, 943 green, zero skips. ADR: 0032, 0036, 0040 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Sonnet round over Step 1's final state found the deferral pointed at a phase that disowns the mechanism. Four carriers said app.scope's carrier "arrives with authentication in Phase 02b" because the flag derives from the actor's role. Phase 02b says in its own scope that it delivers "only the authentication plumbing" and that User, Membership, Role and Permission land in Phase 03; neither phase document contained the string app.scope at all, so a reader chasing the deferral followed the link and landed nowhere. The owner is Phase 03, with Phase 02b's authenticated principal as the prerequisite, and Phase 03 now says so itself. Two more Accepted ADRs carried statements false when written and were missed by the sweeps that corrected their siblings. ADR-0040's interface sketch still had SetTenantContextAsync issuing app.scope, contradicting its own Amendment 1. ADR-0022's tenant-runtime block calls the SetTenant member that has never existed, treats HostResolution as a nullable scalar, and reads Request.Host directly — which the Packet 4 analyzer now fails the build for; ADR-0036 Amendment 2's carrier list did not reach it. And ADR-0032's new Amendment 3 named Security Standards as a carrier that says "transient". It has zero occurrences of the word, and that section declares itself the authority for session-variable placement only. The previous commit's message called a false evidence line "a defect in the exact slot ADR-0041 polices" and then introduced one; this corrects it. Behaviour unchanged: no C# in the diff, 943 green, zero skips. ADR: 0022, 0032, 0040 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ITenantContext and CapturedContext carried raw Guid / Guid?. Packet 6
created TenantId and OrganizationId as Vogen value objects; this converts
both contracts in one pass, because a half-typed intermediate is a state
where a call site can pass a tenant id where an organization id belongs and
the compiler agrees.
The delicate part is not the type change, which the compiler drives. It is
that every serialization site kept compiling while its meaning moved.
Measured on Vogen 7: an uninitialized id's ToString() returns the literal
"[UNINITIALIZED]", while string interpolation of the same value returns "".
Two spellings of "print this id" disagree, and one of them was on the path
to set_config('app.tenant_id'), where PostgreSQL casts it with ::uuid and
raises 22P02 on the first policy evaluation instead of filtering. So every
emission site now reads .Value under an IsInitialized() gate — the idiom
the UserId branches already used — which is also what keeps the exported
wire format byte-identical: span tags, Serilog properties, Sentry tags, the
local-file JSON envelope and the idempotency fingerprint all still carry a
bare GUID string.
Two of those were unconstrained by any test and are now asserted rather
than assumed: the JSON envelope's shape, and the fail-closed empty string.
The second was mutation-checked — with the guard removed the new case fails
with 'found "[UNINITIALIZED]"', which is the exact fault it exists to catch.
IIdempotencyStore's (Guid, string) key space is ADR-0037's and is not part
of this conversion, so the underlying value crosses that seam once, at a
single site, rather than at each of its five call sites.
944 green, zero skips.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Opus round over 8043840 found one thing that mattered and proved it by mutation: the organization branch of TenantAssertionMiddleware.Mismatch had no test. Replacing its null clause with the natural-looking OrganizationId is { } r && organization != r.Value leaves all 944 tests green while turning an organization asserted against a tenant-wide context from a 404 into a 200 — a header widening a request's scope, which is the one thing ADR-0036 says an assertion may never do. The commit spent six comment lines explaining the semantics and nothing enforced them. TenantWideFixture and its two cases now do; the mutant dies on the 404, and the companion case proves the 404 is not a mis-wired fixture. Five sites read TenantId.Value under IsResolved alone while gating their sibling ids with IsInitialized() two lines below, with no comment saying why the tenant id was different. Two of them carry "must never throw" contracts (the OTel span processor runs inside Activity.Start; a Serilog enricher that throws takes down the line it enriches) and LoggingBehavior.BuildScope runs outside any try of its own. Before the conversion these were Guid reads that could not throw. They are gated now. The unit of work refuses Guid.Empty alongside the uninitialized case, since IsInitialized() only validates the value's shape and the domain already refuses the all-zero id by hand, and it logs an Error when a context claims to be resolved and yields no usable tenant — the empty string keeps that fail-closed, but silence at that boundary is the worst diagnostic there is. The fail-closed test now seeds a session-scoped leftover before asserting, so it constrains what its name says: that the setter overwrites, not that the variable happened to be unset. Three doc samples stopped compiling when ITenantContext.TenantId stopped being a Guid, and Standards 02 gains the rule the whole conversion turns on. 946 green, zero skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Sonnet round mutation-tested the guards the previous commit added and
found most of them uncovered. Stripping the IsInitialized() check from the
span processor, the Serilog enricher, LoggingBehavior and
LocalFileErrorTracker.Unwrap — all four at once — left the whole suite
green. So did deleting the unit of work's != Guid.Empty clause. Of the five
guards that commit claimed to install, only one had a test.
Two production sites were also still ungated, and measurement decided both.
TenantAssertionMiddleware.Mismatch read OrganizationId.Value with only a
null check: on a resolved context whose organization is present but never
assigned, that throws, escapes into UseExceptionHandler, and answers 500 —
replacing the clean fail-closed 404 this middleware exists to produce, on a
pre-auth path, for an attacker-supplied header. And the idempotency
fingerprint still interpolated the UserId wrapper; measured, $"user:{id}"
and "user:" + id.Value are byte-identical for a real id, but for one
nothing assigned interpolation silently yields the literal "user:" while
every sibling component throws — two callers with a corrupted principal
would share a digest and replay each other's response bodies.
TenantId.From(Guid.Empty) does not throw and reports IsInitialized() as
true — these ids declare no Validate, so Vogen checks the value's shape and
not that it names anything. That is why the unit of work refuses the
all-zero id separately, and it now has the test that says so, plus an
assertion that the Error it logs actually fires: that log line is the only
signal that state produces, since the request itself merely reads nothing.
Every new guard was mutation-checked, including the one this commit adds:
each fails under the mutation it exists to catch and passes as shipped.
ADR-0023 gains Amendment 7 for the generator consequence none of this was
foreseen by, and ADR-0032's Amendment 3 records the second stale shape in
the section it already corrects.
955 green, zero skips.
ADR: 0023, 0032
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The second defense-in-depth layer, ahead of the resolver that fills it: every entity marked [TenantOwned] gets a global query filter, and the two entities that must not are asserted rather than assumed. The mechanism is one property on the DbContext, and that is the whole of it. EF compiles a filter into the model once per context type, so anything it closes over that is not reached through the context instance is evaluated then and baked in as a SQL literal. Measured, and not what one would guess: the baked-in failure here is not "the first request's tenant served to the second" — the model is built on first use, reliably before any tenant is resolved, so the literal that bakes in is the all-zero id and every query returns zero rows for the life of the process. Fail-closed, total, and indistinguishable from an empty database. Two_contexts_under_two_tenants_each_see_only_their_own_rows is what holds the property; it fails against the baked-in form, which is why it asserts on the tenant ids the rows carry rather than on counts. Scope is by table class, never by "has a TenantId property". `tenants` is tenant-owned self-keyed — it carries the marker, implements no interface, and gets no filter, because its id is the tenant key and its policy says so. `platform_host_to_tenant` has a TenantId property and carries no marker at all: a tenant-keyed predicate on the table read in order to determine the tenant makes host resolution return zero rows forever, on the anonymous page-load path, with no error anywhere. A marker-gated rule cannot catch a missing marker, so that negative is its own case. Three mechanical consequences, each measured rather than assumed. The internal TenantOwned validation helper in Tenancy.Domain is renamed TenantOwnership, because a non-attribute class of that name makes [TenantOwned] ambiguous under CS1614 in the assembly that needs it most. ModuleDbContextRegistration moves to ActivatorUtilities, since a tenant-scoped context now takes ITenantContext alongside its options. Tenancy.Infrastructure gains a reference to core Infrastructure, where the seam lives: it calls EF model-building APIs, and SharedKernel's EF reference is sanctioned for Vogen-emitted converters only. 959 green, zero skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two findings from the Step 3 review, both measured, both about this step's
own work.
The architecture rules were vacuous. They asserted
filter.ToString().Contains("TenantId") — which the context-side member name
CurrentTenantId satisfies on its own, so the row side of every comparison
went unchecked. Two verifiers independently rebuilt BuildFilter to compare
two context members to each other and watched the whole suite stay green.
The rules now walk the expression tree and collect only member reads whose
target is the lambda's own parameter, so a filter that narrows nothing
cannot pass. That mutation now kills two of the three cases.
And `tenants` had no filter at all. Standards 05 § Table classes mandates
`t.Id == currentTenantId` for the tenant-owned self-keyed class, and four
other corpus locations say the same; the step shipped an assertion
forbidding it instead. Measured: SELECT ... FROM tenants emitted no WHERE
clause, correct only because Row Level Security sits underneath — which is
the one argument this project does not accept for dropping a layer. The
builder now branches on SelfKeyed and filters on Id, which is what the
table's own policy keys on, so filter and policy finally agree.
959 green, zero skips.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TenantScopedDbContext took an injected ITenantContext and held it for life. That contract is registered transient and resolved from the accessor, so the context captured whatever the accessor happened to hold at construction and never moved again. Measured: a context built under tenant A kept filtering to A after the accessor moved to B. Not reachable today — every flow the corpus designs writes the accessor before the context is built, the resolver middleware at scope start and the event transport per delivery — so the snapshot was correct, and correct for a reason that lives in the calling order rather than in the mechanism. Step 5 adds the first component that builds a resolved context from untrusted input; this is cheaper to hold now than to diagnose then. It is also what ADR-0032 Sub-decision 10 already says every cross-cutting reader does. The base now takes ITenantContextAccessor and reads Current on each access. StaticTenantContextAccessor carries the two non-request callers — the design-time factory, where dotnet ef has no tenant, and a test building a context for its model alone. The test asserts the discriminating observation rather than the obvious one. Moving only the accessor leaves app.tenant_id on A, because SET LOCAL is transaction-local and this transaction already issued it, so a following filter narrows to B and intersects RLS's A to nothing. Zero rows is the answer only the following implementation gives; a frozen one still agrees with the policy and hands back tenant A's rows. Reading the emitted SQL cannot tell them apart — both emit the same parameterised text and differ only in the value bound. Mutation-checked against the frozen form. 960 green, zero skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The review round was cut short by a session limit, so these are the findings whose verification I ran by hand. The measured claim in TenantQueryFilters was one of two directions, and not the one production would get. Whichever tenant is current when the model is first built becomes the baked-in literal; in the API host the first build is necessarily inside a request — the module registration refuses to resolve a context outside the ambient transaction — so the literal would be a real tenant's id and every later request would carry it. The all-zero, always-empty direction is what a test or design-time host produces, and it is what this repository measured, which is exactly how a measurement can name the wrong direction confidently. ApplyTenantQueryFilters was public over the bare interface, so an implementer that is not a DbContext could satisfy its shape and defeat the mechanism it exists to hold. It now constrains to DbContext. The sweep also skips owned and TPH/TPT-derived entity types, which EF refuses a filter on — the first module to model either would otherwise have discovered the rule as a startup exception. Two assertions were not asserting. The organization rule enumerated the tenant-owned set, so an entity carrying only [OrganizationScoped] was invisible to both rules; it now enumerates its own marker and requires the pair. And the nullability check read the CLR property type, which IOrganizationScoped already fixes — it asserted the compiler. It now reads the mapped column, and a configuration marking organization_id required fails it, which is the way a tenant-wide row becomes unrepresentable. Three skills still taught the pre-Step-3 mechanism, including the two a new entity and a new module are built through — add-tenant-owned-entity said the filters were Packet 7's to invent, add-backend-module showed a context that does not derive from the seam, and add-ef-migration said both rules "check nothing". The catalogue also carried two contradictory Status lines on the organization rule, one of them mine. 960 green, zero skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The round found Step 3's runtime behaviour correct and its own fix rounds' drift not. Three of the four worst items were in the files a next author reads first. add-backend-module told a scaffolder to write a DbContext that does not compile: its Step 2 reference graph never gained the core Infrastructure edge that Step 4's sample requires, and the comment above that sample still said a module context takes ONE constructor parameter because the registrar uses Activator.CreateInstance — contradicting the two-parameter class three lines below it and the registrar that moved to ActivatorUtilities in this very step. add-tenant-owned-entity said in Step 2 that both architecture rules are implemented and in Step 4 and Validation that nothing catches a missing filter automatically. A skill that contradicts itself inside one file is worse than one uniformly stale: the reader cannot tell which half to trust. The organization term had no test at any layer. Measured: flipping its OR to an AND survived all 960. It decides whether a tenant-wide row is visible to an organization-scoped request, which is the distinction ADR-0017 exists for. The fixture already seeded the three shapes needed, so the case is small and it kills the flip. Two rules gained their reverse directions. An entity implementing ITenantOwned without the marker is filtered at runtime while both scoping rules skip it, so its policy, tenant key and migration go unchecked; and core Infrastructure now has CoreInfrastructure_DoesNotDependOn_AnyModule, the half that keeps the new Module.Infrastructure edge one-way — core is referenced by every module, so a single edge back makes the graph cyclic. The edge itself is now written down: Standards 01 § Dependency Direction has the node, the arrow, a text fallback and the reason it may not be used for a capability of its own, and the Tenancy component diagram names it. 963 green, zero skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The read that happens before any tenant exists, and the middleware that decides which requests need it. CachedHostToTenantResolver opens a short transaction of its own — not a module DbContext, not the ambient IUnitOfWork, both of which refuse to exist before one — announces the host through app.resolving_host, and reads the one row the policy then admits. The announcement must be set_config's function form: SET takes no bind parameter, so the parameterised spelling every other query uses is unavailable, and interpolating a host into SET on the anonymous page-load path would be an injection site. Removing the announcement makes three of the eight resolution cases fail, which is the mechanism being held rather than described. Both flags gate the answer. Active and publicly live are distinct states — the row exists from submission onward, before DNS points anywhere — and reading only one is how a guessed hostname serves an unlaunched tenant's catalog to a stranger. ADR-0036 invalidates this cache on the transaction that flips either flag, which is only meaningful if both feed the answer. The two answers go down two paths. Found ones through ICacheService; unknown ones through a structure capped on its own, because the shared cache is one process-wide pool trimmed oldest-first across every family and a stored null never reads back as a hit there — routing negatives through it would buy eviction and no cache. The split forfeits GetOrSetAsync's single flight, so the resolver re-adds coalescing: one round trip per host however many callers arrive, retired on the flight's termination rather than on a caller's exit, which is the shape Packet 5 convicted in InMemoryCacheService. HostClassificationMiddleware runs over /api/v1 only, before authentication, with the exclusions as a prefix list — a closed list of endpoint literals would 404 the entire Hub contract surface the first time it grew a route. An unknown host gets a bodyless 404 that is byte-identical to the routing 404 for the same path, because anything a caller can tell apart confirms which hostnames exist. The platform branch short-circuits before any database work, and that goes all the way down: the resolver holds Lazy<NpgsqlDataSource>, so constructing it builds nothing. Without that, landing classification broke every Docker-free host suite in the assembly — they run on localhost, which is a platform host, and were paying for a data source they never used. 1003 green, zero skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both were on the anonymous pre-authentication path and both turned the designed bodyless 404 into a 500 plus an unsampled error-tracker capture. A trailing dot walked an IPv4 literal past EffectiveHost's gate. The check sits on the value before the trailing-dot strip, so `1.2.3.4.` reached the strip as a name and left it as a literal — and GetAscii's compatibility mapping folds U+3002 and U+FF0E into a dot after that. Measured: `1.2.3.4.`, `1.2.3.4.:443`, `127.0.0.1.`, `9.`, `2130706433.` and `010.010.010.010.` all came back as accepted hosts and every one then threw in CacheKey.ForHostMapping, which the resolver calls as its first statement. The throw also precedes the negative cache, so repeats never coalesced, and since only a host that reaches the resolver can produce it, the 500 was a positive host-existence oracle against the indistinguishability this step built. The refusal now runs on the produced value. That is the general form and this is its second instance — Amendment 1 already made the same argument for the character set and left the IPv4 check on the input side. ADR-0036 carries the erratum and Amendment 4, because the order it publishes is transcribed, and the shipped code was a faithful transcription of the bug. UnknownHostCache.Trim threw under concurrent Add at the cap. Enumerating a ConcurrentDictionary through LINQ buffers via ICollection.CopyTo after a stale Count read; measured, eight threads adding at the cap threw on 33% of adds, ArgumentException from a concurrent insert and ArgumentNullException from a default slot left by a concurrent removal. Add is unguarded in the resolver and the middleware has no catch. The comment licensing the race said the worst case was overshooting the cap; the worst case was throwing. It now snapshots atomically, sweeps the lapsed entries in the pass it already pays for — nothing else swept, so the map ratcheted to its cap for the process lifetime — and trims to a low-water mark. Re-measured: 0% of 1600 adds throw. Two behaviour fixes alongside them. The key composition fails closed, so the next divergence between the two validators is a rejection rather than an incident. And the cache write moved inside the flight: the flight runs on CancellationToken.None so one caller hanging up does not cancel the lookup others wait on, but with the write in the caller's tail that completed lookup threw its answer away. Three headline mechanisms had no test at all — each was deleted in a mutation and left 1003 green. Coalescing, counted at the physical-connection initializer; the cancelled-caller publish; and the unnamed-host branch the IPv4 blocker escaped through. Plus the pairing property whose absence let the blocker exist: for every input Normalize accepts, ForHostMapping must not throw. Connection-string validation runs at boot again when the key is present, which the Lazy had deferred to the first tenant request. 1026 green, zero skips. ADR: 0036 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Sonnet round measured five mechanisms this step exists to guarantee that survive deletion with 1026 green — including the fix the previous commit shipped. That is the failure Packets 5 and 6 both recorded as their most repeated lesson, a test agreeing with the code instead of constraining it, and it landed hardest on the guard for a blocker one round old. Publish-inside-the-flight had the worst of it. Moving the cache write back to the caller's tail — the exact shape 5c50914 replaced — left all ten cases in HostResolutionTests green, six reruns of six. The case named for the property was false twice over: it cancelled its token BEFORE calling, and InMemoryCacheService.GetAsync throws on a cancelled token as its second statement, so no flight was ever created; then it polled with uncancelled ResolveAsync calls, each of which published the answer itself. It now takes an ACCESS EXCLUSIVE lock on the table so the caller can be cancelled while its flight is provably mid-read — waited for on pg_stat_activity, not on a sleep — and waits on a counting cache decorator rather than by resolving again. Its sibling asserts twelve waiters produce exactly one publish: counting physical connections proves the flight ran once, which is not the same claim. The prefix-matching case diverged from every exclusion prefix at its first character, so it answered true under segment and character matching alike, and all 23 tests passed with StartsWithSegments replaced by StartsWith. `/api/v10` is the path that separates them, and ADR-0024's own versioning plan makes it the one that arrives — under character matching a second major would be swallowed whole by the first's prefix. For the exclusion list the difference is unobservable through this predicate, which the comment now says rather than asserting another vacuous case. The eager connection-string validation was invisible to its own guard file: every case there calls BuildApplicationDataSource, which validates whichever way the composition root behaves. Asserted now on AddLearnStackPersistence itself, with the deliberate absent-key case pinned beside it. The low-water mark, the rejection counter, and the Debug level on the rejected host had no reader at all; the last is load-bearing, since the host is attacker-authored and a bump to Information for observability passed everything. It is asserted against the middleware directly, because Serilog is wired without writeToProviders and a provider in DI receives nothing — measured. Two behaviour changes, both small. A null entry in Tenancy:PlatformHosts was the one spelling that passed: Normalize(null) is null, so `null != null` is false, while "" and " " are both refused. And ADR-0036 now states that the static list beats a mapping row naming the same host — true today, stated nowhere, and the losing row is silent. Also: this ADR's amendments were out of chronological order, because the previous commit inserted Amendment 4 above Amendment 3. Amendment 4's own "every carrier changed" list had omitted the catalogue, which is the mechanism that should have caught it. 1039 green, zero skips. Every guard above was re-measured against a mutation and fails without it. ADR: 0036 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Packet 7 step 5. The matrix ADR-0036 writes as a table becomes a pure function: TenantResolutionAttempt carries the signals AND the answers to the two questions that need a database, so TenantContextFactory.Create is literally the signature the ADR published — no Async, no CancellationToken — and all seventeen rows are drivable from a unit test with no container. The middleware does the I/O; the factory does the decision. Fifteen cases cover the rows, including the ones no request can reach until Phase 02b, because a pure function is reachable as a function and shipping the arithmetic of an authority ceiling with no evidence is how it is wrong when authentication lands. TenantContext's constructor is internal, not private. C# has no friend types, so a private constructor and a top-level TenantContextFactory — the name an Accepted ADR, the glossary and two roadmap lines all carry — are mutually exclusive, and both normative carriers say only "no public constructor". Internal blocks every other assembly because this one has no InternalsVisibleTo, which the rule now asserts: one attribute would hand a whole assembly the constructor. The residual an internal constructor leaves is a caller inside the kernel, and reflection cannot see a `new` expression, so that conjunct is a source scan. Origin joins ITenantContext as a nullable default member. The default is fail-closed only under an allow-list, which is the obligation this hands step 6: `Origin != HostOnly` passes for null and hands an unstated context the run of the API. IOrganizationScopeValidator is the seventh sanctioned setter of app.tenant_id and obeys the rule the four before it obey — its own short transaction, its own connection, as learnstack_app. It ships registered with no reachable caller, and Standards 11 now says so beside the table that lists it: the assertion path the ADR names is subsumed by TenantAssertionMiddleware refusing on any difference before belonging can matter, and its only non-vacuous caller is row 7, which needs a claim. DenyAllTenantMembershipReader is the same honesty in code: rows 7 and 14 fail closed, nothing can reach the call, and both facts are written down so a green suite is not misread as coverage. The first real-database test found the implementation bug: set_config is (text, text, boolean) and a uuid parameter raises 42883 on the first call. Two guards then survived mutation and were fixed rather than recorded — the pooled connection case passed against a session-scoped set_config because Npgsql sends DISCARD ALL on return, which is the driver cleaning up after the bug and exactly what a PgBouncer in transaction pooling does not do. It runs under NoResetOnClose now. Reading organizations by id alone also survives every runtime case, because with the announcement made the policy hides the row either way; that one is caught structurally, which is the only instrument that can see it. ADR-0036 carries an erratum and Amendment 5: its staging table claims Packet 7 makes rows 2, 3, 6, 9 and 10 live, and the paragraph directly beneath it says the authenticated tier is dormant until Phase 02b. Rows 6, 9 and 10 need a claim. The rows this packet makes live are 2, 3 and 13, and it makes 16 reachable for the first time — Packet 4 shipped the assertion comparison against a context that never resolved. 1069 green, zero skips. Four architecture rules and the validator's announcement, composite key, transaction locality and soft-delete clause were each re-measured against a mutation. ADR: 0036, 0040 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five defects, three of them mine to own, and one of those live today. The context carried Kestrel's TraceIdentifier as its correlation id. The contract on ITenantContext says W3C traceparent, CorrelationHeaderMiddleware rejects TraceIdentifier by name for being per-connection and absent from every response, and this middleware is the first writer of the accessor on an HTTP path — so from the previous commit every span, Serilog line and Sentry scope on the two live matrix rows carried a handle correlating with nothing the caller was given, and IntegrationEventEnvelope's ActivityContext.TryParse was armed to throw at the first outbox enqueue. Measured: TryParse is false for a TraceIdentifier and true for the value now written. SetTenant_Callers_Are_The_Enumerated_Four asserted nothing outside a Tenancy folder. The scan was narrowed to paths containing "Tenancy", which deleted the writer that had already shipped — InProcessEventBus, the integration-event handler scope, which ADR-0036 Amendment 2 names as the fourth caller — and meant a fifth writer anywhere else passed green. A rule whose whole job in this packet is the negative cannot be scoped to the folder its positives live in. It scans the tree now and names both writers, and three shipped sentences claiming this middleware was the first of the four are corrected. Two claim shapes had no row and were answered anyway, both too generously: an organization claim with no tenant claim took the claim's organization under the anonymous HostOnly ceiling — row 11's forbidden scope change reached by omitting a field — and a tenant claim with no subject minted ClaimAndMembership, the strongest ceiling, with a null user. One predicate refuses both, and it also earns the two dereferences in the resolver's port calls that would otherwise have thrown the moment Phase 02b populated claims. Row 10 asked membership about an organization the context would not carry: the resolver asked the strictly weaker tenant-level question while the factory granted the host's organization, where ADR-0036 says the context resolves (T, O) iff M covers (T, O). Rows 7 and 14 were self-consistent only because the host names no organization on either, which is why it was invisible. The question and the grant are one expression now, and the parameter that bounds the authority lost its default so "forgot to narrow" stops compiling. Five fail-closed guards survived the whole suite and now do not. The worst was DenyAllTenantMembershipReader returning true: nothing instantiated the type, so the only membership behaviour the corpus exhibited was a permissive double — for a class whose own documentation says it exists so nobody makes the default permissive to unblock a demo. Row 11 passed with its entire organization term deleted, because the membership guard caught it for an unrelated reason that Phase 03 removes. And the pipeline order had no test at all: moving resolution below the assertion comparison restores the unreachable branch that made every Packet 4 comparison vacuous, serving an X-Tenant-Id that names another tenant while only a metric goes quiet. Also: the validator's transaction is READ ONLY, which four documents already called it; my Standards 11 note had split the closed seven-setter table with a blank line, dropping four rows into literal text; the erratum this packet added named an unwritten delivery record as a changed carrier and omitted row 1; and "the resolver runs before routing has selected an endpoint" was false — measured, routing has run, and the true reason no module name is carried is that resolution must not vary by route. 1078 green, zero skips. Every guard above was re-measured against a mutation. ADR: 0036 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Sonnet round found no behavioural defect in reachable code. What it found was an overclaim I introduced two commits ago and three guards no test constrained. Amendment 5 said "the factory implements all seventeen and Packet 7 tests them as a pure function" — four lines after saying the factory's suite does not cover row 1. It decides twelve: the rows expressible as a TenantResolutionAttempt, 2, 3 and 6-15. Row 1 is host classification's, rows 4 and 5 belong to an authentication outcome nothing implements, row 16 to TenantAssertionMiddleware and row 17 to EventTenantContext.FromEnvelope. The sentence mattered more than an ordinary slip because that amendment's whole subject is honesty about what a green suite proves, and because it is what a Phase 02b author would read when deciding whether those four rows already have a home. Corrected in place — it is this branch's own dated text, unmerged and unreviewed — along with the same claim in the factory's own doc and in the glossary entry, which is where it had spread to. TenantContextBehavior's remarks said this behavior "adds a second rejection here" for an origin exceeding what the request type permits. Nothing anywhere reads Origin: grep over Application and Api returns nothing, Handle checks only IsResolved, and the file's own TODO forty lines below still asks for the discriminator. So the file asserted that ADR-0036's authority ceiling — the single control that makes a forged host harmless — was already mechanical. It is Step 6's, and the remark now says so, including that the check must be an allow-list because Origin is a nullable default member. SetTenant_Callers_Are_The_Enumerated_Four had a latent false positive of its own: whitespace is stripped before the search, so ".Current =" becomes ".Current=", which is a substring of ".Current == null" — the idiom tracing code uses, in the very middleware this rule is about, which now reads Activity.Current. Reproduced: a planted equality check failed the rule with an "unauthorized writer" message. The needle is narrowed, which is what the rule's own note prescribes; the folder is not, which is what broke it last round. Three guards no test constrained. The validator's uninitialized/all-zero refusal, whose comment promises the answer is "no" rather than "no, by accident" — deleting it left all five Docker cases green, and an uninitialized Vogen id reaching .Value is a 500 where a documented false was claimed. The two ClaimAgreesWithHost conjuncts, load-bearing not for the factory (which refuses row 11 on its own standalone check) but for the port economy the middleware reads them for. And SET TRANSACTION READ ONLY, which four documents already called this transaction. The read-only guard is worth naming, because the first test of it was the mistake this packet keeps making: it reproduced the statement sequence on its own connection, so it proved what PostgreSQL does and not what the validator does, and the production statement still survived deletion. It now has two legs — a scan that the file issues it and issues it before the announcement, and the Docker case that the statement has the effect claimed — and the test says out loud which half it is. 1084 green, zero skips. Each guard was re-measured against a mutation, and the read-only pair against two. ADR: 0036 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Packet 7 step 6. ADR-0036's ceiling stops being a decision and becomes a mechanism: a tenant context assembled from the host alone reaches only request types marked [PublicSurface], which is what makes a forged Host harmless — it reaches exactly the pages that hostname already serves to anyone who types it. Two gates, nested rather than sequential, and the shape is the whole design. Gate 1 asserts a context exists and returns either way; [AllowsUnresolvedTenantContext] exempts a request from it and from nothing else. Gate 2 is an allow-list over stated origins. Three plausible shapes are wrong in three different directions and each now fails a test. A negation — `Origin != HostOnly` — passes for null, and Origin is a nullable default interface member, so it would hand every context that never considered its authority the run of the API. Fusing the gates so the marker skips both lets an anonymous caller reach a provisioning command by typing a live tenant's hostname. Sequencing them so an unresolved context falls into the ceiling 404s precisely the rows 13 and 15 requests the marker exists to admit, because an unresolved context states no origin and the allow-list is fail-closed on null. Ambient is on the list and that is not a judgement call: EventTenantContext resolves with exactly that origin and InProcessEventBus has written it into handler scopes since Packet 5, so omitting it stops every integration-event consumer. The ceiling's refusal reuses TenantContextFactory.Refused rather than minting a second lockey_not_found. The two refusals a caller can provoke — this one, and an unresolvable host — must be byte-identical, and sharing the Error makes that a compile-time fact. It is asserted at the wire anyway, on the same path, on the raw body with only the correlation id normalized: the two responses are written by different writers, one by UseStatusCodePages and one by MVC, and a media-type spelling once made two 404s tellable apart without reading a body. TestResolvedTenantContext went red the moment the ceiling landed, which is the gate working — a resolved context that never decided its authority must reach nothing. It states HostAndClaim now. That failure is worth naming because the tempting fix is to loosen the gate, and the suite could not have told the correct implementation from the forbidden one without A_Resolved_Context_That_States_No_Origin_Reaches_Nothing. Both markers ship with no users — there is not one production request type in the solution — and all three catalogue entries say so rather than reading as coverage. PublicSurface_Requests_Are_Never_ReadSensitive is the sharpest case: its catalogued instrument was an audit-catalogue cross-check against a catalogue that arrives in Packet 9, so its Type field is corrected and it lands as set-emptiness, which turns red the day a marked type appears before the cross-check exists. What is NOT vacuous is the reverse direction — Standards 04's table may not name a type that carries no marker — and the attributes' own AttributeUsage, since reading a marker with inherit: false against Inherited = true is a silent mismatch rather than an error. Tenant_Scope_Widening_Is_Never_Set_From_Request_Input stays Registered: nothing sets app.scope, and the catalogue already says the rule becomes non-vacuous in Phase 03. 1100 green, zero skips. Four gate shapes, two parity mutations and three rule mutations were each measured. ADR: 0036 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Step 6 review found the counting apparatus scoped to the wrong place, and it is the same failure the previous step's review found one commit earlier: a rule whose whole job is the negative cannot be scoped to where its positives happen to live. RequestSurfaceTests listed nine assemblies. Measured: an identically marked request type fails all three rules from LearnStack.Modules.Tenancy.Application and passes all three from LearnStack.Modules.Tenancy.Application.Contracts — which is where add-mediatr-handler tells an author to put a command record, and therefore where ProvisionTenantCommand, the first type to carry either marker, is scheduled to land in step 9. TenantContextBehavior reads both markers off typeof(TRequest) and knows nothing about assembly lists, so the pipeline would have granted the widest surface it can grant while the rule that exists to count that grant reported clean. The sweep is derived from the tree now, loads every project without a null filter, and has a leg asserting its own completeness — a narrowed sweep does not fail, it passes over less code. Two MediatR shapes were invisible to every rule and run with no pipeline at all. Measured against 12.4.1: IStreamRequest<T> has no interfaces and is not assignable to IBaseRequest, so the request filter never saw it; and typeof(IRequestHandler<>) has no interfaces either — the void handler does not derive from IRequestHandler<T, Unit> — while Unit does not implement IResultBase, which every behavior here is constrained on. So a stream or a void request reaches its handler with no authority ceiling, no validation, no audit classification and no TransactionBehavior, hence no SET LOCAL app.tenant_id. Row Level Security keeps EF reads fail-closed; what is exposed is every effect that is not an EF read. Requests_Are_Never_Streamed bans the request shape and Handlers_Return_Result now rejects both handler shapes. Handlers_Return_Result was also dropping assemblies it could not load, which turns "I could not read this code" into "this code is clean". It loads them. One test isolation defect, mine: AuthorityCeilingHttpTests refuses a host in its parity case, and learnstack_host_classification_rejected_total is process-wide, so a MeterListener asserting an exact count raced it. Green alone, green paired, red in a full parallel run — the shape of a race. The two suites share a collection now rather than the count being loosened to "at least one", which would have kept the suite green by asserting less. Four consecutive full runs green. 1105 green, zero skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two review rounds of this step reached opposite conclusions about the same measurement, so I reproduced it against the shipped pipeline. Both were partly right and the disagreement is itself the finding: the corpus states the invariant more broadly than any mechanism here can deliver, which is why three of ten independent lens runs flagged it. Measured. PUT, DELETE and OPTIONS on a routed path answer 405 on a host the resolver mapped and the shared 404 on one it did not — same path, tellable apart. Routing runs ahead of every user middleware, so nothing in the pipeline sees the request. Not fixed by rewriting 405 to 404, and the reason is the flag's definition. The only hosts that reach routing are ones the resolver admitted, and it admits a row only under `is_active AND is_publicly_live` — which ADR-0036 defines as DNS pointing at LearnStack and the tenant's public site being served. A host without the flag resolves to nothing and answers the unknown-host 404 here too. So the disclosed bit is exactly the one that is public by definition, and once Phase 02d ships the first [PublicSurface] page a plain GET discloses it more directly by returning 200. Rewriting would cost every legitimate client its 405/415 diagnostics to hide something a GET hands out. What the invariant does cover is now stated separately from what it does not, and pinned: on the paths the ceiling controls, a live tenant host and an unknown one are byte-identical, and nothing anywhere names which tenant. A disallowed method on an unrouted path is identical on both — the shape an attacker probing for hostnames would actually use — and that half is asserted. The measurement also turned up something neither round reported: a platform host is separable from both by `code` rather than status. An unmarked request there resolves no tenant and fails gate 1 with tenant_mismatch, where a tenant host under the ceiling and an unknown host both carry not_found. That discloses membership of Tenancy:PlatformHosts — a short static list an operator publishes as its own entry point — and nothing about any tenant. Recorded rather than changed. One real gap closed: the request filter had no positive case. Every set the four rules produce is empty today, so deleting the IStreamRequest arm changed nothing any of them asserted — a detector nobody had run. It is driven against local types now, including the measurement it exists for, and deleting the arm fails. 1106 green, zero skips, two consecutive full runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Packet 7 step 7. EnterAsync opens a learnstack_platform connection and transaction of its own and hands back a handle; disposing without committing rolls back. A second connection, never SET ROLE — ADR-0003 gives three reasons and each rules the alternative out alone: membership would make BYPASSRLS a standing capability of the application role, a plain SET ROLE survives COMMIT and rides a transaction-pooled connection into the next tenant's request, and per-role settings like statement_timeout are applied at login and do not follow a role switch. The corpus disagreed with itself about the shape and this resolves it rather than propagating it. Two editable carriers said the scope opens "a DI scope whose DbContext is built on that data source"; ADR-0003 and architecture/09, both Accepted, say "a second connection". The connection is also the only shape that compiles against what is already here — every module DbContext is bound to IUnitOfWork.Connection, which comes from the application data source the composition root guards by name — and it forecloses nothing, since EF can be built on the handle. Standards 05 and the glossary are corrected. The platform data source gets the INVERSE of the application guard. Reusing the application builder would have made the credential refuse its own first connection, because that builder rejects any role reaching BYPASSRLS and this role IS that role. The initializer here asserts the opposite, and the failure it catches is the one that looks like nothing at all: a learnstack_platform that lost the attribute still connects, and every cross-tenant query simply returns fewer rows. The entry gate ships refusing everyone, following DenyAllTenantMembershipReader exactly. There is no principal until Phase 02b and no permission until Phase 03, so "nobody holds a platform-scope permission" is the true answer rather than a placeholder, and it makes ADR-0036's "checked before the scope opens" a call rather than a sentence. Worth naming ahead of time: Packet 9's GDPR handler is the first real caller and inherits a closed gate. What ships is a log line, not an audit trail. Warning level, the reason and the call site, and deliberately no tenant id — TenantId leaves the platform sentinel unfixed and Packet 9 chooses it with the schema that stores it. The line sits between the connection open and the transaction begin because that is the position Packet 9's SecurityEvent row takes over, and it must be written before the operation runs so an operation that later fails is still recorded. The corpus calls this path audited; this packet does not, because it is not yet. The Docker suite is written as provenance rather than isolation. A bypass-role test asserting "I see both tenants" passes identically against an inert policy set, which is why CLAUDE.md forbids the shape; each case asserts instead that the same query on an application connection sees less, at the same moment, on the same data. Two measured gaps closed while building. The suite first created its own data source, so BuildPlatformDataSource never ran and swapping the inverted initializer for the application one left all six cases green — the guard had no test. It routes through the builder now, and a case takes BYPASSRLS off the real role to prove the guard fires, restoring it in a finally. And leg 2 of the resolution rule scanned for the word "PlatformAdmin", which matched the type names — a rule matching its own vocabulary rather than a credential. 1123 green, zero skips. Nine mutations measured: the gate, its ordering, the rollback, both data-source guards, and each of the four rules. ADR: 0003, 0036 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three real defects, all in the parts a later step inherits rather than the parts this step proves, and all measured. A COMMIT that faults stranded the one BYPASSRLS connection in the process. The handle marked itself committed AFTER the await, so a faulted commit left the flag false, disposal issued ROLLBACK on a transaction already over, that threw, and both disposals were skipped — measured, a pool of three exhausted after three entries, with the bookkeeping exception replacing the caller's real one. NpgsqlUnitOfWork states the rule twenty files away: resolve before the await, dispose in a finally, and leave a faulted commit Indeterminate rather than attempting to undo an outcome nobody knows. This now does the same. A resolved handle stayed usable. Measured: after a successful commit the connection was still open, a plain SELECT on it returned every tenant's rows in autocommit, and a write issued there survived DisposeAsync — the exact opposite of what the type's own doc promised. Both accessors are fenced; fencing Transaction alone would have left the autocommit hole open. The gate-uniqueness rule scanned only the assembly declaring the interface, so a permissive gate in Infrastructure or Api — precisely the "registered elsewhere for a demo" its own message names — was invisible. My mutation of it passed because I planted the probe in SharedKernel. It sweeps every production assembly now, and a probe in either of the two places it was blind to fails. Two coverage gaps closed. Gutting CommitAsync to a no-op left the whole suite green, because every case only ever checked that things did NOT survive; and deleting the absent-credential guard left its test green, because control fell through to a second message that also names the key — so the assertion is on text unique to the branch now. I also added a capability that already existed. PostgresFixture has ExecuteAsSuperuserAsync, whose own remarks name ALTER ROLE ... BYPASSRLS as the case it exists for; I added a raw superuser connection string beside it, which is strictly wider. Removed, and the one caller routes through the helper. Four documentation claims corrected, three of them written by this step. The Docker suite's class doc named DROP POLICY as its falsifier — measured, that leaves every case green, because FORCE RLS with no policy is default-deny and dropping it makes the application side see LESS; the mutations that do falsify are DISABLE ROW LEVEL SECURITY and a second permissive policy, the ADR-0003 Amendment 3 defect. Standards 05 still offered the audit row as a live mitigation four paragraphs below the section this step rewrote. The composition root's class remark still called the key it now reads "deliberately absent". And the catalogue carried two Notes assigning "live" to opposite conjuncts, one written when the rule was Registered and one when it landed. The fourth rule this step shipped had no catalogue row at all, so its canonical name lived only in the test file. One thing stated rather than fixed: with the rollback now guarded, the commit ordering is no longer independently observable — no test here fails if it is reverted, and none pretends to. What it still buys is the semantics. 1126 green, zero skips. Six mutations measured across the two fixes and the widened sweep. ADR: 0003, 0033, 0036, 0040 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two majors, both about what a later step inherits rather than what this step
proves.
The keyed-resolution boundary had a hole an ordinary idiom walks through.
GetConnectionString(name) is sugar for configuration[$"ConnectionStrings:
{name}"], so the indexer form reads the identical value and carries none of the
needle the rule scans for — and it is not a contrived evasion: the idiom is
already used four times in this solution, for Telemetry:* and Deployment:Mode.
A contributor reaching for the familiar pattern would have tripped none of the
four rules, could have built a raw NpgsqlDataSourceBuilder with no
RequireBypassRole initializer so even a wrong-role credential passed silently,
and would have skipped the gate, the reason and the log line. A fifth leg
catches it, asserted separately so a failure names the idiom.
The corpus's only worked example of this scope would have raised 42501. The
GDPR redaction handler in architecture/31 issues its UPDATE on the injected
AuditDbContext, which ADR-0040 binds to IUnitOfWork.Connection and therefore
keeps on the request's learnstack_app connection whatever scope surrounds it —
while the comment directly above it says the redaction runs as
learnstack_platform, and the same document revokes UPDATE on audit_log from
learnstack_app. It was wrong before this step and this step made it reachable,
since Packet 9 is the first real caller and will copy it. It runs on the
handle now, commits explicitly, and the inbox write moved outside the scope so
nothing reads as though it rides the cross-tenant transaction.
Two paths had no test, and one of them was added by the previous fix round. The
guarded rollback on an abandoned handle whose connection is already dead — its
catch, its filter and its swallow — was unreachable, because the only case that
kills a backend does so after the handle is resolved and the only case that
abandons one rolls back a healthy connection; narrowing the filter to an
unreachable type left all 1126 green. And the Warning line, which is this
packet's entire record of a cross-tenant bypass until Packet 9, was observed by
nothing: both suites passed NullLogger, so demoting it, renumbering it, or
gutting the path shortener each passed.
Writing that second test found a real defect. The [Caller*] attributes were on
the interface only, and C# fills them from the static type of the receiver — so
every caller holding the concrete PlatformAdminScope, which is every test that
constructs it, logged <unknown> at <unknown>:0. Losing the provenance silently
is exactly what the line exists to prevent. They are on the implementation now.
And one of my own assertions was too specific: NotContain("/Users/") missed the
mutation that keeps every path segment, because Split drops the leading slash
and the result reads "Users/cemililik/...". It asserts the shape — at most two
segments — which is the property the code actually claims.
1128 green, zero skips. Five mutations measured, including the two the previous
round left uncovered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Packet 7 step 8. With app.tenant_id unset every Row Level Security predicate is NULL, so a tenant-owned read returns zero rows and a write is refused — fail-closed already, and quiet. The symptom reaching an operator is missing data, not a fault, and missing data gets investigated as a bug in the feature. TenantContextGuardInterceptor turns that silence into a TenantContextMissingException, and the first test asserts the silence itself so the guard's value is evidence rather than assertion. Keyed on the transaction, not on the table. Some corpus sentences describe it as guarding [TenantOwned] tables; the catalogued rule's own name and the packet plan describe the transaction. Matching table names would put a parser between every query and the database, wrong on the first CTE, to decide something the transaction already answers — every command from a module context belongs to a request that had a tenant to announce. Both standards are narrowed to what shipped. The exemption list is empty, and that is a property rather than an oversight. EF interception sees only commands EF issues, so the set_config pair the setter sends needs no self-exemption, and CachedHostToTenantResolver, OrganizationScopeValidator and PlatformAdminScope are invisible by construction. That last one matters most: it is a BYPASSRLS connection that announces no tenant by design, and a hand-written exemption for it is an exemption someone later widens. Only one of the seven sanctioned setters marks anything, and the catalogue says so rather than implying seven. Four do not exist in code yet; two issue raw NpgsqlCommands. The marker is read through a new IUnitOfWork member, which owes ADR-0040 Amendment 5 — that ADR enumerates the seam member by member, and its Amendment 1 exists because an earlier addition was nearly left silent. It takes the command's transaction rather than returning a flag because the reference check is load-bearing: measured, a pooled data source hands back the same NpgsqlTransaction instance across cycles, so a bare flag would vouch for a later transaction on an earlier one's announcement. One correction only Packet 7 could reveal. TenantContextMissingException carried lockey_tenant_mismatch, which maps to 404 — so the first wiring bug to trip this guard would have reached a client byte-identical to the deliberate refusal an unresolvable host gets, the one response Steps 4 through 6 spent three rounds making indistinguishable on purpose. A server fault hiding inside the anti-oracle 404 is invisible in monitoring. It carries internal_error now. Nothing threw the exception before, so nothing else changes. The blast radius was zero: every EF command in the existing suite already runs on an announced transaction. Which also meant nothing exercised the throw, so eight cases were written for it — and two of them exist because the first pass did not cover what it claimed. Commenting the guard out of the three synchronous overrides left everything green, because EF does not route the blocking APIs through the async ones; and dropping the ReferenceEquals check left everything green, because the reset at BEGIN hides it in every sequential case. Also corrected: a comment claiming UseApplicationServiceProvider is what lets a DI-registered interceptor be found. Measured false on EF Core 10 for both interceptor kinds; an interceptor reaches a context through AddInterceptors, which is the line now beside it. Stated rather than fixed: the flag is set after the round trip so a failed announcement vouches for nothing, and that ordering is not independently observable — making the announcement fail means breaking the connection under it, and the unit's disposal then throws before any assertion is reached. 1136 green, zero skips. Five mutations measured. ADR: 0040 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The review found no wrong result reaching a caller. It found a coverage ring around correct code, and one sentence of mine that would have misled the next implementer. The miscount is the part that mattered. I wrote that of the seven sanctioned setters "the two that do [exist] — CachedHostToTenantResolver and IOrganizationScopeValidator — issue raw NpgsqlCommands". CachedHostToTenantResolver is not one of the seven at all: it sets app.resolving_host, and the closed table forty lines above says so. What my sentence displaced is "the integration-event transport, per delivery — ambient, it opens it" — the one other setter that OPENS the ambient transaction and must therefore announce it. A Phase 02b implementer reading that is told the remaining in-code setters are exempt because they use raw commands, ships a transport that opens the transaction without announcing, and every module command in every event handler throws. That is the miscount ADR-0040 Amendment 3 already fixed once, re-entering through the document this packet names as the placement authority. Corrected in both carriers; ADR-0040 needs nothing, its own list was right. Three arms of the guard were asserted by nothing. Deleting Guard from BOTH Scalar overrides left 1136 green — a third of the surface, in a file whose own comment claimed the mutation standard had been applied to every arm. And replacing both NonQuery bodies with an unconditional throw ALSO left everything green, because the only let-through assertion was a reader: a guard that refused every write would have passed. Both directions are pinned now, on all three pairs. The `transaction is not null` term was unconstrained. After a commit _transaction is null and nothing clears the flag there, so without the term ReferenceEquals(null, null) would make the unit vouch for any command carrying no transaction at all. The existing null assertion ran while a transaction was live, where the reference check alone already answers false — it constrained the wrong half. And a comment named the one shape its body does not run: it reasoned about a SaveChanges INSERT arriving on ReaderExecuting while the case underneath issued raw SQL on the NonQuery arm. Writing the case it described turned up something Step 9 needs: EF wraps a failing SaveChanges in DbUpdateException, so an assertion written as a bare ThrowAsync<TenantContextMissingException> fails on the path the first real handler will take. Pinned, so the next step writes the right assertion rather than discovering the wrapper. 1146 green, zero skips. Three mutations measured. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three findings, no production code, and one number of mine that was wrong.
The synchronous let-through arm was asserted by nothing. Replacing all three
blocking override bodies with an unconditional throw left every guard case
green — so a guard that refused every blocking call would have shipped. This is
the same defect class the previous round's own commit message says it closed
("a guard that refused everything would have passed"): that round fixed the
refusal direction on all three pairs and the let-through direction on the async
half only. Both directions are now asserted on all six overrides.
The exception's code was pinned by nothing. Reverting lockey_internal_error to
lockey_tenant_mismatch left the entire suite green — the 404-to-500
reclassification that is the headline of the previous commit had no test at
all. Asserted on the CODE and not only the status, because internal_error falls
into HttpStatusMap's default rather than an explicit arm, so a mistyped key
would still yield 500 and go unnoticed while `code` is what an RFC 7807 client
matches on.
ADR-0040 Amendment 5 wrote the check with a term missing. It gives
`ReferenceEquals(transaction, _transaction) && _tenantContextIssued`; the code
that shipped in the same commit has `transaction is not null` first, and that
term is load-bearing — remove it and exactly one case fails, because after a
commit _transaction is null and ReferenceEquals(null, null) would vouch for any
command carrying no transaction at all. The sentence never described the code,
so it was false when it entered the record rather than having aged: an
ADR-0041 inline erratum, disclosed by Amendment 6.
And the count. The previous commit message says 1146; the tree at that commit
had 1138. The error looks like the two rounds' new-test counts being summed
rather than the second applied on top of the first. Commit messages are
history and are not rewritten, so it is corrected here and the Packet 7
delivery record will cite the measured total rather than a computed one.
1139 green, zero skips, measured rather than derived. Three mutations killed.
ADR: 0040
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 9's crux, measured rather than reasoned. The policy on `tenants` is WITH CHECK (id = app.tenant_id), so creating a tenant means announcing the tenant being created — an id that names nothing resolvable, because it does not exist yet. Against a live PostgreSQL with the shipped policy transcribed verbatim: with app.tenant_id unset the INSERT fails 42501; with it set to the empty string, which is exactly what the unit of work writes for an unresolved context, it fails 42501 identically; with it set to the new tenant's own id the whole sequence — INSERT tenants, INSERT organizations, UPDATE the back reference — commits. A re-announce inside the transaction also works, and the value does not survive COMMIT, so nothing rides a pooled connection. That settles the collision Step 7's design pass flagged and left open, and it settles it against the corpus's standing sentence: learnstack_app provisions on its own and PlatformAdminScope is not needed. Which is fortunate, because Step 7 shipped a gate that refuses everyone, its handle hands back a raw DbConnection rather than a DbContext, and it opens a second connection — putting the two aggregate writes outside the single commit point ADR-0042 exists to guarantee. The announcement rides on the request, not on the handler. TransactionBehavior reads IProvisionsTenant and announces once. A handler announcing a second time would leave a window inside the ambient transaction where app.tenant_id is the empty string and every statement in it is silently fail-closed, and would hand every handler in the solution the ability to move the ambient tenant. This way TransactionBehavior stays the only caller, so ADR-0040's setter set is still closed at seven — the same setter announcing a different value for one request shape, not an eighth. The `!IsResolved` term is the load-bearing half. Without it a caller already authenticated for tenant A could send a provisioning request naming tenant B and announce B. With it, such a request falls through to the ordinary path, the transaction is announced with A, and B's insert is refused by the policy — the confused deputy closed by the database rather than by a check someone has to remember. Every misuse of the new setter throws rather than degrading: no transaction, a joiner, an already-announced transaction, an uninitialized or all-zero id. The joiner case is a throw and not the ambient setter's silent early return on purpose — a joiner that believed it announced surfaces as 42501 three frames away, on an INSERT that reads as a permissions problem. Also here because the handler cannot compile without it: IAggregateWriteStore, the first persistence abstraction in the solution. Application -> Infrastructure is a forbidden edge, every DbContext lives in Infrastructure, and Infrastructure already references Application, so the reverse is a cycle. Typed rather than named so the cross-aggregate rule can count it; write-only, because a provisioning transaction announces a tenant that does not exist yet and no read works inside it. 1139 green, zero skips. The aggregate promotion and the command follow. ADR: 0040, 0042 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Packet 6 shipped four types with public factories, top-level DbSets and no navigation from Tenant, which is not the containment the module spec's ERD describes — and the two halves do not resolve the same way. Promotion: TenantDomain and TenantSetting become roots, one token each, because a surrogate Vogen id, an AuditableEntity base, a row_version and their own RLS policy are all already shipped. Containment: TenantLocale and TenantFeatureFlag become navigations, because a composite natural key with no surrogate id is not an IAggregateRoot<TId> under any reading. Tenancy has four roots. The row_version bump the roadmap requires for a locale or flag write is a consequence of that containment and not a second mechanism. Version advances only inside AuditableEntity.Touch, reached only from MarkUpdated — so routing every child write through a root method bumps it, and a caller holding a locale directly could not have produced one. The two factories are internal now and the DbSets are gone: a detached child write through DbSet<TenantLocale> would have left Tenant.row_version where it was, silently, because the root would never be tracked. Not owned types, and the reason is mechanical rather than stylistic. An owned mapping is the natural way to say "part of the root" and would silently remove both query filters — the filter builder skips entityType.IsOwned() — so the rows would lose the EF half of the four-layer isolation and the correspondence test would fail looking for a type no longer in the model on its own. The single-default invariant lands as a partial unique index AND an aggregate guard, because they answer different questions. The guard produces a readable error for a caller that asks twice in one unit of work; two transactions each promoting a different locale both pass it, neither able to see the other's uncommitted row, and one of them has to lose at the database. Both directions are pinned, including the partiality: measured, an unfiltered unique index on tenant_id passes the second-default case and silently allows only ONE LOCALE per tenant, which is not the invariant. Clear-then-set is not cosmetic either. EF emits one UPDATE per changed row, so a swap is two statements, and measured against the index the new-first order fails 23505 while old-first succeeds. The aggregate does it in that order and a test asserts both halves. The scaffolder laid the trap the design pass predicted: mapping the two navigations introduced the first relationships into a model that had none, so `dotnet ef migrations add` also emitted AddForeignKey for two constraints that already exist as raw SQL in the first tenancy migration, invisible to the snapshot. Deleted by hand — that failure lands at `make migrate` against an existing database, not at build, so a green local suite would have proved nothing. Twelve existing tests called the two factories directly. They go through the root now, which is the containment working rather than a cost: the guards are unchanged and run inside the same factories. 1142 green, zero skips, measured. Four mutations killed. ADR: 0042 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ADR-0042 sanctions exactly one operation to write two aggregate roots on one transaction, and this is it: the command, its handler, its validator, the two write ports, and the architecture rule that counts them. The rule counts PORT TYPES rather than names. The catalogue registered it as a scan for DbSet use in handlers, which under the shipped dependency rules can never fire — Application may not reference Infrastructure, so no handler can name a DbSet at all. A rule at Implemented status that cannot fire claims coverage the suite does not have. The validator shares the aggregates' guards rather than skipping them. Leaving slug shape and the mapped widths to the factories alone was measured wrong: ArgumentException has no entry in HttpStatusMap, so a mistyped slug became a 500 — raised after ValidationBehavior passed the command, after the transaction was opened, and after the tenant was announced. The shape and the two numbers are declared once in the domain and read by both layers, and Cascade(Stop) keeps the regex off a null a deserializer could supply. Two guards had no test until the mutation round said so. The ordinary SetTenantContextAsync survived being made session-scoped against all 292 integration cases, because Npgsql's DISCARD ALL cleans up after the bug — which a PgBouncer in transaction-pooling mode does not. Both setters now have a case that suppresses the reset and holds the pool at one. Module: Tenancy ADR: 0042, 0040, 0003 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ADR-0042 § Implementation Notes specified the cross-aggregate rule as a source scan for DbSet writes. That scan could never fire, and not because anything changed since: Application may not reference Infrastructure, so no handler can name a DbSet at all. An inline erratum marks the sentence and Amendment 1 records what ships — a reflection rule counting IAggregateWriteStore ports, which also survives a rename. The catalogue row moves to Implemented with the three mutations that turn it red. The module spec's sequence diagram had the handler opening the transaction and setting the context. It does neither, and the distinction is load-bearing: a BeginTransactionAsync from a handler is a joiner rather than a boundary, and an announcement from a handler would be an eighth setter of app.tenant_id against a set two ADRs close at seven. The diagram also carried `SET LOCAL app.tenant_id = <id>`, which is not valid SQL. The two matrices said the module's operations do not exist yet. Two of them do now, so permissions.md records why provisioning is deliberately unauthorized — unresolved context by construction, SystemActor attribution, no HTTP endpoint — and audit.md records that it is unaudited until Packet 9, with the line it will be written on. Module: Tenancy ADR: 0042 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 2 of Step 10's review. Both findings were in round 1's own fixes. Idempotency-by-conflict keyed on the top-level error code, which was a safe proxy while provisioning was the only command: every cause of business_rule_violation really was "this row exists". MapHostToTenantCommand broke that in the same round — it returns the same code for a host already taken, an organization that is not this tenant's, and a host the deployment reserved — and only the first means there is nothing to do. So a wrong organization id in SeedData, the plausible copy-paste between two tenants declared side by side, made the seeder log "already present", exit 0, and never write the row that decides whose data an anonymous request sees. The classification now reads the field-level reason. That branch was unreachable from a test because the runner read SeedData directly, which is how the defect survived a round; RunAsync now takes the tenants, and SeedComposition takes the reserved hosts. The cache invalidation ran before the commit, not after, so its comment claimed a guarantee the placement does not give: TransactionBehavior commits after the handler returns, and a request arriving in between still misses the uncommitted row and re-caches the miss. What the call does guarantee is the case that actually happens — the request after the write. Closing the rest needs a post-commit seam on IUnitOfWork, whose surface ADR-0040 governs, so it is an amendment rather than an edit and it is named with the obligation that will owe it. Also: the local-dev-setup skill told the reader `make seed` does not apply migrations, in the same commit that made it depend on migrate. Module: Tenancy ADR: 0036, 0040 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 11. Packet 6 shipped all five against the schema, driven with set_config — statements about the migration and its policies. These drive the same five through HostClassificationMiddleware, TenantResolverMiddleware, TenantContextBehavior, TransactionBehavior's announcement and the EF query filters, which is the path a browser takes. A policy that holds under set_config and a resolver that never sets it would pass the first suite and fail every real request; only this one can tell them apart. Nothing here stubs ITenantContext. Every other HTTP fixture replaces it with a header-driven double, which is right for their subjects and fatal for this one: the tenant a request gets is the thing under test. The host header is the only input, and the data is what SeedRunner wrote, so these cases also answer whether the seed actually serves a request. Two design facts the framework taught rather than the plan anticipating them. A TenancyDbContext injected into a controller is refused at resolution — it would read zero rows from every tenant-owned table, silently — so the probe goes through ISender and the pipeline opens the transaction that announces the tenant. And the read query needs [PublicSurface]: a request carrying only a host resolves HostOnly, and the authority ceiling admits that origin for marked types alone. Without it every read returned 200 with an empty body, which is the ceiling working. That also settles the write case: an anonymous host-only request cannot create an organization at all, so the foreign tenant id in the body is refused twice over rather than once. The suite constrains the composite outcome, not any single layer, and says so: deleting both query filters leaves all five green because RLS alone holds. The filters are not unconstrained — the same mutation turns three other tests red — and the remark names them, so the next reader does not mistake defense in depth for a gap. Also swept the repository's Packet 7 comments: eight sites across src and tests claimed in the future tense what this packet has now shipped. Module: Tenancy ADR: 0003, 0036, 0040 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 1 of Step 11's review, and the finding was that three of the five cases tested something other than their name — one of them nothing at all. Write_With_Foreign_TenantId_Is_Rejected_By_WithCheck performed no INSERT. It asserted that an anonymous POST failed, which a 404 satisfies, so it passed against a DELETED endpoint and against a database with every policy dropped. It now issues the INSERT on the ambient connection, naming a tenant other than the announced one, and asserts the 42501 that WITH CHECK raises. Raw SQL deliberately: a write through EF carries the filter's tenant and could never name a foreign one. Org_X_cannot_read_Org_Y read `organizations`, which is the tenant-WIDE class — within a tenant every organization is visible to every other, by design — and the narrowing came from a Where the probe handler wrote itself. It now reads `tenant_settings`, the organization-scoped class, and narrows nothing. TenantWide_Row_Of_TenantB_Is_Invisible_To_TenantA read `platform_host_to_tenant`, which is platform-scoped and whose policy has no organization term, so it named the wrong mechanism entirely. It now reads the row shape the superseded template actually leaked: tenant-owned, organization_id IS NULL. Unsetting_tenant_context never reached a handler or read a table — it duplicated an existing 404-parity assertion more weakly. It now runs a tenant-owned SELECT under a PlatformHost request, which is the only way to put a real query in front of an unresolved context. The remark about what the suite constrains was half a measurement. Deleting both query filters leaves all five green because RLS holds; disabling RLS leaves the four reads green because the filters hold; removing both turns all five red. Both directions are now recorded, along with the write case being the one that observes a policy alone. Also: the file moved under Database/, where Standards 06 puts Docker-bound tests and where the trait guard can see it; the probe returns ToActionResult() so a refusal is a status rather than a 200 with an empty body; and two Packet 7 comments the sweep missed. Module: Tenancy ADR: 0003, 0036 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 2 of Step 11's review. No blockers; two long-standing promises the packet was the right place to settle. ADR-0037 said of the idempotency port's raw Guid tenant: "The strongly-typed TenantId lands with the tenancy schema in Packet 6, and both move together." They did not. Packet 6 typed ITenantContext.TenantId and left IIdempotencyStore on a raw Guid, with IdempotentAttribute carrying a comment naming the seam it crossed at a single call site — and nothing recorded the divergence, which is how a promise like that gets discovered three packets later by someone who trusted it. The three methods, the internal key and the census now take TenantId, the unwrapping site is gone, and Amendment 4 records it. The guard changed shape with the type, not just its signature. It refused Guid.Empty; a typed id has two ways to be unassigned, and reading .Value on the first throws from inside the id type. It now tests IsInitialized() before both sentinels, matching AuditInput.EnsureValid and the unit of work's setter, and its test drives the unassigned value from an array element because default(TenantId) does not compile. The Phase Exit Decision names a case that reads with app.tenant_id RESET rather than merely unset, and nothing asserted it. The distinction is the whole reason the policies use NULLIF: a never-set GUC reads NULL, a reset one reads the empty string, and ''::uuid raises 22P02 — so without NULLIF a reused connection would error rather than filter. Measured: removing NULLIF from the nineteen policy predicates turns the new case red with exactly that 22P02. Module: Tenancy ADR: 0037, 0003 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The delivery record, the status blocks in three carriers, the three architecture rules this packet added and never registered, and the two isolation entries that promised a request-level case without naming one. Measured at close: 1187 tests green — counted from a run, not computed. An earlier commit message in this packet carried a total that was eight short because it summed two review rounds instead of applying the second on top. The record is long for the reason Packets 5 and 6's were. Eleven steps, each reviewed twice, and the second round repeatedly found the first round's fix: a conflict translation that turned a 409 into a 500, a seeder idempotency check that masked validation failures, a cache invalidation whose comment claimed a guarantee its placement did not give. The sharpest finding was in the last step — a test that asserted an anonymous POST failed, and therefore passed against a deleted endpoint and against a database with every policy dropped. It also records what the packet did not ship and who owns each: nothing is audited until Packet 9, nothing is authorized until Phase 03, PlatformAdminScope ships with no reachable caller, and the host-resolution cache is invalidated before the commit rather than after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Important Review skippedToo many files! This PR contains 199 files, which is 49 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to Team to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (199)
You can disable this status message by setting the Comment |
cemililik
force-pushed
the
development
branch
from
September 3, 2026 10:10
e944717 to
8428639
Compare
The host resolver opened a transaction and announced app.resolving_host in it without SET TRANSACTION READ ONLY. Four carriers — Database Standards, Security Standards, the glossary and ADR-0040 — describe that transaction as read-only, and read-only is the property that makes an out-of-band setter acceptable at all: learnstack_app holds write grants on the tables the connection reaches, so nothing but the statement made the prose true. The sibling setter two files away had carried it since Packet 6. A behavioural test cannot see it — the transaction is opened, used and disposed inside one method — so the guard is a source scan over both setters, checking presence and position. SET TRANSACTION must precede the transaction's first statement or PostgreSQL refuses it, and the integration half pins the fact the design rests on: a read-only transaction admits set_config and refuses a write with 25006 while the grant is still held. The platform-admin guard accepted rolsuper. A superuser does bypass RLS, so it answered the literal question correctly — and that was the trap. It also bypasses the GRANT matrix that bounds the role, which 02-create-roles.sql writes NOSUPERUSER for on purpose. A deployment that promoted the role widened the platform credential from "reads across tenants" to "does anything" and the guard said nothing. An unparseable connection string was echoed through a redaction regex whose userinfo pattern could not cross a '/' or a second '@' inside a password. Both are legal password characters; either put the secret in a startup log. Measured with two canaries. The message no longer repeats the value at all — an unparseable one cannot be reliably redacted — and names the key and the expected form instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ADR-0022 said its resolver cache was "(Dapr State / Valkey)". That was true when the ADR was accepted; ADR-0035 later demand-gated the adapter. Stale is not false, and Standards 13 is explicit that a statement true when written gets an amendment and never a rewrite — so the sentence is restored and a dated amendment records the current adapter, the trigger, and where the resolver now lives. ADR-0036 gained a paragraph in its Decision body assigning the Tenancy:PlatformHosts collision check to whichever packet builds the host-mapping writer. That is an obligation, not an explanation, and code now enforces it, so it belongs in a dated amendment rather than in the body of an Accepted ADR. Moved verbatim, with what Packet 7 did about it. ADR-0032's Amendments sat above its Decision Drivers, newest first — the section placement predates this packet, the newest-first entry did not. Standards 13 puts amendments at the bottom of the file. Moved there and ordered 1-2-3; verified the file's line multiset is unchanged, so nothing but position moved. ADR: 0022, 0032, 0036 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three majors from the review, each a guard that existed in name only. TenantResolutionAttempt declares HasValidatedPrincipal as the bit separating matrix rows 13 and 15 — "both resolve no tenant, and only one has a principal" — and TenantContextFactory never read it. An attempt carrying a claim with the bit clear produced a HostAndClaim context, so the matrix's own trust distinction was decorative. Unreachable today because authentication registers in Phase 02b and nothing populates a claim, which is exactly why it is worth closing before the first caller decides for itself. No validator checked an inbound strongly-typed id. Reading .Value on an uninitialized Vogen id raises from inside the id type, and the provisioning cross-field rule compares both — so a client's malformed id was an exception inside the validator, a 500 for an input the caller can fix. One shared rule now refuses both sentinels for every command, including the optional organization id on the host mapping. SetFeatureFlag stamped the root before the child validated. ChangeStatus stamps first for a stated reason — MarkUpdated is the only statement in it that can throw — and that reason does not transfer here, so a malformed JSON value moved UpdatedAt, UpdatedBy and row_version for a change that never happened. Since row_version is the concurrency token, the next writer would lose an update to a write that was rejected. Also: AuditableEntity claimed TenantQueryFilters gates on DeletedAt. It does not, and Database Standards makes that exclusion opt-in per aggregate rather than universal. Nothing soft-deletes yet; the comment now says so and names where it will bite first — ux_tenant_domains_host is partial on deleted_at IS NULL. Module: Tenancy ADR: 0036 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A uniqueness conflict told the seeder a name was taken, not that it was taken by us. platform_host_to_tenant's primary key is the host, globally, so "already present" covered both our own prior run and another tenant holding the name — and the seeder exited 0 with the demo host pointing at somebody else's data. It now verifies ownership under the tenant's own announcement, which is what makes the check cheap: RLS shows the row only if the row is ours. Scoped to the act that conflicted, because an earlier shape asked "do we own either?" and let the organization just created vouch for a foreign host. AddLocale accepted a first locale with isDefault:false. The partial unique index guarantees at most one default and nothing guarantees at least one, so a tenant could publish in a language and have no default — a state every reader of "the tenant's default locale" must handle and none expects. The first locale is now the default; a second still is not, or the caller's answer would never matter. The seeder took --connection-string. The value carries a database password and an argument is visible to every local user through ps for the life of the process. The flag is gone; the environment variable was already how make seed passes it. Module: Tenancy Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The seed-tenant skill listed tenant_domains, tenant_settings, tenant_locales and tenant_feature_flags among what the seeder writes. It writes none of them — three commands, covering tenant, organization and host — so the list is now split into what shipped and what a later packet owns, and says where a test that needs those rows gets them instead. The roadmap and the module spec said Packet 7 ships "the first command that touches" the promoted children. Nothing writes a TenantDomain or a TenantSetting; the promotion is a statement about the model — Tenant gained the two navigations, the child factories went internal — not about a command that exists. The add-architecture-test skill published a migration scan under the canonical name Every_TenantOwned_Entity_HasFilterAndRlsPolicy. The scan is file-granular: delete one table's policy from a migration that creates eight and it stays green, because a sibling's block satisfies the Contains. The shipped rule verifies per entity against the EF model. The example is renamed and says which it is. Two comments in CachedHostToTenantResolver contradicted each other about who populates the negative cache — the flight does, whatever the caller does — and UnknownHostCache still said nothing calls Forget, which Packet 7's host-mapping writer does. Also: the catalogue's census was Packet 6's (36 methods, 55 cases); a run says 59 and 77. The Packet 7 status marker was still ⏳ in the packet-sequence body. And Standards 01 and 11 gained material from ADR-0042, ADR-0036 and ADR-0040 without naming them in their Derives-from headers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cemililik
force-pushed
the
development
branch
from
September 3, 2026 12:56
8428639 to
6172a8b
Compare
cemililik
changed the title
cemililik
changed the title
EF keeps a failed entry in the state it had, so an Added row a uniqueness violation refused stays Added. A caller that turns the conflict into Result.Fail and carries on writing therefore has the rejected INSERT still queued, and the next SaveChanges on the same context re-sends it — the row is gone from the database, and the tracker is a claim that outlived its subject. Reachable through nesting, which ADR-0040 permits: an outer handler may absorb an inner failure and keep going on the same scope, and the scope is one DbContext. Added entries only — a Modified entry's original values are what the database still holds, so detaching it would discard a change the caller may retry. The resolver's coalescing remark now says what bounds the flights, because the dictionary does not: a flight is retired in the read's finally, so its size is lookups in flight rather than hosts ever seen, and the ceiling is the Npgsql pool. A distributed flood of novel hostnames degrades into queueing there — the rate limiter bounds one peer, the negative cache bounds repeats, neither bounds first sight — and the admission gate that would is named rather than built, because its trigger is a measurement nobody has taken. Module: Tenancy ADR: 0040 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cemililik
force-pushed
the
development
branch
from
September 3, 2026 16:23
4f517a5 to
1367800
Compare
Four high findings, each verified by measurement before it was acted on. Promoting a default locale through the aggregate raised 23505 every time. PromoteDefault clears the incumbent and then sets the target in memory, and EF does not preserve that order: same-table commands go out in the comparer's order, so the UPDATE that sets the new default can precede the one that clears the old. The composite key (tenant_id, locale) sorts en-US before tr-TR, which is exactly the seeded pair. Nothing caught it because the cases covering this index drive raw SQL in an order they choose — they pin what PostgreSQL does with two statements, not what EF emits for one save. The PR body cited this invariant as the migration's safety rationale; that claim was false. The store now saves in two passes: the promotion is lowered, the clears are saved, the promotion is raised and saved. A partial unique index permits zero defaults, so the intermediate state is one the schema allows, and both saves are inside the caller's transaction. Holding the property back with IsModified alone does not work — SaveChanges accepts current values, so the second pass believes the database already holds true and writes nothing, leaving no default at all. Two architecture rules had escapes. Effective_Host_Computed_In_One_Place banned X-Forwarded-Host, which appears nowhere in the source, and not the header the code actually reads — TrustedHopOptions.HostHeaderName, a public const carrying X-LearnStack-Host. A second file reading it skips IsTrustedHop's CIDR check and constant-time secret comparison, and the rule stayed green. Resolving_Host_Is_Set_In_One_Place exempted its sole setter by filename suffix, so NoopCachedHostToTenantResolver.cs would have been exempt too. Both proven by dropping the shape into the tree and watching the suite pass. The host-reclaim case asserted only that a released hostname can be re-claimed. Dropping ux_tenant_domains_host's uniqueness left it green, because nothing asked for a conflict — and that index is the schema's only guarantee that two tenants cannot hold one hostname at once, with RLS hiding the collision from both sides. Module: Tenancy ADR: 0003, 0036 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Host resolution cached both directions and invalidated one. Clearing only the negative cache covers activation — a host that starts resolving — which is the harmless half. The half that matters is a host deactivated, released or re-pointed at another tenant: it kept serving the previous tenant's answer for the whole positive TTL, which is a cross-tenant answer coming from a cache rather than from a policy. The resolver owns both caches, so it is the invalidator now, and the port says "any cached answer" rather than leaving it to a reader. Host classification was hardcoded to /api/v1 while ApiVersioningExtensions declares which majors are live. A second live major would have skipped classification on its whole surface — an unknown host reaching a handler instead of the bodyless 404, and a tenant-facing route running with no HostClassification feature at all. The prefixes follow LiveMajors, and the test asserts the correspondence rather than the current contents. TransactionBehavior's provisioning announcement is write-only and now says so. It announces the tenant to PostgreSQL and does not touch the accessor the EF query filters read, so an EF read inside a provisioning transaction would return zero rows silently. Nothing reads today — ADR-0042 enumerates three writes, and inserts are not filtered — and making a read safe would mean a fifth writer of a member ADR-0036 Amendment 2 closes at four, which is a decision rather than an edit. Module: Tenancy ADR: 0034, 0036 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TenancyDbContext said `tenants` gets no query filter. It gets one of a different shape — self-keyed, comparing `Id` rather than a `TenantId` column — which the self-keyed branch in TenantQueryFilters has applied since Packet 7. One of the eight entity types genuinely gets none, not two. permissions.md named `tenancy.tenant.admin` as the key that will govern the host mapping, while the matrix directly above gives `HostMapping` its own resource with no `write`. The prose now names `tenancy.hostmapping.admin` and says why the resource is separate: pointing a hostname at a tenant is an admin-scope act, not something the everyday tenant-admin role should carry. ADR-0036's Decision still prints the normalization order Amendment 1 measured as wrong — rejecting IPv4 literals before stripping a port, which lets 1.2.3.4:8080 through because TryParse fails on the port-bearing string and never runs again. The statement was false when it entered the record, so it takes the default instrument: an inline erratum beside it, pointing at the amendment that corrects it. The Decision is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cemililik
force-pushed
the
development
branch
from
September 3, 2026 21:03
402c749 to
97fabdd
Compare
Three commits on this branch tripped the 72-character subject limit, each caught by CI after a push — and a subject is only fixable by rewriting history, so each one cost a force-push. The rule was never the problem; where the failure landed was. The commit-msg hook enforces exactly what CI's meta job enforces: Conventional Commits shape and the 72-character subject. No local-only rule, and nothing CI does not already have — the difference is that locally it is a retry. Merge, revert and fixup subjects are generated by git rather than authored and are skipped. The ADR: and Module: trailers are deliberately not checked: whether a commit owes one depends on judgement a hook does not have, and a hook that guessed is a hook people disable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A locale could be the tenant's default while disabled. The partial unique index says at most one locale is default and says nothing about whether it is enabled, so both entry points produced it — AddLocale(isDefault: true, isEnabled: false) directly, and SetDefaultLocale by promoting a locale added disabled. Every reader of "the tenant's default locale" then holds a row that answers the question and cannot serve it. The guard is in PromoteDefault, which both doors go through — and putting it there exposed a second defect the test caught immediately: AddLocale adds the row and stamps the root BEFORE promoting, so a refused add left the locale added and the version moved for a call that threw. Same shape as SetFeatureFlag's ordering, fixed the same way. Modules_Do_Not_Inject_IEventBus_Directly compared the declared parameter type, so Lazy<IEventBus>, Func<IEventBus> and IEnumerable<IEventBus> all escaped — each injects the port just as effectively, and the first is a shape this codebase already uses for NpgsqlDataSource. The rule now unwraps type arguments transitively, because the wrappers nest. The cross-aggregate census counts IAggregateWriteStore derivations, so a port that does not derive is invisible to it — and one already exists deliberately, IPlatformHostMappingStore, because PlatformHostMapping is a projection with a string key. That exemption is fine; being silent about it is not, because a second such port would join it with nothing to notice. Non-deriving write ports are now enumerated, detected by shape rather than name: an interface whose method takes a type from a module's Domain assembly. Module: Tenancy ADR: 0042 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The review said the pipeline registration used Add rather than TryAdd, so a second call would double every behaviour. I wrote a marker guard for it, then measured: MediatR's AddBehavior already deduplicates — seven behaviours and eleven registrations either way — and removing my guard changed nothing. So the guard went, because a guard no test can kill is a comment. The property is worth pinning even though it is MediatR's rather than ours. Every fixture in the repository registers its probe handler by hand specifically to avoid re-running AddMediatR; if deduplication ever stopped holding, that workaround would be load-bearing rather than cautious, and nothing would say so. RunMigrateTarget read stdout to the end and only then stderr, before waiting. That deadlocks whenever the child fills the second pipe's buffer while this side is blocked on the first — the classic Process pitfall. It has never happened because the role check is terse, which is exactly why it would land on whoever makes the target chattier. Both pipes now drain concurrently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The integration project carried PackageReferences to Testcontainers.Redis and Respawn and used neither, restoring both on every build for nothing. The PackageVersions stay: Directory.Packages.props already names a consumer for each — the Valkey-backed ICacheService adapter in Phase 11, and Phase 03's second module context — and a version binds nothing on its own. The comment there now says which of the two states each package is in, because "declared and used by nothing" did not distinguish them. The decisions README said a superseded ADR is "kept as a redirect". Two are not: 0014 and 0016 keep their status in place, because the record still explains why the decision was made and what replaced it. Redirect stubs under _redirects/ are the other case, where the number was reassigned and only the pointer is worth keeping. Both are readable at their original path; neither is deleted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Database Standards states the property in the same breath as the guards that are supposed to deliver it: "a tenant-scope reporting query may read across organizations, but nothing may write outside its organization." A tenant-wide row belongs to no organization, and both AS RESTRICTIVE guards admitted one to any session — their first arm was a bare `organization_id IS NULL`. That arm exists so a tenant-scope session, which has no app.organization_id, can write those rows. It also let an organization-scoped session rewrite the tenant-wide fallback every other organization reads. Measured before the fix: a session announcing tenant A and organization A1 updated tenant A's organization_id IS NULL row without refusal. The correction is in the canonical template, because that is where the defect lives — every organization-scoped table is told to copy it, and the corpus has shipped a broken template into four files once already. ADR-0003 Amendment 4 records it; ADR-0041 licenses correcting a template in place. `tenant_settings` is the only shipped org-scoped table and a forward-only, policy-only migration brings it into line, reversibly. Intra-tenant, not cross-tenant: the tenant term is untouched and no row crosses a tenant boundary. It is a write-scope correction, not an isolation fix. The refusal is silent by construction — a RESTRICTIVE USING clause on UPDATE filters the rows a statement may target rather than raising — so the new case asserts zero rows affected and the value unchanged. It also made an existing case unreachable: OrganizationIdIsImmutableAfterInsert attempted the NULL -> value move from an org-scoped session, which can no longer target the row at all, so the trigger never fired and the case would have passed while testing nothing. It runs tenant-scope now. ADR: 0003, 0041 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both are real, both are Packet 4's, and both are recorded at the line rather than fixed here — because the fix in each case is a change to a decision that was made deliberately, and the tail of a 200-file packet is the wrong place to revisit one. The store's admission check reads a census refreshed at most once per sweep interval, so within that window the per-tenant and total caps bound nothing: every new key is admitted against a count that has not moved. Throughput and the anonymous rate limiter are what actually limit it. Closing it means live counters decremented on every removal path — sweep, abandon, completion expiry — where a wrong decrement is worse than the current softness, and it belongs with the durable store whose trigger ADR-0037 Amendment 1 sets. The idempotency filter buffers the response into an unbounded MemoryStream and applies the 256 KiB cap to its length afterwards, so an oversized body is materialised in full before being judged too large to store. Capping the stream is not the whole fix: the buffer also holds the body back until the outcome is recorded, and delivering past the cap means streaming as the action writes, which reverses the record-before-deliver ordering ADR-0037 chose and argues for. No idempotent endpoint returns a large body today — every one answers with an identifier, a receipt or a status. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Summary
policies; this gives them something to enforce — a host that resolves, a context the
pipeline announces, and the first commands that write through it.
demo-englishanddemo-yoga, written bymake seedthrough thesame commands a request uses, so a successful seed is evidence about the request path.
from a run, not computed.
Approach
Resolution.
HostClassificationMiddleware,TenantResolverMiddleware,CachedHostToTenantResolver,EffectiveHostnormalization, a separately-capped negativehost cache, and a four-origin
TenantContextFactorywhoseCreateis pure, total andsynchronous.
The authority ceiling.
TenantContextBehavior's two nested gates.[AllowsUnresolvedTenantContext]and[PublicSurface]are deliberate holes and anarchitecture rule counts each — a hole nobody counts becomes a hole everybody uses.
Provisioning.
ProvisionTenantCommand, the one operationADR-0042
sanctions to write two aggregate roots on one transaction, with the
SetProvisioningTenantContextAsyncannouncement seam andIAggregateWriteStore<TRoot,TId>— the codebase's first persistence port. Plus
CreateOrganizationCommandandMapHostToTenantCommand, both taking their tenant from the context and never from therequest.
Where to look first. The
Packet 7 delivery record
is the honest account: eleven steps, each reviewed twice, and the second round repeatedly
found the first round's fix. The sharpest finding was a test that asserted an anonymous
POST failed — which a 404 satisfies — and therefore passed against a deleted endpoint
and against a database with every policy dropped.
Tests
CI=true dotnet test backend/LearnStack.slnx— 1208 green, no skips.suite re-run to confirm something fails. Where a mutation survived it is recorded rather
than hidden — deleting both EF query filters leaves the isolation suite green because
RLS alone holds, and the suite says so, naming the three tests that do catch it.
WebApplicationFactory<Program>with a real Postgres container: five cases driven by thehost header alone, no stubbed
ITenantContext, reading what the real seeder wrote.make seedend to end. The dev compose stack's postgres was down andstarting it would have changed the reviewer's environment. The seeder is proven against
Testcontainers with the same schema and the same
learnstack_approle.Migration / Rollback
Two migrations.
1.
20260903014131_tenant_locale_single_default:Forward. Adds
ux_tenant_locales_tenant_id_is_default— a partial unique index,UNIQUE (tenant_id) WHERE is_default— so a tenant cannot have two default locales. Theaggregate guard alone does not hold across concurrent transactions, which is why the
invariant is in the database.
Additive and non-destructive. No column is dropped, narrowed or retyped, and no data
is rewritten. It creates an index and nothing else.
Rollback.
DROP INDEX ux_tenant_locales_tenant_id_is_default. Reversible with nodata loss; EF's generated
Downdoes exactly that.Precondition. Applying it fails if any tenant already holds two default locales.
Nothing in the shipped code can produce that state —
PromoteDefaultclears before itsets — and the seed does not.
2.
20260903213832_tenant_settings_org_write_guard:Policy-only. No table, column, index or data change — which is why the model
snapshot is untouched and both directions are raw SQL.
Forward. Narrows the two
AS RESTRICTIVEwrite guards ontenant_settingsso anorganization-scoped session cannot write a tenant-wide row. Their first arm was a bare
organization_id IS NULL, which exists so a tenant-scope session can write thoserows and also admitted an organization-scoped one — measured: a session announcing
tenant A and organization A1 rewrote tenant A's tenant-wide row without refusal.
Database Standards states the property the guards were
meant to deliver: "nothing may write outside its organization."
The template is the real fix. Every organization-scoped table is told to copy those
guards, so the correction is in the canonical template, recorded by ADR-0003
Amendment 4.
tenant_settingsis the only shipped table with them.Rollback.
Downrestores the previous predicates exactly. Neither directionrewrites a row.
Intra-tenant, not cross-tenant. The tenant term is untouched and nothing crosses a
tenant boundary; this is a write-scope correction, not an isolation fix.
migrations land in their own PR, and these do not. Flagged rather than hidden,
with the measurement of why splitting is not clean — see the note at the end.
Related
(tenant resolution), 0040 (ambient unit of
work), 0042
(the sanctioned cross-aggregate write),
0003 Amendment 3 (the RLS
template).
Tenancy:PlatformHostsprecedence and whoowns the check), 0037 (Amendment 4 —
IIdempotencyStoretakesTenantId, one packetlater than promised), 0042 (Amendment 1 — the rule counts aggregate roots, not
DbSetuse), 0022 (the cache adapter behindICacheService).Packet 8, the Tenant Customization foundation.
What it deliberately does not ship
AuditLogBehaviorlights up in Packet 9;TransactionBehaviorcarries the
TODOmarking the line the MUST-class write goes on.HTTP endpoint — their callers are the seeder and, from Phase 02c, the Hub.
PlatformAdminScopehas no reachable caller. It ships with its gate closed.store's admission check reads a census refreshed once per sweep interval, so within
that window its caps admit every new key; and the filter buffers a response in full
before applying the 256 KiB cap. Both are Packet 4's, both fixes revisit a decision
ADR-0037 made deliberately, and neither is reachable through an endpoint that exists.
is the request after the write, not one racing it; closing the rest needs a post-commit
seam on
IUnitOfWork, whose surface ADR-0040 governs.One known deviation from the workflow standard, measured
The migration is not in its own PR (Git Standards § Branching,
line 90). I tried to split it and measured that it cannot be done cleanly:
d29cc84, which also carries the model change the migration wasgenerated from — the
Tenantnavigations and the partial-unique-index configuration.Separating those would leave a snapshot that does not match its own migration.
mainconflicts, because it depends on076398a(
[TenantOwned]markers and the EF query filters), which in turn conflicts, because itdepends on the typed-identifier and corpus commits before it.
Packet 7 — which is not a migration PR, it is this PR under another name.
The remaining honest option is to hand-author a standalone index migration on
mainandrebase this branch onto it. That trades a workflow deviation for a migration that was
never the one tested, and a snapshot that has to be reconciled by hand — the exact class
of migration/model mismatch the corpus warns about. I have not done it.
What the migration actually is, for the reviewer the rule exists to protect: one
additive
CREATE INDEX(partial, unique), no column dropped or retyped, no datarewritten,
Downdrops the index, and it fails to apply only if a tenant already holdstwo default locales — a state nothing in the shipped code can produce.
🤖 Generated with Claude Code