Skip to content

Commit 64310f8

Browse files
cemililikclaude
andauthored
docs: close Packet 7 against what actually merged (#16)
* docs: close Packet 7 against what actually merged The delivery record was written at packet close, and fifteen commits landed after it from the pull request's own review rounds — including the sharpest defect of the packet. A record that stopped before them would be a false account, so it now carries a § What the pull-request review found: the shipped invariant that did not hold (PromoteDefault's order is not EF's, and this packet's PR description cited it as the migration's safety rationale), the canonical template that admitted an organization-scoped session to tenant-wide rows, three credential and transaction gaps, two architecture rules with escapes, three ADR edits made outside the rules that govern them, and two findings the measurement refuted. The measured total moves with it: 1208, counted from a run. Three architecture rules the review added were never registered — Every_Write_Port_Is_Countable_Or_Enumerated, which closes the census's blind spot for ports that do not derive; Out_Of_Band_Setters_Open_Read_Only_Transactions, which a behavioural test cannot express; and Registering_The_Pipeline_Twice_Registers_It_Once, which pins a property MediatR provides and the repository depends on. Database Standards now names ADR-0003 Amendment 4 in its Derives from header, because the template it carries is what that amendment corrects. ADR: 0003, 0042 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(decisions): renumber a colliding amendment, widen a rule ADR-0003 already had an Amendment 4 — the 2026-08-29 note that its Phase 02a table list had gone stale — and the write-guard amendment was appended with the same number. An erratum inside the same file says "Recorded in Amendment 4" and now pointed at two of them. It becomes Amendment 5, takes the file's own `##` heading shape, and every carrier that named the wrong number follows: Database Standards, the delivery record, the schema test and the migration's remarks. The Status header listed Amendments 1-3 only, so a reader following the authority metadata could not tell either Amendment 4 or 5 was part of the record. Both are named now. Every_Write_Port_Is_Countable_Or_Enumerated claimed to find interfaces taking a domain type and checked the parameter's own assembly, so IEnumerable<Course> answered System.Private.CoreLib and escaped — which is how a bulk write port is written. It unwraps generic arguments, array elements and by-ref transitively now, the same shape the IEventBus rule uses, and a probe bulk port is caught. Out_Of_Band_Setters_Open_Read_Only_Transactions is a source scan that orders SET TRANSACTION against the first set_config, and the catalogue described it as ordering against the first statement. PostgreSQL refuses SET TRANSACTION after any statement, so a setter that ran a SELECT in between would satisfy the rule and fail at runtime. The entry now says what the comparison measures and what it therefore does not catch — loudly, since that failure is immediate rather than silent, and parsing every command execution is the expensive alternative. ADR: 0003, 0042 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 parents 801d2f4 + 57b0c58 commit 64310f8

7 files changed

Lines changed: 183 additions & 9 deletions

File tree

‎backend/src/Modules/Tenancy/LearnStack.Modules.Tenancy.Infrastructure/Persistence/Migrations/20260903213832_tenant_settings_org_write_guard.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ namespace LearnStack.Modules.Tenancy.Infrastructure.Persistence.Migrations
2525
/// <para>
2626
/// Intra-tenant rather than cross-tenant — the tenant term is untouched, and no row
2727
/// crosses a tenant boundary — so this is a write-scope correction, not an isolation
28-
/// fix. See ADR-0003 Amendment 4 and Database Standards § Tenant-Owned and
28+
/// fix. See ADR-0003 Amendment 5 and Database Standards § Tenant-Owned and
2929
/// Organization-Scoped Tables, which carries the corrected template.
3030
/// </para>
3131
/// <para>

‎backend/tests/LearnStack.Tests.Architecture/AggregateWriteTests.cs‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ public void Every_Write_Port_Is_Countable_Or_Enumerated()
8080
.Where(type => type.IsInterface)
8181
.Where(type => type.GetMethods().Any(method =>
8282
method.GetParameters().Any(parameter =>
83-
domainAssemblies.Contains(parameter.ParameterType.Assembly))))
83+
Unwrap(parameter.ParameterType).Any(inner =>
84+
domainAssemblies.Contains(inner.Assembly)))))
8485
.Where(type => !WriteStoreConstructions(type).Any())
8586
.Select(type => type.Name)
8687
.Distinct()
@@ -163,4 +164,39 @@ private static IEnumerable<string> ProductionAssemblies() =>
163164
.Select(Path.GetFileNameWithoutExtension)
164165
.Where(name => !string.IsNullOrEmpty(name))
165166
.Select(name => name!);
167+
168+
/// <summary>A declared type and every type reachable through it.</summary>
169+
/// <remarks>
170+
/// Through wrappers, because a bulk write port takes one: <c>IEnumerable&lt;Course&gt;</c>
171+
/// lives in <c>System.Private.CoreLib</c>, so a check on the parameter's own assembly
172+
/// sees the wrapper and not the domain type inside it — and the port escapes the
173+
/// enumeration while satisfying every word of what this rule claims to detect.
174+
/// Transitive, and through arrays and by-ref, because those nest too:
175+
/// <c>Course[]</c> and <c>in Course</c> are the same port with different syntax.
176+
/// </remarks>
177+
private static IEnumerable<Type> Unwrap(Type declared)
178+
{
179+
yield return declared;
180+
181+
if (declared.HasElementType && declared.GetElementType() is { } element)
182+
{
183+
foreach (var inner in Unwrap(element))
184+
{
185+
yield return inner;
186+
}
187+
}
188+
189+
if (!declared.IsGenericType || declared.IsGenericTypeDefinition)
190+
{
191+
yield break;
192+
}
193+
194+
foreach (var argument in declared.GetGenericArguments())
195+
{
196+
foreach (var inner in Unwrap(argument))
197+
{
198+
yield return inner;
199+
}
200+
}
201+
}
166202
}

