From 2d37be94748ece25ca91f33cec5b4ad09c35689c Mon Sep 17 00:00:00 2001 From: shum Date: Sat, 22 Aug 2026 11:20:31 +0000 Subject: [PATCH] plan: record a2 id type findings --- plans/2026-08-21-badges-web-checkout.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plans/2026-08-21-badges-web-checkout.md b/plans/2026-08-21-badges-web-checkout.md index d20c2e9993..058a52dbdd 100644 --- a/plans/2026-08-21-badges-web-checkout.md +++ b/plans/2026-08-21-badges-web-checkout.md @@ -1337,6 +1337,8 @@ Append here when a step contradicts this plan: the step id, what was wrong, and - **A1 — the migration entry is inserted chronologically, not appended.** The step said to append after `20260723_contact_request_rejection`, "whose current last entry is" that migration. Merging `master` into this branch added `20260813_auto_accept_group_invitations` after it, so `20260731_user_badges` now goes *between* the two, in both lists. Read the lists before editing rather than trusting a stated last entry; the same applies to any later step that appends to a shared list. - **A1 — `Store/Postgres/Migrations/chat_schema.sql` is NOT regenerated. OPEN, and a release blocker.** The step's Verify requires committing it. The development environment has no PostgreSQL binaries and no container runtime, so the `client_postgres` suite cannot run: it builds and links, then fails at `Connection refused` on port 5432. Nothing catches this automatically — `build-linux-postgres` in `.github/workflows/build.yml:389` is gated on `startsWith(github.ref, 'refs/tags/v')` and only builds the library; it never runs the Postgres schema-dump spec. **Regenerate and commit that dump on a machine with PostgreSQL before this branch merges.** A3 has the same Postgres verification leg and will hit the same wall. +- **A2 — three more `Int64` id fields need the same correction the step already mandates.** The step lists `BadgePurchase.paymentId`, `BadgePayment.paymentId` and `BadgeIssuance.issuanceId`. An audit of `badges-rpc.schema.json` against all four modules found the same defect in three further places, all `TEXT` columns typed as `Int64`: `StatementCreditType.SCCharge {chargeId}` (`Badges/Service.hs:163`, against `subscription_charges.charge_id TEXT NOT NULL PRIMARY KEY`), and `BadgeCharge.chargeId` and `BadgeCharge.paymentId` (`Badges/Types.hs:163-164`). `SCCharge` is the load-bearing one: it is a wire type whose `taggedObjectJSON` instance A2 writes, the schema declares `chargeId` as `string` (`badges-rpc.schema.json:230`), and Aeson would encode an `Int64` as a JSON number — so leaving it ships a payload that fails its own schema. Its sibling `SCPayment` already carries `Maybe InvoiceId`, a newtype over `Text`. A2 corrects all six. +- **`LedgerCreditType.CTPayment {invoiceId :: Int64}` and `CTCharge {chargeId :: Int64}` are wrong against their columns but are marked `-- confirmed`. OPEN — needs a decision, not a mechanical fix.** `invoices.invoice_id` and `subscription_charges.charge_id` are both `TEXT`. These are the DB-side twins of the wire types above, and `CTTransferIn {fromPurchaseId :: Maybe Int64}` beside them is correct because `from_purchase_id` really is `INTEGER`. A2 does not touch them: they sit outside its tagged-sum list, and altering a type someone marked confirmed is above a mechanical step. C1's `insertLedgerEntries` is the first code that would persist them, so this must be settled before C1. - **A1 — `chat_lint.sql` gains 5 fkey-index advisories, left unfixed by design.** The badge migration introduces unindexed foreign keys: `badge_invoices.offer_id`, `badge_invoices.price_id`, `badge_offers.price_id`, `badge_issuances.entry_id`, `users.shown_badge_id`. The lint output is committed literally rather than adding indexes, since index design is outside A1's scope and the repo has precedent for this (`9e000d6bc`). The first three point at rarely-mutated reference tables. The last two are the ones likely to matter under load — `badge_issuances.entry_id` for issuance lookup by ledger entry, and `users.shown_badge_id` for per-user badge display (C1's `getShownPurchase`). Decide on indexes for those two before release. ## 10. End-to-end verification