From fa19cb0306e373d194844722afd1209b4947f54d Mon Sep 17 00:00:00 2001 From: mforce <> Date: Wed, 12 Aug 2026 19:17:36 -0700 Subject: [PATCH 1/3] chore(infra): record why AuditEvents is not time-partitioned (#505) Parks the #494 review's partitioning question as a decision record instead of leaving it to be re-litigated: the dominant provenance read carries no date predicate, so monthly partitioning would strictly worsen that query's index lookup, add a new INSERT-fails failure mode inside the same transaction as the audited mutation (#93), and drag in a migration + schema regen + sim-harness pass for a table nowhere near needing it (~20 MB/year at 100 flocks). If it ever bites, partition by AccountId instead, once genuinely multi-tenant. --- AGENTS.md | 1 + .../505-audit-events-no-time-partition.md | 53 +++++++++++++++++++ docs/decisions/README.md | 1 + 3 files changed, 55 insertions(+) create mode 100644 docs/decisions/505-audit-events-no-time-partition.md diff --git a/AGENTS.md b/AGENTS.md index 07430c0e..a7cd6263 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,6 +50,7 @@ dotnet test Cluckwork.sln # 688 tests as of 2026-07; integratio - **Base reference data via guarded raw-SQL migrations (#283).** The default account, four assignable roles, default egg grades, and packed-unit conversions are **static reference data baked into the migrations** as hand-written `migrationBuilder.Sql` with `WHERE NOT EXISTS` guards — **never `HasData`/`InsertData`** (which key on the PK and would either collide or emit `UpdateData`/`DeleteData` that silently reverts the farm's own edits). No runtime seeder, no `Seed:*`. The **grades** guard is whole-set (empty catalog, because grades are user-renamable); roles/conversions/account are **per-key** (keys aren't user-mutable). Pinned by `MigrationSecurityReviewTests` + `BaseReferenceDataMigrationTests`. **Regenerate these into `HasData` and a later model-diff reverts a renamed grade.** → [`docs/decisions/283-migrations-base-provisioning.md`](docs/decisions/283-migrations-base-provisioning.md) - **Migrations: `InitialCreate` frozen, one migration per change (#407).** `Persistence/Migrations/` is one squashed `InitialCreate` (recorded id `20260801190854_InitialCreate`); it is **frozen and never regenerated**, and **every schema change gets its own `dotnet ef migrations add`**. EF never re-runs an applied migration, so a column **hand-folded into `InitialCreate` silently does not exist** on any booted DB — it surfaces as broken behaviour (a failed login, #399), not a migration error. `InitialCreate` also carries four un-regenerable `lower("Name")` expression indexes + the base-reference SQL; regenerating mints a new timestamp that desynchronises `__EFMigrationsHistory` everywhere. Pre-squash or pre-#407 dev DBs can't migrate forward — drop and recreate. `MigrationSecurityReviewTests` freezes the id + a portable operation digest. → [`docs/decisions/407-migration-freeze.md`](docs/decisions/407-migration-freeze.md) - **Schema docs are generated, committed, and regenerated with every migration (#417).** `docs/schema/` (mermaid ERD + full catalog: every column, constraint, and index — including the raw-SQL expression indexes and partial predicates EF reflection can't see) is produced by `tools/schema-docs/generate.sh` from an ephemeral migrated Postgres via digest-pinned tbls. **Every PR that adds a migration runs the generator and commits the result in the same PR** — CI's `build-and-test` runs `generate.sh --check` (byte-diff against a fresh generation) and fails a stale PR. Generated files under `docs/schema/` are **never hand-edited**, and a conflict there after a rebase is resolved by regenerating, never by hand-merging. Pinned from three sides by `SchemaDocsTests` (postgres-pin uniformity across every tracked file, no environment leakage, completeness against the live catalog). → [`docs/decisions/417-schema-docs.md`](docs/decisions/417-schema-docs.md) +- **`AuditEvents` is not time-partitioned, on purpose (#505).** The dominant read (`GetProvenanceAsync`, run on every Flocks/Egg grades/Daily entries/Sales/Expenses page load) filters on `AccountId`+`EntityType`+`EntityId` with **no date predicate**, so partitioning by month would convert one index lookup into one per partition — strictly worse, forever — plus a PK change, a new INSERT-fails-without-next-month's-partition failure mode inside the same transaction as the audited mutation (#93), and no partition-maintenance job to prevent it. At current growth (~20 MB/year at 100 flocks) this is not close to needed; if it ever is, partition by `AccountId` (the column the query actually filters on) once genuinely multi-tenant, not by time. → [`docs/decisions/505-audit-events-no-time-partition.md`](docs/decisions/505-audit-events-no-time-partition.md) - **Seed / simulation data is never boot-seeded (#280, #284, #279).** Run it explicitly against an **already-base-seeded, non-Production** database: `ASPNETCORE_ENVIRONMENT=Development dotnet Cluckwork.Api.dll seed --profile demo|simulation` (an unset env → Production → blocked). The verb migrates, seeds, then exits — **authoritative** (ignores config) and **fail-loud** (a real exit code, never a silent no-op). `simulation` additionally records a durable date anchor + completion marker, so a clean re-run — even across a UTC-midnight rollover — converges to `AlreadySeeded`; a polluted account fails the exact-count validation **closed**. → [`docs/decisions/280-seed-and-simulation.md`](docs/decisions/280-seed-and-simulation.md) - **Break-glass recovery (#265):** `recover-admin` is a second one-off CLI verb on the same binary, same run-then-exit shape as `seed` — but deliberately **NOT** environment-gated (it must work against a real Production database): `dotnet Cluckwork.Api.dll recover-admin --email [--account ] [--reason ]`. For a locked-out account (the sole-Owner-lost-password case, no email/SMTP reset path) it, in one transaction, resets to a **freshly generated** temporary password (never one passed on the command line), rotates the security stamp, revokes every refresh token, and writes a conspicuous `User.BreakGlassReset` audit row carrying `--reason`. The temp password is printed to **stdout only** (never the logger/OTLP) and exit `0`; failures go to stderr with exit `1` and change nothing. `AdminRecoveryService` orchestrates; `IdentityProvider.BreakGlassResetAsync` shares the reset/revoke core with `SetUserPasswordAsync`. Full procedure + verification drill: `docs/runbooks/break-glass-account-recovery.md`. - **First-run admin provisioning: `bootstrap-admin` (#283).** `dotnet Cluckwork.Api.dll bootstrap-admin --email ` (run-then-exit, always available in Production) migrates, then — **only if the default account has no Owner** — creates one with a **freshly generated password (stdout only, never the logger/OTLP)** and `MustChangePassword=true`; a re-run is a **silent no-op**. While the flag is set the JWT carries `must_change_password`, `MustChangePasswordMiddleware` 403s every endpoint except `auth/change-password`+`auth/logout` (before `UseAuthorization` and before idempotency), and the SPA renders **Set your password**. Any successful reset clears the flag (one invariant in `IdentityProvider`). **Deliberately a separate credential type from #265/#308** — never conflated. → [`docs/decisions/283-first-run-admin-provisioning.md`](docs/decisions/283-first-run-admin-provisioning.md) diff --git a/docs/decisions/505-audit-events-no-time-partition.md b/docs/decisions/505-audit-events-no-time-partition.md new file mode 100644 index 00000000..32195bae --- /dev/null +++ b/docs/decisions/505-audit-events-no-time-partition.md @@ -0,0 +1,53 @@ +# `AuditEvents` is not time-partitioned (#505) + +Parked from the #494 review, so the reasoning is not rediscovered. + +## Question + +`AuditEvents` is append-only with no purge sweep, so it only grows. #494 +added ~1 row per daily entry per day (`DailyEntry.Create` + `DailyEntry.Submit`), +on top of every other audited mutation. Should we get ahead of it by +range-partitioning the table by month? + +## Answer: no, and not merely "not yet" + +Time partitioning is the wrong axis for this table. Partition pruning only +helps a query that filters on the partition key. The dominant read — +`AuditEventRepository.GetProvenanceAsync`, run on every Flocks / Egg grades / +Daily entries / Sales / Expenses page load — filters on `AccountId` + +`EntityType` + `EntityId` and carries **no date predicate at all**. +Partitioning by month converts one index lookup on +`IX_AuditEvents_AccountId_EntityId` into one lookup *per partition*, getting +worse every month. It would degrade the exact query #494 introduced. + +Three further costs, none of them the deciding one: + +1. Postgres requires the partition key in every unique constraint, so the PK + moves from `Id` to `(Id, OccurredAtUtc)`. Converting an existing table + means create-new + copy + rename, not an `ALTER`. +2. Partition maintenance becomes a new failure mode with teeth. If next + month's partition does not exist, `INSERT` fails; audit writes share the + transaction with the change they record (#93), so *every audited + mutation* fails with it. No partition-maintenance job runs today. +3. It drags in a migration (#407), a `docs/schema/` regeneration (#417), and + a sim-harness pass (#370) — for a table that does not need any of them yet. + +## Current size, for the record + +~2 rows per flock per day. 10 flocks → ~7.3k rows/year; 100 flocks → ~73k +rows/year, roughly 20 MB/year. Not a problem at any plausible farm size. + +## If it ever does bite, in order + +1. **Archive or roll up** rows older than the retention the farm actually + needs. +2. **Partition by `AccountId`** — the column the provenance query *does* + filter on, so pruning would apply. Only meaningful once the deployment is + genuinely multi-tenant. + +## Separate, and real + +The Audit page pages with `OFFSET`, which degrades on deep pages as the +table grows. Independent of partitioning and of #494; worth keyset +pagination on its own merits if it ever shows up in practice — no issue +filed for it yet. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 036715e9..a2089676 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -33,6 +33,7 @@ follow the `→` link from the `AGENTS.md` bullet to get here. | [A write-contract change must update its non-CI callers (#394)](394-write-contract-callers.md) | AGENTS · Conventions | | [Production logs: compact JSON on stdout (#404)](404-production-logs.md) | AGENTS · Conventions | | [Generated PostgreSQL schema documentation (#417)](417-schema-docs.md) | AGENTS · Conventions | +| [`AuditEvents` is not time-partitioned (#505)](505-audit-events-no-time-partition.md) | AGENTS · Conventions | | [Writing a guard (a test that asserts an invariant)](407-writing-a-guard.md) | AGENTS · Writing a guard | | [CI security gates, lock-file healing, Dependabot, action pinning (#146)](146-ci-security-gates.md) | AGENTS · CI security gates | | [Releases and image publishing — internals (#351)](351-releases.md) | AGENTS · Releases · and `README.md` | From bbed29ecf434f4176b8328e0dbfc3e9647c209ff Mon Sep 17 00:00:00 2001 From: mforce <> Date: Wed, 12 Aug 2026 19:21:53 -0700 Subject: [PATCH 2/3] docs: soften an overstated partition-cost claim in the #505 record (#505) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local pi review: "strictly worse, forever" overstated the cost — a partitioned scan reads the same total data, so the degradation is per-partition constant-factor overhead (planner cost, tuple routing), not a data-read increase. The decision doc's own wording ("getting worse every month") was already precise; only the AGENTS.md compressed bullet needed the fix, kept consistent with it. --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index a7cd6263..9848d903 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,7 +50,7 @@ dotnet test Cluckwork.sln # 688 tests as of 2026-07; integratio - **Base reference data via guarded raw-SQL migrations (#283).** The default account, four assignable roles, default egg grades, and packed-unit conversions are **static reference data baked into the migrations** as hand-written `migrationBuilder.Sql` with `WHERE NOT EXISTS` guards — **never `HasData`/`InsertData`** (which key on the PK and would either collide or emit `UpdateData`/`DeleteData` that silently reverts the farm's own edits). No runtime seeder, no `Seed:*`. The **grades** guard is whole-set (empty catalog, because grades are user-renamable); roles/conversions/account are **per-key** (keys aren't user-mutable). Pinned by `MigrationSecurityReviewTests` + `BaseReferenceDataMigrationTests`. **Regenerate these into `HasData` and a later model-diff reverts a renamed grade.** → [`docs/decisions/283-migrations-base-provisioning.md`](docs/decisions/283-migrations-base-provisioning.md) - **Migrations: `InitialCreate` frozen, one migration per change (#407).** `Persistence/Migrations/` is one squashed `InitialCreate` (recorded id `20260801190854_InitialCreate`); it is **frozen and never regenerated**, and **every schema change gets its own `dotnet ef migrations add`**. EF never re-runs an applied migration, so a column **hand-folded into `InitialCreate` silently does not exist** on any booted DB — it surfaces as broken behaviour (a failed login, #399), not a migration error. `InitialCreate` also carries four un-regenerable `lower("Name")` expression indexes + the base-reference SQL; regenerating mints a new timestamp that desynchronises `__EFMigrationsHistory` everywhere. Pre-squash or pre-#407 dev DBs can't migrate forward — drop and recreate. `MigrationSecurityReviewTests` freezes the id + a portable operation digest. → [`docs/decisions/407-migration-freeze.md`](docs/decisions/407-migration-freeze.md) - **Schema docs are generated, committed, and regenerated with every migration (#417).** `docs/schema/` (mermaid ERD + full catalog: every column, constraint, and index — including the raw-SQL expression indexes and partial predicates EF reflection can't see) is produced by `tools/schema-docs/generate.sh` from an ephemeral migrated Postgres via digest-pinned tbls. **Every PR that adds a migration runs the generator and commits the result in the same PR** — CI's `build-and-test` runs `generate.sh --check` (byte-diff against a fresh generation) and fails a stale PR. Generated files under `docs/schema/` are **never hand-edited**, and a conflict there after a rebase is resolved by regenerating, never by hand-merging. Pinned from three sides by `SchemaDocsTests` (postgres-pin uniformity across every tracked file, no environment leakage, completeness against the live catalog). → [`docs/decisions/417-schema-docs.md`](docs/decisions/417-schema-docs.md) -- **`AuditEvents` is not time-partitioned, on purpose (#505).** The dominant read (`GetProvenanceAsync`, run on every Flocks/Egg grades/Daily entries/Sales/Expenses page load) filters on `AccountId`+`EntityType`+`EntityId` with **no date predicate**, so partitioning by month would convert one index lookup into one per partition — strictly worse, forever — plus a PK change, a new INSERT-fails-without-next-month's-partition failure mode inside the same transaction as the audited mutation (#93), and no partition-maintenance job to prevent it. At current growth (~20 MB/year at 100 flocks) this is not close to needed; if it ever is, partition by `AccountId` (the column the query actually filters on) once genuinely multi-tenant, not by time. → [`docs/decisions/505-audit-events-no-time-partition.md`](docs/decisions/505-audit-events-no-time-partition.md) +- **`AuditEvents` is not time-partitioned, on purpose (#505).** The dominant read (`GetProvenanceAsync`, run on every Flocks/Egg grades/Daily entries/Sales/Expenses page load) filters on `AccountId`+`EntityType`+`EntityId` with **no date predicate**, so partitioning by month would convert one index lookup into one per partition — added per-partition overhead that only grows as months pass, for no pruning benefit — plus a PK change, a new INSERT-fails-without-next-month's-partition failure mode inside the same transaction as the audited mutation (#93), and no partition-maintenance job to prevent it. At current growth (~20 MB/year at 100 flocks) this is not close to needed; if it ever is, partition by `AccountId` (the column the query actually filters on) once genuinely multi-tenant, not by time. → [`docs/decisions/505-audit-events-no-time-partition.md`](docs/decisions/505-audit-events-no-time-partition.md) - **Seed / simulation data is never boot-seeded (#280, #284, #279).** Run it explicitly against an **already-base-seeded, non-Production** database: `ASPNETCORE_ENVIRONMENT=Development dotnet Cluckwork.Api.dll seed --profile demo|simulation` (an unset env → Production → blocked). The verb migrates, seeds, then exits — **authoritative** (ignores config) and **fail-loud** (a real exit code, never a silent no-op). `simulation` additionally records a durable date anchor + completion marker, so a clean re-run — even across a UTC-midnight rollover — converges to `AlreadySeeded`; a polluted account fails the exact-count validation **closed**. → [`docs/decisions/280-seed-and-simulation.md`](docs/decisions/280-seed-and-simulation.md) - **Break-glass recovery (#265):** `recover-admin` is a second one-off CLI verb on the same binary, same run-then-exit shape as `seed` — but deliberately **NOT** environment-gated (it must work against a real Production database): `dotnet Cluckwork.Api.dll recover-admin --email [--account ] [--reason ]`. For a locked-out account (the sole-Owner-lost-password case, no email/SMTP reset path) it, in one transaction, resets to a **freshly generated** temporary password (never one passed on the command line), rotates the security stamp, revokes every refresh token, and writes a conspicuous `User.BreakGlassReset` audit row carrying `--reason`. The temp password is printed to **stdout only** (never the logger/OTLP) and exit `0`; failures go to stderr with exit `1` and change nothing. `AdminRecoveryService` orchestrates; `IdentityProvider.BreakGlassResetAsync` shares the reset/revoke core with `SetUserPasswordAsync`. Full procedure + verification drill: `docs/runbooks/break-glass-account-recovery.md`. - **First-run admin provisioning: `bootstrap-admin` (#283).** `dotnet Cluckwork.Api.dll bootstrap-admin --email ` (run-then-exit, always available in Production) migrates, then — **only if the default account has no Owner** — creates one with a **freshly generated password (stdout only, never the logger/OTLP)** and `MustChangePassword=true`; a re-run is a **silent no-op**. While the flag is set the JWT carries `must_change_password`, `MustChangePasswordMiddleware` 403s every endpoint except `auth/change-password`+`auth/logout` (before `UseAuthorization` and before idempotency), and the SPA renders **Set your password**. Any successful reset clears the flag (one invariant in `IdentityProvider`). **Deliberately a separate credential type from #265/#308** — never conflated. → [`docs/decisions/283-first-run-admin-provisioning.md`](docs/decisions/283-first-run-admin-provisioning.md) From 95ef99875d452932f00526d071badecd2cad76ac Mon Sep 17 00:00:00 2001 From: mforce <> Date: Wed, 12 Aug 2026 19:24:33 -0700 Subject: [PATCH 3/3] docs: caveat the #505 record's archival suggestion against breaking provenance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit codex review of #522: "archive or roll up" as a future mitigation was underspecified enough to be unsafe — GetProvenanceAsync reads CreatedBy*/ MadeOfficialAtUtc straight from the original audit events for as long as the record stays visible, so archiving by age alone could silently blank a still-visible record's History column. Now says so explicitly. --- docs/decisions/505-audit-events-no-time-partition.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/decisions/505-audit-events-no-time-partition.md b/docs/decisions/505-audit-events-no-time-partition.md index 32195bae..f693439f 100644 --- a/docs/decisions/505-audit-events-no-time-partition.md +++ b/docs/decisions/505-audit-events-no-time-partition.md @@ -40,7 +40,15 @@ rows/year, roughly 20 MB/year. Not a problem at any plausible farm size. ## If it ever does bite, in order 1. **Archive or roll up** rows older than the retention the farm actually - needs. + needs — but not naively. `GetProvenanceAsync` derives `CreatedBy*` from + the record's original `*.Create` event and `MadeOfficialAtUtc` from its + promotion event (submit/confirm), for as long as the record itself stays + visible — which for a flock or a sales order can be indefinitely. Moving + or aggregating those specific rows out of `AuditEvents` while the record + they describe is still shown would silently blank its History column. + Any archival design must keep provenance-source events queryable (or + copy their fields forward) for every record still visible, not just + drop/roll up by age. 2. **Partition by `AccountId`** — the column the provenance query *does* filter on, so pruning would apply. Only meaningful once the deployment is genuinely multi-tenant.