‎backend/tests/LearnStack.Tests.Integration/Database/TenancySchemaTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ public async Task An_Organization_Scoped_Session_Cannot_Write_A_Tenant_Wide_Row(
569569
//
570570
// Both AS RESTRICTIVE guards used a bare `organization_id IS NULL` first arm,
571571
// which exists so a TENANT-scope session can write those rows — and admitted an
572-
// org-scoped one to them as well. Measured before ADR-0003 Amendment 4: a session
572+
// org-scoped one to them as well. Measured before ADR-0003 Amendment 5: a session
573573
// announcing tenant A and organization A1 rewrote tenant A's tenant-wide row.
574574
//
575575
// The refusal is silent by construction: a RESTRICTIVE USING clause on UPDATE

‎docs/decisions/0003-tenant-isolation-defense-in-depth.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44

55
Accepted (Amendment 1: 2026-05-18 — adds Organization scope; Amendment 2: 2026-05-19 —
66
identity row terminology; **Amendment 3: 2026-08-08 — corrects the RLS policy template
7-
and adds the database role model**; see bottom of document)
7+
and adds the database role model**; Amendment 4: 2026-08-29 — the Phase 02a table list
8+
has gone stale; **Amendment 5: 2026-09-04 — the write guards exclude an
9+
organization-scoped session from tenant-wide rows**; see bottom of document)
810

911
## Decision
1012

@@ -301,7 +303,7 @@ The single authority for which class a table belongs to is
301303
into three documents drifts in three directions, and this copy already had — which
302304
is why the assignment lives in one file and this section keeps only the *rule*.
303305

304-
### Amendment 4 (2026-09-04): the write guards admitted an organization-scoped session to tenant-wide rows
306+
## Amendment 5 — The write guards admitted an organization-scoped session to tenant-wide rows (2026-09-04)
305307

306308
**What was wrong.** Amendment 3's template closes the `USING`-only write paths with two
307309
`AS RESTRICTIVE` guards, one `FOR UPDATE` and one `FOR DELETE`. Both read:

‎docs/roadmap/phase-02a-kernel-tenancy.md‎

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2437,9 +2437,11 @@ one — and the second round repeatedly found the first round's fix.
24372437

