Skip to content

refactor(inventory): put general inventory behind an IInventoryModule contract - #1027

Merged
mforce merged 4 commits into
mainfrom
refactor/855-inventory-contract
Oct 2, 2026
Merged

mforce merged 4 commits into
mainfrom
refactor/855-inventory-contract

Conversation

@mforce

@mforce mforce commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

Closes #855

General Inventory now sits behind an IInventoryModule contract, the fourth module after Finance (#849), Farm (#851) and Flock Management (#852). No route, response, status, query or schema changes.

Built on #1026 (the #1022 fix, merged as 582f8c8a). This branch was rebased onto main after that merge and retargeted to it.

Checkpoint decisions (owner-approved)

Rationale: docs/decisions/855-inventory-contract.md. AGENTS.md's #849 paragraph now names General Inventory and the lock-order rule.

Reach

Before (main 6e3f584, placeholder contract) After
Adapter crossings into General Inventory through a non-contract type 33 (22 types) 0
Adapter rows reaching General Inventory (matrix Platform -> GeneralInventory) A (18) A (18)
#850 reads of General Inventory tables from outside it, unregistered 35 0
...admitted by the 5 listed repository implementations 27
...registered compatibilityExceptions rows 8

No edges added or removed. The regenerated coupling-matrix.md is byte-identical, and the adapter pruning list (Loosenable) is empty.

Change map

BEFORE                                         AFTER
InventoryEndpoints (13 adapters)               InventoryEndpoints
WaterUsageEndpoints (3 adapters)               WaterUsageEndpoints
 ├─► 8 handlers                                 └─► IInventoryModule ◄── new front door
 ├─► 5 repositories                                  └─► InventoryModule (only forwards)
 └─► InventoryItem (ToResponse)                           ├─► 8 handlers (unchanged)
                                                          ├─► 5 repositories
ListFeedUsage / ListWaterUsage                            └─► returns 5 *Details records
 └─► IFlockLookup (#852)                       ListFeedUsage / ListWaterUsage
                                                └─► IFlockLookup (#852, unchanged)

SimulationDataSeeder                           SimulationDataSeeder
 ├─► 5 handlers + IInventoryItemRepository      ├─► IInventoryModule
 └─► db.Inventory* reads                        └─► db.Inventory* reads (7 rows, deleteWhen #858)

RecordFeedUsageHandler (body untouched)
 item FOR UPDATE → IFlockLookup eligibility → lots FOR UPDATE, one transaction

CurrencyBoundRowProbe (code untouched)
 rows: Finance #855 → #854;  + GeneralInventory, owner Farm, #854

module-ledger.json
  owners.GeneralInventory.contract        = IInventoryModule, 5 *Details records,
                                            7 commands, RecordFeedUsageResponse
  owners.GeneralInventory.implementations = FeedUsageRepository, InventoryItemRepository,
                                            InventoryLotRepository, InventoryMovementRepository,
                                            WaterUsageRepository
  compatibilityExceptions                 + 7 SimulationDataSeeder rows (Platform, #858)
                                          + 1 CurrencyBoundRowProbe row (Farm, #854)
                                            1 re-dated (Finance probe row, #855 → #854)
  edges                                   unchanged
coupling-matrix.md                         regenerated, unchanged

Files this PR shares with #1021 (merged) end in #1021's shape plus these lines:
InventoryEndpoints, WaterUsageEndpoints, SimulationDataSeeder constructor, DI
registrations, module-ledger.json, AGENTS.md.

Verification

Test counts, measured, never quoted:

Project main 6e3f584 main 08cf44c (#1021) main 582f8c8 (#1026) This branch
Domain 495 495 495 495
Application 639 655 655 661 (+6 InventoryModuleTests)
AppHost 10 10 10 10
Integration 1873 1879 1880 1881 (+1 lock-order test B)

dotnet build Cluckwork.sln is clean with warnings as errors. No existing test was edited. The only test-file changes are the new InventoryModuleTests, test B appended to #1026's new FeedUsageLockOrderTests, and ledger data.

EF model digest: SHA-256 of the design-time Model.ToDebugString(LongDefault) is 45F550E426163CEBEFF36E509CFC6E53B36D26D3D89C72E0F728855BCC8FB9AE on main 6e3f584 and on this branch's head. The 1486-line dumps are byte-identical.

Ordering and race evidence. FeedUsageLockOrderTests holds one row lock in a separate connection, waits until the usage request is provably parked on it (pg_blocking_pids), archives the flock, then releases the lock:

  • Test A (fix(inventory): see a flock archived while feed usage waits on the item lock #1026), holding the item lock: refused 422 FeedUsage.FlockNotActive. Moving the eligibility read above the item lock turns it RED.
  • Test B (this PR), holding the lot lock: recorded 200, because eligibility was read before the lots and the flock row is deliberately unlocked. Moving the lot read above the eligibility read turns it RED.

Suites run one by one on this branch, all green and unedited:

Suite Passed Covers
FeedUsageLockOrderTests 2 item lock → eligibility → lots
FeedUsageTests 13 backdated FIFO, insufficient-stock rollback, parallel usages on one lot, archived flock, daily-entry link
InventoryTests 8 purchases, unit lock, unit change racing first purchase, parallel deactivates, parallel updates
WaterUsageTests 7 record guards, archived flock read-only, parallel updates on one Version (exactly one wins), daily-entry link
FlockScopeTests 16 flock scope and eligibility on feed and water writes
TenantScopedLockTests 2 foreign-tenant item lock never waited on
CurrencyLockRaceTests 9 #162 currency lock vs item create, item update and purchase
CurrencyLockSerializationTests 4 #162 serialization
FarmSettingsTests 54 currency change refused after feed money or a lot exists, allowed with only a costless item (the probe)
NamedRowProjectionTests 24 feed and water flock names
RoleMatrixTests 18 route authorization
AdminGatingTests 7 admin-only inventory routes

Mutations, each RED, then reverted:

  • IInventoryLotRepository lots added back to InventoryEndpoints.ListLots: AdapterReachRealTreeTests reports contract bypass ... ListLots -> GeneralInventory through ...IInventoryLotRepository.
  • A member returning InventoryItem added to IInventoryModule: ModuleContractRealAssemblyTests fails.
  • InventoryLotRepository removed from implementations: CompatibilityExceptionRealTreeTests reports its eight members as undeclared.
  • The probe's General Inventory row removed: CompatibilityExceptionRealTreeTests fails.
  • QuantityReceived and QuantityAvailable swapped in the lot copy: InventoryModuleTests.ListLots_CopiesEveryField fails.
  • MeterStart and MeterEnd swapped in the water copy: InventoryModuleTests.ListWaterUsage_... fails.
  • Eligibility moved above the item lock: test A fails. Lot read moved above eligibility: test B fails.

Cleanup. pstack:deslop ran on this branch's diff before opening and found nothing to remove. A /noslop self-review of the full diff followed and found nothing either.

Nothing user-visible changes, so this PR has no screenshots.

@mforce

mforce commented Oct 2, 2026

Copy link
Copy Markdown
Owner Author

Review of record: Codex gpt-6-astra (xhigh), round 1, at head ac25073a. Posted by the coordinator. Log paths named below are local to the review host.

Approve with one documentation nit. No product defect or blocking correctness finding was found.

Reviewed PR #1027, issue #855, at ac25073a68a73d02e28609aa7f5899e574e37e37 against stacked base debd841d. Scope was the 12-file, +666/-55 diff. Review and execution used /home/mforce/.cluckwork-slices/review-1027. Read the PR, issue comments and audit, AGENTS.md, and the seven requested decision records. No delegation.

The one nit is separate from correctness findings:

  • P3 · CONFIRMED · docs/decisions/855-inventory-contract.md:93. “The three handlers' peer calls” incorrectly counts the handlers carrying the three named peer dependencies; those calls span six handlers. Scenario: a maintainer using this paragraph to inventory the peer calls cannot identify a group of three handlers containing all the described dependencies. Evidence: CreateInventoryItemHandler.cs:11, UpdateInventoryItemHandler.cs:10 and RecordPurchaseHandler.cs:16 inject IAccountRepository; the separate RecordFeedUsageHandler.cs:21, RecordWaterUsageHandler.cs:13 and UpdateWaterUsageHandler.cs:15 inject IFlockLookup. The first two of that latter group also inject IDailyEntryRepository. Replace “three handlers” with “existing handlers.” This is documentation only, not a product defect.

Behavior review found parity:

  • All inventory and water routes retain their methods, authorization, validation, status/error mapping, paging bounds, tenant checks and flock-scope paths. The module forwards all eight writes with the same arguments and cancellation tokens. No handler body changed.
  • All five *Details copies preserve the fields their endpoint responses use. Money retains minor units, currency code and currency precision; nullable fields, on-hand quantity and the item/lot/feed/water versions are copied correctly. Water updates retain their existing Version conflict handling. Generic database metadata remains outside the existing responses, consistent with Standardize business-record timestamps and chronological list ordering #819.
  • The repository calls and their order are unchanged. Item lists remain name-ordered; chronological lot, movement, feed and water lists retain business date, CreatedAtUtc, and shadow Sequence. FIFO locking still uses ReceivedDate, Id and excludes lots received after the usage date.
  • The seeder's replacement expression is exactly db.InventoryLots.AnyAsync(l => l.InventoryItemId == itemId, ct), matching InventoryItemRepository.HasLotsAsync. Both use the same scoped context and tenant-filtered set. Seeder rerun tests passed.
  • Item lock, fresh flock eligibility and FIFO lot locks remain in that order within the transaction. Test B successfully archives the flock while usage holds the item lock and waits for the lot, then records usage. The flock row remains unlocked. The three currency snapshot callers retain IAccountRepository.GetCurrentSharedLockedAsync and Close the §4.6 currency-lock race properly (shared lock on the account row across money-writing handlers) #162's FOR SHARE behavior.

The ledger is accurate. Its edge and adapter sections are structurally identical to the base. The five implementation entries are exactly FeedUsageRepository, InventoryItemRepository, InventoryLotRepository, InventoryMovementRepository and WaterUsageRepository, each implementing its matching inventory repository interface. The seven new seeder rows name the actual existence/count readers, belong to Platform and trigger on #858. The new probe row belongs to Farm, names FeedUsages, InventoryItems and InventoryLots, and triggers on #854. The Finance probe row moves from #855 to #854. This matches the approved decision to defer the whole probe until Commerce has a contract.

I regenerated the coupling matrix using CLUCKWORK_REGENERATE_MATRIX=1; it is byte-identical to HEAD, SHA-256 261b8f0e55921aa5bf446991b0006e34d178c45d7aef5682c53f81cdd2d2ad19.

Local execution passed:

Check Result
Solution build, including #985 style gates 0 warnings, 0 errors
Full Application suite 661 passed
FeedUsageLockOrderTests 2 passed
FeedUsageTests 13 passed
InventoryTests 8 passed
WaterUsageTests 7 passed
FlockScopeTests 16 passed
TenantScopedLockTests 2 passed
CurrencyLockRaceTests / CurrencyLockSerializationTests 9 / 4 passed
FarmSettingsTests 54 passed
NamedRowProjectionTests 24 passed
RoleMatrixTests / AdminGatingTests 18 / 7 passed
BusinessRecordChronologyMigrationTests / ListChronologyTests 1 / 1 passed
Inventory and feed/water seeding, plus inventory/feed/water/expense rerun 3 passed

That is 169 distinct integration tests. After restoring mutations, the 13 selected contract/projection/matrix checks and both lock-order tests plus the chronology list test passed again. The initial parallel build exited with MSB4166 child-worker failures; serial builds with DOTNET_PROCESSOR_COUNT=2 -m:1 -nr:false passed. Docker tests ran through sg docker -c.

All 11 deliberate mutations failed for the intended assertion, rather than a compilation error:

Mutation Observed rejection
Inject IInventoryLotRepository into InventoryEndpoints.ListLots Adapter contract bypass
Add InventoryItem directly to InventoryItemDetails Contract entity leak
Add nested dictionary/list payload containing InventoryLot Contract entity leak
Add abstract payload with a derived case containing FeedUsage Contract entity leak through the derived case
Add unregistered Infrastructure read of InventoryLots Compatibility exception guard
Add unregistered Api read of InventoryLots Compatibility exception guard
Remove InventoryLotRepository from implementations Eight undeclared repository members
Remove the probe's General Inventory exception Undeclared probe read
Move FIFO lot acquisition before flock eligibility Test B expected 200 OK, received 422 UnprocessableEntity
Swap received/available lot quantities Lot-copy equality assertion
Swap water meter start/end Water-copy equality assertion

Evidence is alongside this report in 1027-astra-application.log, 1027-astra-integration.log, their TRX files, 1027-astra-final-build.log, 1027-astra-final-guards.log, 1027-astra-final-locks-chronology.log, and 1027-astra-mutation-*.log. The reproducible mutation script and summary are 1027-astra-mutations.py and 1027-astra-mutations.json.

Every throwaway mutation was reverted; final git status --short is empty. No commits, pushes or GitHub comments were made. These local checks do not replace full CI after #1026 merges and #1027 is retargeted to main.

Verdict: APPROVE WITH NIT for debd841d..ac25073a; no product defect found, with main-targeted CI still required after retargeting.

Base automatically changed from fix/1022-flock-lookup-untracked to main October 2, 2026 14:51
mforce added 4 commits October 2, 2026 14:52
… contract

InventoryEndpoints, WaterUsageEndpoints and SimulationDataSeeder now reach
General Inventory through IInventoryModule, which forwards to the existing
handlers and repositories and returns *Details records. Handler bodies are
untouched, so RecordFeedUsage keeps item lock, flock eligibility, then the
FIFO lots, in one transaction.

The ledger lists the contract and the five repositories as implementations,
registers the seeder's seven remaining Inventory reads for #858, and adds
the currency probe's Inventory row. Both probe rows now wait for #854,
because Commerce is the last of the probe's modules to get a contract.
@mforce
mforce force-pushed the refactor/855-inventory-contract branch from ac25073 to 208335a Compare October 2, 2026 14:58
@mforce

mforce commented Oct 2, 2026

Copy link
Copy Markdown
Owner Author

Rebased onto main after #1026 merged (582f8c8a) and retargeted to main. New head: 208335a5.

  • Doc fix from the Astra review's P3, in 208335a5: 855-inventory-contract.md now names the six handlers that keep peer calls. Item create, item update and purchase keep IAccountRepository. Feed usage, water usage and the water correction keep IFlockLookup. The two record handlers also keep IDailyEntryRepository. No code change.
  • Local run at the new head: Domain 495, Application 661, AppHost 10, Integration 1881, all green. Build clean.
  • CI: 18 of 18 checks pass; publish is skipped as on every PR. closingIssuesReferences shows [C] #514 slice 13: General Inventory contract and the flock-eligibility read #855.

@mforce
mforce merged commit 54a5f29 into main Oct 2, 2026
33 of 36 checks passed
@mforce
mforce deleted the refactor/855-inventory-contract branch October 2, 2026 15:11
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 13: General Inventory contract and the flock-eligibility read

1 participant