Skip to content

refactor(farm): put farm settings behind an IFarmModule contract - #1015

Merged
mforce merged 4 commits into
mainfrom
refactor/851-farm-contract
Oct 2, 2026
Merged

mforce merged 4 commits into
mainfrom
refactor/851-farm-contract

Conversation

@mforce

@mforce mforce commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Closes #851

Epic #514, Track C slice 9. Farm settings, logo and banner now sit behind an IFarmModule contract, the second after Finance (#849). Tenant identity stays in Platform. No route, response, status or schema changes.

Scope decisions (owner-approved at the design checkpoint)

  • Contract surface. IFarmModule reads the current farm's settings as FarmSettingsDetails, reports whether the currency may still change, writes settings through UpdateFarmSettingsCommand, and reads, sets and removes the logo and banner. FarmModule only forwards to the existing repositories and handlers. owners.Farm.contract lists IFarmModule, FarmSettingsDetails, FarmBrandingHashes, FarmLogoMetadata, FarmLogoContent and UpdateFarmSettingsCommand.
  • Adapters moved to it. Account, settings, logo and banner endpoints. The [C] #514 slice 7: Finance pilot — first module behind a contract #849 guard also forced three adapters outside the brief: ExpenseEndpoints.ListExpenses and PaymentEndpoints.ListCustomerBalances (currency label) and SimulationDataSeeder (timezone phase).
  • Sign-in goes through IIdentityProvider.ResolveFarmCodeAsync, not the Farm contract. That lookup establishes identity before TenantContext exists, so IFarmModule must not run it. IdentityProvider implements it with the IAccountRepository it already holds. The query (FindBySlugAsync), the check order (farm code, then suspended, then credentials), the error codes and the statuses are unchanged. Each branch does the same database work as before, so the change adds no timing difference between them.
  • FarmClock stays on IAccountRepository. It sits in Cluckwork.Infrastructure.Time, which is the Platform hub, and inside the stable account seam. No guard sees it. Switching it would construct FarmModule and its five handlers on every dated request and buy no check.
  • The stable account seam stays (audit recommendation). Access, Commerce, General Inventory, Finance, Egg Operations and Flock Management keep calling IAccountRepository and Domain.Accounts directly. That includes the Close the §4.6 currency-lock race properly (shared lock on the account row across money-writing handlers) #162 FOR SHARE locked currency snapshot. The issue body's "lock-aware currency port the Finance pilot introduced" does not exist: CreateExpenseHandler.cs:49 calls accounts.GetCurrentSharedLockedAsync.
  • Farm -> Commerce stays a declared edge. Commerce has no contract to call. The row now also names FarmSettingsDetails and IFarmModule (same file), because the record carries Domain.Catalog.EggUnit.
  • Left out: the locked currency port; role vocabulary, assignments and account lifecycle writes (Access slice); SeedDefaults; Domain.Accounts and Domain.Media vocabulary used inside method bodies; ReportQueries, ExportQueries and CurrencyBoundRowProbe ([C] #514 slice 8: close or date every compatibility exception #850); a contract check for peer modules ([C] #514 slice 7: Finance pilot — first module behind a contract #849 limit).

Rationale: docs/decisions/851-farm-contract.md. AGENTS.md's #849 paragraph now names Farm and links it.

Reach

Before After
Adapter calls that reach Farm outside its contract 23 (11 types) 0
Adapter rows reaching Farm (matrix Platform -> Farm) A (18) A (17)
AuthEndpoints.Login reach Access, Farm Access
Farm -> Commerce edge symbols 3 5

The "before" count comes from running the #849 adapter check on main with a placeholder Farm contract. No edges were added or removed.

Change map

BEFORE                                     AFTER

AccountEndpoints                           AccountEndpoints
FarmLogoEndpoints                          FarmLogoEndpoints
FarmBannerEndpoints                        FarmBannerEndpoints
 ├─► IAccountRepository (Account entity)    └─► IFarmModule ◄── new front door
 ├─► IFarmLogoRepository                           │
 ├─► ICurrencyBoundRowProbe                        ▼
 ├─► UpdateFarmSettingsHandler                 FarmModule (just forwards)
 └─► Set/Remove Logo/Banner handlers (4)        ├─► IAccountRepository
                                                ├─► IFarmLogoRepository
                                                ├─► ICurrencyBoundRowProbe
                                                ├─► 5 handlers (unchanged)
                                                └─► returns plain records:
                                                    FarmSettingsDetails,
                                                    FarmBrandingHashes,
                                                    FarmLogoMetadata/Content

ExpenseEndpoints.ListExpenses              ExpenseEndpoints.ListExpenses
 ├─► IFinanceModule                         ├─► IFinanceModule
 ├─► IInsightsModule                        ├─► IInsightsModule
 ├─► IFlockRepository                       ├─► IFlockRepository  (Flock slice, #852)
 └─► IAccountRepository (currency)          └─► IFarmModule       (currency)

PaymentEndpoints.ListCustomerBalances      PaymentEndpoints.ListCustomerBalances
 └─► IAccountRepository (currency)          └─► IFarmModule       (currency)

SimulationDataSeeder                       SimulationDataSeeder
 ├─► IAccountRepository                     └─► IFarmModule
 └─► UpdateFarmSettingsHandler

AuthEndpoints.Login                        AuthEndpoints.Login
 ├─► IIdentityProvider                      └─► IIdentityProvider (Platform port)
 └─► IAccountRepository.FindBySlugAsync          ├─► ResolveFarmCodeAsync ◄── new
                                                 └─► IdentityProvider
                                                      └─► IAccountRepository.FindBySlugAsync

Unchanged: FarmClock, TenantResolutionMiddleware, TenantContext, the stamp
interceptor, middleware order, every peer-module IAccountRepository caller.

Ledger:

module-ledger.json
  owners.Farm.contract = [IFarmModule, FarmSettingsDetails, FarmBrandingHashes,
                          FarmLogoMetadata, FarmLogoContent, UpdateFarmSettingsCommand]
  edges  Farm -> Commerce   + FarmSettingsDetails, IFarmModule (reason extended)
  adapters AuthEndpoints.Login   [Access, Farm] -> [Access]   (pruned Loosenable)
coupling-matrix.md   regenerated: Farm -> Commerce R (3) -> R (5); Platform -> Farm A (18) -> A (17)

Verification

Test baselines, measured, never quoted:

Project main b9f7f76 main 7ce95cf (base) This branch
Domain 495 495 495
Application 570 593 595 (+2 FarmModuleTests)
AppHost 10 10 10
Integration 1873 1873 1873

dotnet build Cluckwork.sln is clean with warnings as errors.

Security suites, run one by one on this branch, all green. The only test edits in the PR are the two one-line ResolveFarmCodeAsync pass-throughs in the IIdentityProvider decorators in MustChangePasswordGateTests and StepUpAuthTests.

Suite Passed
AccountLockoutTests 5
AccountSuspensionTests (#579) 16
AmbientPrincipalOnLoginTests 6
AuthCookieContractTests 6
StepUpAuthTests 45
MustChangePasswordGateTests 5
CredentialEpochTests 14
CredentialEpochRaceTests 9
CredentialEpochMiddlewareOrderTests 3
FirstRunLoginNoticeTests 4
LoginCounterKeyProbeTests 1

Tenant query-filter and stamping tests are unedited and pass. No entity or mapping changed, so the EF model is identical.

Mutations, each red and then reverted:

  • IFarmLogoRepository logos added back to FarmLogoEndpoints.GetLogo makes AdapterReachRealTreeTests report a contract bypass.
  • Task<Account?> LeakAsync(CancellationToken ct) on IFarmModule makes ModuleContractRealAssemblyTests fail.
  • Swapping DateFormatOverride and TimeFormatOverride in FarmModule's copy makes FarmModuleTests fail.

The first full integration run on this branch had two SeedCommandTests demo-seed cases exceed their 60 s budget while still seeding, at load average 11.8. The demo seeder is untouched. Those 11 tests then passed alone, and a second full run passed 1873/1873.

CI

A red image check is expected and unrelated (#1006).

mforce added 2 commits October 2, 2026 04:00
Login establishes identity before TenantContext exists, so its farm-code
lookup moves behind the Platform identity port instead of reaching Farm's
IAccountRepository from the endpoint. Same query, same check order, same
error codes and statuses.
Account, settings, logo and banner endpoints, the expense and customer-balance
currency reads and the simulation seeder reach Farm through IFarmModule.
FarmClock and peer modules keep IAccountRepository as the stable account seam.

Closes #851
@mforce

mforce commented Oct 2, 2026

Copy link
Copy Markdown
Owner Author

Review of record: Codex gpt-6-astra, round 1, at head 661d2782. Posted by the coordinator. Log paths named below are local to the review host. The cross-PR finding will be handled in #1013 (#850), which is already fixing the table-ownership defect this review describes; #1015 merges first and #1013 then adds the genuine Farm exception rows.

Review of PR #1015

Reviewed 661d2782dd4eaf90c13eb93e9d77d844374193b9 against 7ce95cf475318e161c39647529e5b5f17a41b24a, including all 21 changed files, the issue's September 26 audit and the requested decision records. Review and tests were performed directly in the assigned detached checkout. No product defect found.

Findings

P2 · CONFIRMED · Guard integration defect, not a product defect

tests/Cluckwork.Application.Tests/Architecture/Data/module-ledger.json:5 — Activating the Farm contract makes #1013's compatibility guard fail because the combined ledger lacks 16 required exception rows.

Failure scenario: merge #1013 and #1015 without reconciling the exceptions, in either order, and CompatibilityExceptionRealTreeTests.RealSourceTree_EveryCompatibilityExceptionIsRegistered fails in the application CI leg. #1015 alone passes. This is a confirmed cross-PR integration problem, not an existing failure on this PR's base.

Evidence: fetched #1013 at f5bedac73662445b930ec12a25e98f9291042b8c, temporarily copied its scanner, tests and ledger parser into this checkout, and combined its compatibilityExceptions with #1015's ledger. Ran dotnet test tests/Cluckwork.Application.Tests --filter FullyQualifiedName~CompatibilityException. Result: 25 passed, 1 failed, with exactly 16 distinct undeclared Farm entries and no semantic-compilation failure. Full output: /tmp/1015-combined-850.log. All temporary changes were reverted.

Coordinate the reconciliation in whichever PR lands second. Preserve the approved identity/account seam; moving login or the currency lock behind IFarmModule is not an appropriate fix. The ownership mismatch described below deserves correction in #1013 rather than eight misleading Farm exceptions.

Interaction with PR #1013: measured counts

Farm owns exactly Accounts and FarmLogos in the table ledger. Banner data shares FarmLogos; there is no separate banner table.

Five Infrastructure members read genuinely Farm-owned tables without an allowance. All five read Accounts; none reads FarmLogos.

Member First site
DailyEntryLockSweep.RunAsync src/Cluckwork.Infrastructure/Jobs/DailyEntryLockSweep.cs:49
DemoDataSeeder.MissingBaseDataAsync src/Cluckwork.Infrastructure/Persistence/DemoDataSeeder.cs:212
SimulationDataSeeder.MissingBaseDataAsync src/Cluckwork.Infrastructure/Persistence/SimulationDataSeeder.cs:463
SimulationDataSeeder.SeedSecondAccountAsync src/Cluckwork.Infrastructure/Persistence/SimulationDataSeeder.cs:1925
SimulationDataSeeder.ComputeCountsAsync src/Cluckwork.Infrastructure/Persistence/SimulationDataSeeder.cs:2227

AccountRepository and FarmLogoRepository qualify as implementations of Farm ports. The identity services qualify through their declared Access → Farm edges. ReportQueries.AccountCurrencyAsync qualifies through its declared Insights → Farm edge. FarmClock calls the repository, not a DbSet. ExportQueries and CurrencyBoundRowProbe do not read Accounts.

The actual scanner reports 12 Infrastructure entries, not five: it additionally classifies seven UserRoleAssignments accesses as Farm. These are UserRoleAssignmentRepository.ListByUserAsync at line 14, ListByNameByUserAsync at 39, ListAllAsync at 50, GetByIdAsync at 53, AddAsync at 56, Remove at 59, and FlockScopeGuard.CheckAsync at 81, all in src/Cluckwork.Infrastructure/Repositories/UserRoleAssignmentRepository.cs. Its walk counts DbSet accesses, including writes.

This happens because #1013 resolves ownership from the entity's CLR namespace, Domain.Accounts, without applying the table ledger's explicit Access ownership override for UserRoleAssignments. The repository implements an Access port, so it does not receive the Farm-port allowance.

The scanner also walks projects referencing Infrastructure and reports four API entries:

Member Site Table
AccountSlugLookup.ResolveAsync src/Cluckwork.Api/Cli/SuspendAccountCliCommand.cs:99 Accounts
ListAccountsCliCommand.RunAsync src/Cluckwork.Api/Cli/ListAccountsCliCommand.cs:31 Accounts
CredentialEpochMiddleware.InvokeAsync src/Cluckwork.Api/Middleware/CredentialEpochMiddleware.cs:54 Accounts
FlockScopeResolutionMiddleware.InvokeAsync src/Cluckwork.Api/Middleware/FlockScopeResolutionMiddleware.cs:49 UserRoleAssignments

Thus the unmodified combined guard needs 16 new rows: 12 Infrastructure plus 4 API. Correcting its table ownership handling would leave 8 genuine Farm exceptions, comprising the five Infrastructure members and three API account readers. #1013's expiry field is an issue trigger, deleteWhen: "#...", rather than a calendar date.

Login parity

Compared the entire endpoint and identity path against the base, not just the new wrapper.

  • ResolveFarmCodeAsync calls the same IAccountRepository.FindBySlugAsync once. AccountRepository.cs:42–49 still trims, lowercases invariantly, calls IgnoreQueryFilters(), uses AsNoTracking() and executes the same FirstOrDefaultAsync predicate. The implementation does not use a tenant-filtered read.
  • The per-IP login limiter and IgnoresAmbientPrincipal metadata are unchanged. Ambient authentication is cleared before tenant middleware. Login does not set TenantContext, either before or after this change. The account id is passed explicitly to the same account-scoped user lookup. Subsequent authenticated requests resolve their tenant from the JWT as before.
  • Validation still precedes the farm query. Unknown farm codes still stop after that query with 401 Auth.UnknownFarmCode, no credential lookup/hash and no lockout-counter write. Suspended farms still stop at the same point with 401 Auth.FarmSuspended, before credentials and the first-run notice.
  • Active farms reach the unchanged LoginAsync: account-scoped user query, the same disabled/lockout/password branches, failure-counter persistence, credential-epoch/stamp snapshot, success-counter reset and refresh-token save. The default-account first-run query remains only on the same failed-login branch. Cookies use the same account id.
  • No DB round trip or hash operation was added, removed or reordered on any login branch. The new in-memory two-field record does not introduce a new enumeration branch. Existing farm-code/suspension disclosure and the Suspension: login can still mint an inert credential in the check-then-mint window #579 inert-credential issuance window remain unchanged. This conclusion is based on the unchanged executed query/control-flow paths, not a statistical latency claim.
  • No production IFarmModule call precedes tenant resolution. Endpoint calls follow the existing tenant guard; simulation resolves the tenant at line 295 before its settings phase at line 1032.

The existing security tests were unmodified except for the two permitted decorator pass-throughs. Executed suite counts include lockout 5, suspension 16, ambient-login principal 6, auth cookies 6, step-up authentication 45, credential epoch 14, epoch races 9, epoch middleware order 3, must-change-password 5, first-run notice 4 and login counter probe 1. All passed. The broader selected run also exercised auth body limits, validation, logging and limiter paths.

Other behavior and contract checks

Settings, logo and banner route metadata and authorization are unchanged: settings reads/writes and branding mutations remain Owner-only; the account and image reads remain available to authenticated roles. Validation, timezone rejection, failure/status mapping, upload limits, sanitization, ETags, conditional requests and content hashes still reach their original implementations. Existing mutation handlers retain their Version behavior and concurrency handling.

All 16 FarmSettingsDetails fields map correctly, including currency minor units, nullable day/format fields, sale-allocation policy, discount percentage and version. Expense totals and customer balances retain their currency values and null fallbacks. The simulation seeder now takes an untracked settings snapshot before the unchanged tracked update handler; it carries every setting, including the discount ceiling and version, into the update. The simulation tests passed.

The six listed Farm contract types cover the approved interface, command and result records without exposing entities or EF types. FarmClock, the shared currency lock and the peer-module account seam remain unchanged. The Farm → Commerce edge declares the new DTO references.

Three independent temporary mutations produced the expected failures:

  1. Adding IFarmLogoRepository to FarmLogoEndpoints.GetLogo failed AdapterReachRealTreeTests with a Farm contract-bypass diagnostic.
  2. Adding a default interface method returning Task<Account?> failed ModuleContractRealAssemblyTests, identifying the aggregate leak through Task<Account> → Account.
  3. Swapping the date/time format fields in FarmModule.ToDetails failed FarmModuleTests.GetSettings_CopiesEveryField with the literal expected/actual mismatch.

Logs: /tmp/1015-mutation-bypass.log, /tmp/1015-mutation-entity.log, /tmp/1015-mutation-copy.log.

Verification

  • dotnet build Cluckwork.sln: passed, zero warnings and errors, including the build: enforce file-scoped namespaces and using placement at build time #985 style gate. The initial --no-restore attempt on this fresh checkout lacked assets; the normal build restored and passed.
  • Full application suite: 595 passed.
  • Docker-backed auth/security, farm settings/branding, currency-race and simulation selection: 332 passed.
  • Additional expense/payment, farm clock and tenant-isolation selection: 24 passed.
  • After reverting all mutations and test(arch): register and date every module compatibility exception #1013 files, CLUCKWORK_REGENERATE_MATRIX=1 dotnet test tests/Cluckwork.Application.Tests --filter 'FullyQualifiedName~Architecture|FullyQualifiedName~FarmModuleTests': 288 passed; generated matrix unchanged.
  • Final git status --porcelain and git diff --check: clean. HEAD remained the assigned commit. No branch was created, no commit/push/comment was made, and no other checkout was touched.

Integration commands used sg docker -c 'dotnet test ...'. Results are in tests/Cluckwork.Api.IntegrationTests/TestResults/1015-review.trx and 1015-parity.trx; application results are in tests/Cluckwork.Application.Tests/TestResults/1015-application.trx. Full solution tests and the unrelated image check were not rerun. No SeedCommand timeout finding is asserted.

Nits

P3 · CONFIRMED · Documentation only. docs/decisions/851-farm-contract.md:75 incorrectly says ReportQueries, ExportQueries and CurrencyBoundRowProbe all read the account row and belong to Insights. Only ReportQueries.AccountCurrencyAsync reads it; ExportQueries has no account query, and CurrencyBoundRowProbe lives in Platform's repositories namespace and probes seven financial/inventory DbSets. A later #850 cleanup following this paragraph would target two nonexistent account reads and miss the actual seeder/job exceptions. Correct the paragraph to name the real account reader and distinguish the Finance compatibility work.

Verdict: No product blocker in #1015; resolve the confirmed guard incompatibility before #1013 and #1015 both land.

@mforce

mforce commented Oct 2, 2026

Copy link
Copy Markdown
Owner Author

Fixed Astra round 1's P3 in 871c2249. The "What this does NOT cover" paragraph in docs/decisions/851-farm-contract.md now names ReportQueries.AccountCurrencyAsync as the one Insights read of the account row. It also lists the other direct account readers that #850 will register as compatibility exceptions. It no longer names ExportQueries or CurrencyBoundRowProbe. This is a docs-only change. The ImagePin and Documentation guards pass (20/20). No further review round.

@mforce mforce closed this Oct 2, 2026
@mforce mforce reopened this Oct 2, 2026
@mforce
mforce merged commit 4c429f0 into main Oct 2, 2026
20 checks passed
@mforce
mforce deleted the refactor/851-farm-contract branch October 2, 2026 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C] #514 slice 9: Farm contract, with tenant identity staying in Platform

1 participant