24382438
> **Packet 7 — Tenant and organization resolution, isolation, two tenants ✅**
24392439
>
2440-
> **Measured at close: 1187 tests green** — 1 contract, 76 architecture, 802 unit,
2441-
> 308 integration. Counted from a run, not computed from a plan; an earlier commit
2442-
> message in this packet carried an arithmetic total that was eight short.
2440+
> **Measured at merge: 1208 tests green** — 1 contract, 79 architecture, 813 unit,
2441+
> 315 integration. Counted from a run, not computed from a plan; an earlier commit
2442+
> message in this packet carried an arithmetic total that was eight short. The number
2443+
> moved after this record was first written, because the pull request's own review found
2444+
> more — see § What the pull-request review found.
24432445
24442446
### What shipped
24452447

@@ -2506,6 +2508,50 @@ one — and the second round repeatedly found the first round's fix.
25062508
took a raw `Guid` tenant after the ADR said it and `ITenantContext.TenantId` "both
25072509
move together". Amendment 4 records the conversion and the divergence.
25082510

2511+
### What the pull-request review found
2512+
2513+
The record above was written at packet close. Fifteen commits landed after it, from three
2514+
review rounds on the pull request itself, and the sharpest defect of the packet was among
2515+
them — so the record would be a false account without them.
2516+
2517+
- **A shipped invariant that did not hold.** Promoting a default locale through the
2518+
aggregate raised `23505` every time. `PromoteDefault` clears the incumbent and then sets
2519+
the target in memory, and EF does not preserve that order: same-table commands go out in
2520+
its comparer's order, and the composite key `(tenant_id, locale)` sorts `en-US` before
2521+
`tr-TR` — exactly the seeded pair. Nothing caught it because the cases covering that
2522+
index drive raw SQL in an order they choose, so they pin what PostgreSQL does with two
2523+
statements rather than what EF emits for one save. **This packet's own PR description
2524+
cited that invariant as the migration's safety rationale.** The store now saves in two
2525+
passes; a partial unique index permits zero defaults, so the intermediate state is one
2526+
the schema allows.
2527+
- **A canonical template that admitted the wrong writer.** Both `AS RESTRICTIVE` write
2528+
guards began with a bare `organization_id IS NULL` — the arm that lets a *tenant*-scope
2529+
session write rows belonging to no organization. It admitted an *organization*-scoped
2530+
session to them as well, so one organization could rewrite the tenant-wide fallback
2531+
every other organization reads. Measured. Corrected in the template, because every
2532+
organization-scoped table is told to copy it, and recorded as
2533+
[ADR-0003 Amendment 5](../decisions/0003-tenant-isolation-defense-in-depth.md).
2534+
- **Three credential and transaction gaps.** The host resolver announced a session
2535+
variable in a transaction four documents call read-only and that was not; the
2536+
platform-admin guard accepted `rolsuper`, which bypasses the GRANT matrix that bounds
2537+
the role; and an unparseable connection string was echoed through a redaction pattern
2538+
that could not cross a `/` or a second `@` inside a password.
2539+
- **Two architecture rules with escapes, and one blind test.** The effective-host rule
2540+
banned a header the code never reads instead of the one it does; the resolving-host rule
2541+
exempted its sole setter by filename suffix; and the host-reclaim case passed with the
2542+
hostname uniqueness dropped entirely, because nothing in it asked for a conflict.
2543+
- **Three ADR edits made outside the rules that govern them** — a stale statement rewritten
2544+
rather than amended, a normative paragraph added to an Accepted body, and an amendment
2545+
placed above the decision it amends.
2546+
- **Two findings refuted by measurement**, which is why they are worth recording: MediatR
2547+
already deduplicates pipeline behaviours, so a guard written for it was removed rather
2548+
than shipped; and the query filter's pinned `DbContext` is harmless because its
2549+
`CurrentTenantId` delegates to the process-wide accessor.
2550+
2551+
**A hook now catches what CI caught three times.** Three commits on the branch tripped the
2552+
72-character subject limit, each found after a push — and a subject is only fixable by
2553+
rewriting history. `.githooks/commit-msg` enforces exactly what CI's `meta` job enforces.
2554+
25092555
### What it did not ship, and who owns each
25102556

