Repository navigation
test(arch): table-owner completeness walked from the EF model (#845) - #877
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Local Codex review round on b16908b. Two findings, both rated exotic by the reviewer, both confirmed and fixed in 5a39938 with fixtures:
Stopping the local review loop here, deliberately: the round's whole yield was two exotic shapes. CodeRabbit's round is scheduled for 21:45 UTC behind the other two PRs' windows and will be handled. Verification: |
|
Codex (gpt-5.6-sol) review of record on 5a39938, in lieu of the CodeRabbit bot per the owner's decision: earlier fixes verified present, ledger verified against the 37 mapped tables and the 11 configured cross-owner FKs; two findings, both confirmed and fixed in 2ae22dc with fixtures:
Verification: |
2ae22dc to
89fd87d
Compare
Walk 37 mapped tables and 11 cross-owner FKs from AppDbContext.
Retain the namespace ledger behavior and document six ownership overrides.
Five real-ledger mutations were run red and reverted:
1-remove-payments:
table 'Payments' (entity Cluckwork.Domain.Sales.Payment) has no owner
2-duplicate-expenses:
table 'Expenses' claimed by Farm, Finance
3-remove-farm-logos:
table 'FarmLogos' (entity Cluckwork.Domain.Accounts.FarmLogo) has no owner
4-remove-expense-flock-fk:
undeclared cross-owner foreign key FK_Expenses_Flocks_FlockId from Finance to FlockManagement — add {"name":"FK_Expenses_Flocks_FlockId","from":"Finance","to":"FlockManagement","reason":""}
5-move-payments:
table 'Payments' owner Farm disagrees with CLR namespace owner Commerce (entity Cluckwork.Domain.Sales.Payment)
…845) Codex review round. SplitToTable fragments are now part of the table census, and a cross-owner foreign key is keyed by dependent table and constraint name, with the table in the ledger row, so one row cannot excuse two constraints that share a name on different tables.
…licated ledger sections (#845) Resolve FK constraint names against dependent and principal table store objects, including fragments. Fixtures prove the ChildDetails constraint cannot be excused by a row naming Children, including a qualified schema. Reject every duplicate root property in the ledger. Fixtures cover all five sections present on this branch. Both regression groups were run red first.
89fd87d to
0c2bd14
Compare
…eeders (#846) (#878) Closes #846 Slice 4 of #514 (modular monolith), Track B. **Enforcement only.** No `src/` file, no CI change, no package. **Stacked on #877** (base `chore/845-table-owners`), which stacks on #872. Retarget after those merge; until then CI does not run here and the closing link is inert, so the guards were run locally (below). ## What this adds - `module-ledger.json` gains `adapterRoots` (the namespaces and types that count as adapters: `Cluckwork.Api.Endpoints`, `Cluckwork.Api.Cli`, `Cluckwork.Infrastructure.Jobs`, the two seeders; and the namespaces where persistence types are forbidden outright: `Cluckwork.Api.Endpoints`) and `adapters`, one row per adapter with a non-empty reach: 147 rows today over 397 walked adapters (116 endpoint members, 25 seeder members, 6 CLI verbs). - `Architecture/AdapterReachScanner.cs`: a syntax-only Roslyn walk. An adapter is every method, constructor or primary constructor declared in a type under the adapter roots (a private helper counts too: over-approximation only raises the ceiling). Its reach is the set of module owners resolved from its parameter types, their generic arguments, and the type arguments of `GetRequiredService<T>()`, `GetService<T>()`, `GetRequiredKeyedService<T>()` and `ActivatorUtilities.CreateInstance<T>()` in its body, resolving fully qualified, imported, aliased, relative and same-file names; Platform is excluded. An unmatched simple name is reported as unresolved, never guessed. `AppDbContext`, `DbContext`, `DbSet<>` or `IQueryable` in an endpoint is a hard failure; in CLI verbs, jobs and seeders they are Platform infrastructure and not counted. - **Ratchet, not ban**: an undeclared crossing is red and prints the JSON row; a declared crossing that disappears stays green and is listed as loosenable so a maintainer can prune. - `AdapterReachTests.cs` (temp trees, one assertion per rule) and `AdapterReachRealTreeTests.cs` (the gate plus a floor of 40 adapters). - `docs/decisions/846-adapter-reach-ratchet.md` and one AGENTS.md bullet under Application shape. ## What the walk found - `ListExpenses` reaches Farm, Finance, FlockManagement and Insights: the audit repository interface lives in `Application.Features.Audit`, which the ledger assigns to Insights. - The widest adapter reaches seven owners (the simulation seeder). - CLI verbs, jobs and the simulation seeder hold `AppDbContext` for migrate, sweeps and seeding, which is why the persistence ban is endpoint-only (agent question during the build, answered and recorded in the decision record). ## Mutation evidence (each recorded, then reverted before commit) | # | Mutation | Result | |---|---|---| | 1 | `IProductRepository` parameter added to `ListExpenses` | red: `undeclared adapter reach …ExpenseEndpoints.ListExpenses -> Commerce` | | 2 | `AppDbContext` parameter added to an endpoint handler | red: `forbidden persistence type … in …` | | 3 | The `IFlockRepository` parameter removed from `ListExpenses` | **green**; `FlockManagement` listed as loosenable (the ratchet proof) | | 4 | `GetRequiredService<CreateFlockHandler>()` added to `MigrateCliCommand.RunAsync` | red: `…MigrateCliCommand.RunAsync -> FlockManagement` | ## Verification (local, since the stacked PR gets no CI) - `dotnet build Cluckwork.sln`: 0 warnings. - `Cluckwork.Application.Tests`: 406 passed. - `Documentation` tests and `SchemaDocsTests` green on the new markdown (run before opening). <details><summary>Mutation outputs</summary> **1-commerce-parameter.txt** ``` RED: ListExpenses -> Commerce Command: dotnet test tests/Cluckwork.Application.Tests --no-build --filter FullyQualifiedName~AdapterReachRealTreeTests.RealSourceTree_EveryAdapterReachIsDeclared --logger console;verbosity=detailed Test run for /home/mforce/.cluckwork-slices/846/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll (.NETCoreApp,Version=v10.0) VSTest version 18.0.2 (x64) Starting test execution, please wait... A total of 1 test files matched the specified pattern. /home/mforce/.cluckwork-slices/846/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v4.0.0+05679a7ab5 (64-bit .NET 10.0.12) [xUnit.net 00:00:00.10] Discovering: Cluckwork.Application.Tests [xUnit.net 00:00:00.19] Discovered: Cluckwork.Application.Tests [xUnit.net 00:00:00.21] Starting: Cluckwork.Application.Tests [xUnit.net 00:00:02.14] Cluckwork.Application.Tests.Architecture.AdapterReachRealTreeTests.RealSourceTree_EveryAdapterReachIsDeclared [FAIL] [xUnit.net 00:00:02.15] adapter reach guard failed: [xUnit.net 00:00:02.15] undeclared adapter reach Cluckwork.Api.Endpo ``` **2-endpoint-dbcontext.txt** ``` RED: forbidden persistence type Cluckwork.Infrastructure.Persistence.AppDbContext Command: dotnet test tests/Cluckwork.Application.Tests --no-build --filter FullyQualifiedName~AdapterReachRealTreeTests.RealSourceTree_EveryAdapterReachIsDeclared --logger console;verbosity=detailed Test run for /home/mforce/.cluckwork-slices/846/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll (.NETCoreApp,Version=v10.0) VSTest version 18.0.2 (x64) Starting test execution, please wait... A total of 1 test files matched the specified pattern. /home/mforce/.cluckwork-slices/846/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v4.0.0+05679a7ab5 (64-bit .NET 10.0.12) [xUnit.net 00:00:00.09] Discovering: Cluckwork.Application.Tests [xUnit.net 00:00:00.18] Discovered: Cluckwork.Application.Tests [xUnit.net 00:00:00.20] Starting: Cluckwork.Application.Tests [xUnit.net 00:00:02.11] Cluckwork.Application.Tests.Architecture.AdapterReachRealTreeTests.RealSourceTree_EveryAdapterReachIsDeclared [FAIL] [xUnit.net 00:00:02.11] adapter reach guard failed: [xUnit.net 00:00:02.11 ``` **3-remove-flock-parameter.txt** ``` GREEN: Cluckwork.Api.Endpoints.Expenses.ExpenseEndpoints.ListExpenses -> FlockManagement Command: dotnet test tests/Cluckwork.Application.Tests --no-build --filter FullyQualifiedName~AdapterReachRealTreeTests.RealSourceTree_EveryAdapterReachIsDeclared --logger console;verbosity=detailed Test run for /home/mforce/.cluckwork-slices/846/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll (.NETCoreApp,Version=v10.0) VSTest version 18.0.2 (x64) Starting test execution, please wait... A total of 1 test files matched the specified pattern. /home/mforce/.cluckwork-slices/846/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v4.0.0+05679a7ab5 (64-bit .NET 10.0.12) [xUnit.net 00:00:00.10] Discovering: Cluckwork.Application.Tests [xUnit.net 00:00:00.19] Discovered: Cluckwork.Application.Tests [xUnit.net 00:00:00.21] Starting: Cluckwork.Application.Tests [xUnit.net 00:00:02.08] Finished: Cluckwork.Application.Tests Passed Cluckwork.Application.Tests.Architecture.AdapterReachRealTreeTests.RealSourceTree_EveryAdapterReachIsDeclared [1 s] Standard Output Messages ``` **4-cli-flock-service.txt** ``` RED: MigrateCliCommand.RunAsync -> FlockManagement Command: dotnet test tests/Cluckwork.Application.Tests --no-build --filter FullyQualifiedName~AdapterReachRealTreeTests.RealSourceTree_EveryAdapterReachIsDeclared --logger console;verbosity=detailed Test run for /home/mforce/.cluckwork-slices/846/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll (.NETCoreApp,Version=v10.0) VSTest version 18.0.2 (x64) Starting test execution, please wait... A total of 1 test files matched the specified pattern. /home/mforce/.cluckwork-slices/846/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v4.0.0+05679a7ab5 (64-bit .NET 10.0.12) [xUnit.net 00:00:00.09] Discovering: Cluckwork.Application.Tests [xUnit.net 00:00:00.18] Discovered: Cluckwork.Application.Tests [xUnit.net 00:00:00.20] Starting: Cluckwork.Application.Tests [xUnit.net 00:00:02.12] Cluckwork.Application.Tests.Architecture.AdapterReachRealTreeTests.RealSourceTree_EveryAdapterReachIsDeclared [FAIL] [xUnit.net 00:00:02.12] adapter reach guard failed: [xUnit.net 00:00:02.12] undeclared adapter reac ``` </details>
…e walks (#848) (#879) Closes #848 Slice 6 of #514 (modular monolith), Track B. **Enforcement only.** No `src/` file, no CI change, no package. **Stacked on #878** (base `chore/846-adapter-ratchet`), which stacks on #877 and #872. Retarget after those merge; until then CI does not run here and the closing link is inert, so the guards were run locally (below). ## What this adds - `Architecture/CouplingMatrix.cs`: a pure generator that renders the coupling matrix from the ledger plus the three walks (module edges, table census, adapter reach). A module-to-module cell is the ledger's kind letter with the live symbol count (`W (8)`), plus `fk:<n>` when the table census has cross-owner foreign keys in that direction; the Platform column is `P` (free hub); the Platform row is `A (n)`, the number of adapters reaching that module. `E` and `Q` are not syntactically observable and the file says so. - `Architecture/Data/coupling-matrix.md`: the committed output, generated and checked like `docs/schema/` (#417). `RealTree_CommittedMatrixMatchesRegeneration` regenerates from the real tree and asserts byte equality (unified diff on mismatch; `CLUCKWORK_REGENERATE_MATRIX=1` rewrites it). `RealTree_GeneratedModuleCellCensusMatchesLedgerEdges` pins that the generated cells and the ledger's edge set cannot drift. - The generator carries the design's 2026-08 §3.4 values as a constant purely to print the differences table, which is the last copy of the hand-written matrix anywhere in the repo. - `docs/decisions/848-generated-coupling-matrix.md` and one AGENTS.md bullet under Application shape. ## What the generated matrix says that the hand-written one did not Seven cells differ: five the hand-written matrix marked as no coupling (Access → Commerce `W`, Access → EggOperations `W`, Farm → Commerce `R`, Commerce → Access `R`, GeneralInventory → EggOperations `R fk:2`) and two whose letter was wrong (Access → Farm is `W`, Finance → Farm is `R`), each established and reasoned in #872's review rounds. ## Mutation evidence (each recorded red, then reverted before commit) | # | Mutation | Red line | |---|---|---| | 1 | `kind` removed from one ledger edge cell | `module-ledger registry error(s): 1` | | 2 | one cell of the committed matrix hand-edited | `coupling matrix differs:` followed by the unified diff | | 3 | the live Finance → FlockManagement ledger cell deleted | `undeclared cross-owner edge Finance -> FlockManagement …` and the census/ratchet drift assertion | ## Verification (local, since the stacked PR gets no CI) - `dotnet build Cluckwork.sln`: 0 warnings. - `Cluckwork.Application.Tests`: 424 passed. - `Documentation` tests and `SchemaDocsTests` green on the new markdown (run before opening). <details><summary>Mutation outputs</summary> **1-missing-kind.txt** ``` Cluckwork.Domain -> /home/mforce/.cluckwork-slices/848/src/Cluckwork.Domain/bin/Debug/net10.0/Cluckwork.Domain.dll Cluckwork.Application -> /home/mforce/.cluckwork-slices/848/src/Cluckwork.Application/bin/Debug/net10.0/Cluckwork.Application.dll Cluckwork.Infrastructure -> /home/mforce/.cluckwork-slices/848/src/Cluckwork.Infrastructure/bin/Debug/net10.0/Cluckwork.Infrastructure.dll Cluckwork.Application.Tests -> /home/mforce/.cluckwork-slices/848/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll Test run for /home/mforce/.cluckwork-slices/848/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll (.NETCoreApp,Version=v10.0) VSTest version 18.0.2 (x64) Starting test execution, please wait... A total of 1 test files matched the specified pattern. [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v4.0.0+05679a7ab5 (64-bit .NET 10.0.12) [xUnit.net 00:00:00.09] Discovering: Cluckwork.Application.Tests [xUnit.net 00:00:00.19] Discovered: Cluckwork.Application.Tests [xUnit.net 00:00:00.21] Starting: Cluckwork.Application.Tests [xUnit.net 00:00:02.53] Cluckwork.Application.Tests.Architecture.ModuleLedgerRealT ``` **2-hand-edited-matrix.txt** ``` Cluckwork.Domain -> /home/mforce/.cluckwork-slices/848/src/Cluckwork.Domain/bin/Debug/net10.0/Cluckwork.Domain.dll Cluckwork.Application -> /home/mforce/.cluckwork-slices/848/src/Cluckwork.Application/bin/Debug/net10.0/Cluckwork.Application.dll Cluckwork.Infrastructure -> /home/mforce/.cluckwork-slices/848/src/Cluckwork.Infrastructure/bin/Debug/net10.0/Cluckwork.Infrastructure.dll Cluckwork.Application.Tests -> /home/mforce/.cluckwork-slices/848/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll Test run for /home/mforce/.cluckwork-slices/848/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll (.NETCoreApp,Version=v10.0) VSTest version 18.0.2 (x64) Starting test execution, please wait... A total of 1 test files matched the specified pattern. [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v4.0.0+05679a7ab5 (64-bit .NET 10.0.12) [xUnit.net 00:00:00.09] Discovering: Cluckwork.Application.Tests [xUnit.net 00:00:00.18] Discovered: Cluckwork.Application.Tests [xUnit.net 00:00:00.21] Starting: Cluckwork.Application.Tests [xUnit.net 00:00:03.57] Cluckwork.Application.Tests.Architecture.CouplingMatrixRea ``` **3-deleted-finance-flock-edge.txt** ``` Cluckwork.Domain -> /home/mforce/.cluckwork-slices/848/src/Cluckwork.Domain/bin/Debug/net10.0/Cluckwork.Domain.dll Cluckwork.Application -> /home/mforce/.cluckwork-slices/848/src/Cluckwork.Application/bin/Debug/net10.0/Cluckwork.Application.dll Cluckwork.Infrastructure -> /home/mforce/.cluckwork-slices/848/src/Cluckwork.Infrastructure/bin/Debug/net10.0/Cluckwork.Infrastructure.dll Cluckwork.Application.Tests -> /home/mforce/.cluckwork-slices/848/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll Test run for /home/mforce/.cluckwork-slices/848/tests/Cluckwork.Application.Tests/bin/Debug/net10.0/Cluckwork.Application.Tests.dll (.NETCoreApp,Version=v10.0) VSTest version 18.0.2 (x64) Starting test execution, please wait... A total of 1 test files matched the specified pattern. [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v4.0.0+05679a7ab5 (64-bit .NET 10.0.12) [xUnit.net 00:00:00.10] Discovering: Cluckwork.Application.Tests [xUnit.net 00:00:00.19] Discovered: Cluckwork.Application.Tests [xUnit.net 00:00:00.22] Starting: Cluckwork.Application.Tests [xUnit.net 00:00:01.99] Cluckwork.Application.Tests.Architecture.CouplingMatrixRea ``` </details>
… their surfaces exist (#843) (#880) Closes #843 Track A slice 2 of #514, stacked on #879 (retarget to `main` after it merges). Enforcement only, no `src/` change. ## What it adds A tier is a dated exemption: a namespace, a privilege a module may not otherwise have, the surface that maps it, a reason and a `reviewBy` issue. The ledger gains one `adapterTiers` row today, `Cluckwork.Api.Mcp` → `DirectRepository`, surfaced by `MapMcp`, reviewed by #806. It is committed **dormant**: green now, with no `/mcp` and no tool class in the tree, so the ledger is not wrong the day #806 lands. `AdapterTierScanner` (syntax-only, same walk as the sibling guards) fails on: - a `[McpServerToolType]` type outside every tier namespace, - a `MapMcp` call with no tier row (prints the JSON row to add), - a tier row missing a field, with a privilege outside the closed set, or with a `reviewBy` that is not `#NNN` (an exemption with no end date). A tier also unions into the reach ratchet's adapter roots and its persistence-forbidden set, so a tool class's repository parameters still need an `adapters` row and an `AppDbContext` parameter still fails. ## Mutations run against the real tree (each red before it was claimed, then reverted) | Mutation | Result | |---|---| | `[McpServerToolType] class Probe` under `Cluckwork.Api.Endpoints` | red, tool type outside every tier | | `app.MapMcp("/mcp")` in `Program.cs` with the tier row removed | red, mapped surface with no row; output prints the row | | `reviewBy` removed from the row | red, registry error | | `WaterTools(IWaterUsageRepository)` under `Cluckwork.Api.Mcp` | red, undeclared reach `WaterTools.ctor → GeneralInventory` | | same class taking `AppDbContext` | red, forbidden persistence type | ## Why syntax, not `EndpointDataSource` The issue suggested keying the tier on `EndpointDataSource`. The ledger guards live in the Application test project with no host, and the reach ratchet already finds top-level routes by scanning `Map*` calls in `Program.cs`; a `MapMcp` scan is the same mechanism. The decision record covers this and the other rejected shapes. ## Scope against the issue The issue's second bullet, owner rows for the four OpenIddict tables, is delivered by #877, not here: the table-owner guard fails #795's PR the moment it maps a table with no owner row, so the rows are written there by that author. The "add a mapped table with no owner row → red" criterion is therefore met on `main` already. The remaining criteria are the four mutations above. ## For #806's author Put tool classes under `Cluckwork.Api.Mcp`, add their `adapters` rows when the ratchet prints them, and delete `AdapterTierRealTreeTests.McpTierRow_IsDormantToday`, which goes stale on purpose. Tests: Architecture and Documentation filter 246/246 (was 244). Full solution builds warnings-as-errors clean.
Closes #845
Slice 3 of #514 (modular monolith), Track B. Enforcement only. No
src/file, no migration, no CI change, no package.Stacked on #872 (base branch
chore/842-module-ledger): it extends that PR's ledger. Retarget tomainafter #872 merges; until then CI does not run here and the closing link is inert, so the guards were run locally (below).What this adds
module-ledger.jsongainstables(every relational table under exactly one owner),foreignKeys(every cross-owner FK by constraint name, direction and reason) andtableOwnerOverrides(a table whose design owner differs from its CLR namespace owner, with a reason).Architecture/TableOwnerScanner.cswalks the EF model built from a model-onlyAppDbContext(unreachable host, no connection) and fails on a table with no owner, a table claimed twice, a stale table row, an owner that disagrees with the entity's CLR namespace owner without an override, an undeclared or stale cross-owner FK, a stale override, or fewer than 30 tables walked.TableOwnerTests.cs(fixture model, one assertion per rule) andTableOwnerRealModelTests.cs(the gate: 37 tables, 11 cross-owner FKs, zero violations).docs/decisions/845-table-owners.mdand one AGENTS.md bullet under Data and correctness.What the walk found
__EFMigrationsHistoryis not an entity type.FK_BirdMovements_DailyEntries_DailyEntryId(Flock Management → Egg Operations, mortality provenance).UserRoleAssignmentsis Access by §3.3 but its CLR type lives inCluckwork.Domain.Accounts(Farm); recorded as a reasoned override, no namespace move.Mutation evidence (each recorded red, then reverted before commit)
Paymentsremoved from Commercetable 'Payments' (entity Cluckwork.Domain.Sales.Payment) has no ownerExpensesadded to Farm as welltable 'Expenses' claimed by Farm, FinanceFarmLogosrow removedtable 'FarmLogos' (entity Cluckwork.Domain.Accounts.FarmLogo) has no ownerFK_Expenses_Flocks_FlockIdrow deletedundeclared cross-owner foreign key FK_Expenses_Flocks_FlockId from Finance to FlockManagementPaymentsmoved to Farmtable 'Payments' owner Farm disagrees with CLR namespace owner CommerceVerification (local, since the stacked PR gets no CI)
dotnet build Cluckwork.sln: 0 warnings.Cluckwork.Application.Tests: 366 passed.Documentationtests (19) andSchemaDocsTests(4) green on the new markdown.Mutation outputs
1-remove-payments.txt
2-duplicate-expenses.txt
3-remove-farm-logos.txt
4-remove-expense-flock-fk.txt
5-move-payments.txt