25112557
- **Nothing is audited.** `AuditLogBehavior` lights up in Packet 9; `TransactionBehavior`
@@ -2518,3 +2564,17 @@ one — and the second round repeatedly found the first round's fix.
25182564
- **The host-resolution cache is invalidated before the commit, not after.** The
25192565
guarantee is therefore the request *after* the write, not the one racing it; closing
25202566
the rest needs a post-commit seam on `IUnitOfWork`, whose surface ADR-0040 governs.
2567+
- **Two idempotency limits do not bound memory,** and are recorded at the line rather than
2568+
fixed. The store's admission check reads a census refreshed once per sweep interval, so
2569+
within that window its caps admit every new key; and the filter buffers a response in
2570+
full before applying the 256 KiB cap. Both are Packet 4's, both fixes revisit an
2571+
ordering [ADR-0037](../decisions/0037-idempotency-key-contract.md) chose deliberately,
2572+
and neither is reachable through an endpoint that exists — every idempotent surface
2573+
today answers with an identifier, a receipt or a status. They belong with the durable
2574+
store, on that ADR's trigger.
2575+
- **Two migrations shipped in one pull request,** which
2576+
[Git Standards § Branching](../standards/14-git-workflow.md) says should not happen.
2577+
Splitting was attempted and measured infeasible: the migration commit carries the model
2578+
change it was generated from, and cherry-picking it onto `main` conflicts through the
2579+
marker and typed-identifier commits before it — a "migration PR" would have carried
2580+
roughly the first thirty commits of the packet.

‎docs/standards/05-database.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
(Amendments 1 + 2),
66
[ADR-0003 Tenant Isolation Defense in Depth](../decisions/0003-tenant-isolation-defense-in-depth.md)
77
(Amendment 1: Organization Scope; **Amendment 3: corrected RLS policy template and
8-
database role model**),
8+
database role model**; **Amendment 5: the write guards exclude an organization-scoped
9+
session from tenant-wide rows**),
910
[ADR-0006 Events and Outbox](../decisions/0006-events-and-outbox.md)
1011
(Amendment 1: Dapr pub/sub dispatch transport),
1112
[ADR-0038 Cross-Cutting Port and Event Contracts](../decisions/0038-cross-cutting-port-and-event-contracts.md),

‎docs/standards/21-architecture-tests-catalogue.md‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,6 +1082,81 @@ because the filters hold, and removing both turns all five red.
10821082
- **Status:** **Implemented** (Packet 7, `LearnStack.Tests.Architecture`).
10831083
- **Phase:** 02a Packet 7.
10841084

1085+
#### `Every_Write_Port_Is_Countable_Or_Enumerated`
1086+
1087+
- **Asserts:** every interface in a production assembly whose method takes a type from a
1088+
module's `Domain` assembly — **directly, or inside a generic, array or by-ref wrapper**
1089+
— either derives from `IAggregateWriteStore<TRoot, TId>`, and is therefore visible to
1090+
the cross-aggregate census above, or appears on a literal allow-list. The list holds one
1091+
name: `IPlatformHostMappingStore`.
1092+
- **Wrappers are unwrapped transitively,** because a bulk write port is written with one:
1093+
`IEnumerable<Course>` lives in `System.Private.CoreLib`, so a check on the parameter's
1094+
own assembly sees the wrapper rather than the domain type inside it — and the port would
1095+
escape the enumeration while satisfying every word of what this rule claims.
1096+
- **Why it exists.** The census counts derivations, so a port that does not derive is
1097+
invisible to it. One already is, deliberately: `PlatformHostMapping` is a projection
1098+
with a string key rather than an aggregate root. That exemption is fine; being *silent*
1099+
about it is not, because a second such port would join the first with nothing to notice,
1100+
and the census that keeps ADR-0042's exception at one entry would stop describing the
1101+
system.
1102+
- **Detected by shape, not by name.** "Takes a domain type" rather than "ends in `Store`":
1103+
a rule keyed on a suffix is satisfied by renaming.
1104+
- **Source:** [ADR-0042](../decisions/0042-tenant-provisioning-cross-aggregate-transaction.md).
1105+
- **Type:** xUnit + reflection. **Kind:** structural.
1106+
- **Status:** **Implemented** (Packet 7 review, `LearnStack.Tests.Architecture`,
1107+
`AggregateWriteTests`).
1108+
- **Phase:** 02a Packet 7.
1109+
1110+
#### `Out_Of_Band_Setters_Open_Read_Only_Transactions`
1111+
1112+
- **Asserts:** the two components that announce a session variable outside the ambient
1113+
unit of work — `CachedHostToTenantResolver` and `OrganizationScopeValidator` — each
1114+
contain `SET TRANSACTION READ ONLY`, and contain it **at a lower source offset than
1115+
their first `set_config(`**.
1116+
- **What the offset comparison does and does not prove.** PostgreSQL refuses
1117+
`SET TRANSACTION` after the transaction's *first statement of any kind*, and this scan
1118+
only orders it against the announcement. A setter that ran some other statement — a
1119+
`SELECT`, a second `SET` — between `BEGIN` and `SET TRANSACTION READ ONLY` would satisfy
1120+
the rule and fail at runtime. That failure is loud and immediate rather than silent,
1121+
which is why the cheap ordering check is the one that ships; the expensive alternative
1122+
is parsing the method for every command execution, and
1123+
[§ What a structural test proves](#what-a-structural-test-proves--and-what-it-does-not)
1124+
states the general limit.
1125+
- **Why the property matters at all.** Read-only is what makes an out-of-band setter of a
1126+
session variable acceptable, because `learnstack_app` holds write grants on the tables
1127+
these connections reach — so nothing but this statement stops a future edit from writing
1128+
under an announcement no request made.
1129+
- **Why a scan and not a behavioural test.** The transaction is opened, used and disposed
1130+
inside one method, so nothing outside can observe its settings. Measured: the resolver
1131+
shipped without the statement while four carriers — Database Standards, Security
1132+
Standards, the glossary and ADR-0040 — described it as read-only, and the validator two
1133+
files away had carried it since Packet 6.
1134+
- **Source:** [ADR-0040](../decisions/0040-ambient-unit-of-work.md);
1135+
[05-database.md](05-database.md); [11-security.md](11-security.md).
1136+
- **Type:** xUnit + source scan. **Kind:** structural.
1137+
- **Status:** **Implemented** (Packet 7 review, `LearnStack.Tests.Architecture`,
1138+
`TenancyConventionTests`).
1139+
- **Phase:** 02a Packet 7.
1140+
1141+
#### `Registering_The_Pipeline_Twice_Registers_It_Once`
1142+
1143+
- **Asserts:** calling `AddLearnStackMediatRPipeline` twice on one `ServiceCollection`
1144+
yields the same registrations as calling it once — the same behaviour count and the same
1145+
total.
1146+
- **The property is MediatR's, not ours.** `AddBehavior` deduplicates; measured at seven
1147+
behaviours and eleven registrations either way. It is pinned because the repository
1148+
depends on it and did not write it: every test fixture registers its probe handler by
1149+
hand specifically to avoid re-running `AddMediatR`, and if deduplication stopped
1150+
holding, that workaround would become load-bearing rather than cautious with nothing to
1151+
say so. A doubled `TransactionBehavior` is a nested frame on every request.
1152+
- **A guard of our own was written and removed.** It changed nothing under mutation, and a
1153+
guard no test can kill is a comment.
1154+
- **Source:** [ADR-0032 § Sub-decision 2](../decisions/0032-exception-handling-logging-and-observability.md).
1155+
- **Type:** xUnit + DI registration inspection. **Kind:** structural.
1156+
- **Status:** **Implemented** (Packet 7 review, `LearnStack.Tests.Architecture`,
1157+
`CrossCuttingFoundationTests`).
1158+
- **Phase:** 02a Packet 7.
1159+
10851160
#### `Tenant_Context_Guard_Fires_Only_On_An_Unmarked_Transaction`
10861161

10871162
- **Asserts:** both arms of the `DbCommandInterceptor` guard. A command a module `DbContext` issues on a transaction no sanctioned setter announced throws `TenantContextMissingException`; the same command on an announced transaction runs. One arm is not the rule: a guard keyed on `TransactionBehavior` instead of on the marker passes the first arm and rejects the writes the idempotency store and the audit store legitimately make on their own short transactions.

0 commit comments

Comments
 (0)