Design-quality review — 2026-07-10
Record of 2026-07-10, kept as written. The current truth is in the code, the ADRs and the live docs.
Method. Two-phase funnel, adversarially verified. Phase 1: an 8-scout wrong-model smell census (bug-churn shapes, parallel writers, containment accretion, hand-rolled-vs-standard, coupling, dead schema, FE patterns, fired-trigger deferrals) ranked 15 candidates. Phase 2: 9 deep model reviews on the top candidates + a cold-read attack set + a principles audit — every non-KEEP verdict re-verified by an adversarial pass with kill-criteria (pre-launch scoping, no-overengineering, atomic-first, industry parity). 17 agents, ~1.6M tokens, 438 tool reads. This doc is the curated program of record; per-area detail (current-model reconstructions with file:line, steelmanned alternatives, verifier reasoning) lives in the session transcript.
Headline. Design health is strong — 6 of 11 areas KEEP with verified, citable defenses; 3 confirmed net-deleting redesigns (tax-fact value type, one post-finalize collect policy, required engine deps) go to the owner; the two urgent money items are amending pushed commit 2b3069f before merge and the proration deferred-tax hotfix.
Census summary
Section titled “Census summary”The repo’s overall design health is strong — the census found no rot in the crown-jewel money semantics themselves (credits ledger, rating math, payment-settlement truth, RLS all came back clean or deliberately-layered) — but it exposed a consistent meta-pattern: where a model is wrong-shaped, this codebase contains rather than dissolves, accreting lints, mirror comments, backstop sweeps, and per-instance fixes. Three mega-clusters dominate: invoice-issuance duplication (6 of 8 scouts converged: ~6-14 hand-synced writers for headers/tax facts/collect/lines, with two live defects verified by the ranker — the threshold no-PM arm silently does nothing, and the proration writer drops TaxDeferredAt/TaxPendingReason/TaxErrorCode), simulated-time cohabitation (7 independent containment layers, ~23 fix commits, 4 ADR generations), and narrated-effects-instead-of-recorded-facts (attention banners lying twice in one week + 12 remaining post-commit Dispatch sites). The census also caught a live process failure: the ADR-084/085 recipe dissolution — cited to scouts as a completed precedent — is verified stranded on two unmerged branches while main still ships the superseded conformance-gate machinery, and the claimed ADR numbers are invisible to the documented ls docs/adr/ numbering check. Secondary findings: an undocumented build-vs-buy blind spot in infra (hand-rolled SMTP/MIME with 3 shipped bug classes, an 1107-line migration-runner hybrid mirroring library internals, Redis with triple fallback machinery), and a frontend that lags backend discipline by roughly a year (dual API/type systems, fetch-all-filter-client truncation bugs on money surfaces, zero tests in CI). Many acknowledged deferrals checked out sound (ADR-062 core, payment-settlement stack, coupon cut), which is itself a health signal — but three scouts independently produced evidence that ADR-062’s own count-based consolidation trigger may have quietly fired.
Crown jewels: IMPLICATED by the census: (1) billing cycle — hardest hit; candidates 1, 4, 9, and 14 all live in cycle/threshold/proration issuance, with two ranker-verified live defects (threshold no-PM collect gap at threshold_scan.go:879-891; proration writer missing 3 deferred-tax fields) plus the engine’s nil-collaborator accounting fallbacks (candidate 7); (2) test clocks — candidate 2 is a top-3 finding: 7 containment layers and a post-ADR-086 fix burst defending cohabitation of simulated rows in production tables. PARTIALLY implicated: payment truth — the settlement/reconciler stack itself was explicitly vetted by the containment scout as mapping 1:1 to real Stripe failure modes (prior audits hold), but two divergent ‘PM ready’ predicates (candidate 4) and the webhook-mirror ordering question are residual edges. CLEAN: credits ledger (event-sourced model unflagged by all 8 scouts; only adjacent CN tax-reversal recovery timing appears, inside candidate 13); rating (decimal-rate math ADR-045 unflagged; only its application is re-implemented across the 4 line builders, folded into candidate 4); RLS (zero findings on tenant isolation — the only tenant-adjacent item is the unenforced tenants.status lifecycle in candidate 15, which is not an isolation issue).
Fix-now items (small, shippable; status tracked here)
Section titled “Fix-now items (small, shippable; status tracked here)”| # | Item | Status |
|---|---|---|
| 1 | threshold collect / 2b3069f — DO FIRST, verified still unmerged — Amend fix/threshold-no-pm-collect before merge: replace its whole-block inv.Status==Finalized gate with draft → SetAutoChargePending(true)+return, and correct the commit message’s two false claims (there is no tax-retry collect path for amount_due>0; drafts were never synchronously charged — stripe.go:410 guard). As pushed, it trades the no-PM silence bug for stranding tax-pending threshold invoices that have a card: never enrolled, never swept, never charged, never dunned. | SHIPPED #427+#428 |
| 2 | proration tax hotfix — live money-liveness defect — Add TaxDeferredAt/TaxPendingReason/TaxErrorCode to subscription.ProrationTaxResult + adapter (api/adapters.go:831-844) + writer (subscription/handler.go:2315-2367); stamp BillingTimezone (h.tenantLoc already resolved at :2287); make the local-failure arm (:2226-2238) stamp TaxErrorCode=‘unknown’ + TaxDeferredAt=clock.Now(ctx) + reason. Today a proration invoice deferred during a transient Stripe Tax outage lands tax_error_code=‘’ and is permanently excluded from the auto-retry reconciler (postgres.go:2201/2439 filter) — draft, unfinalized, uncharged until manual rescue. Real-Postgres regression (ListPendingTaxRetry) with mutation-verify. Ship as commit 1 of the TaxFacts redesign or standalone. | SHIPPED #429 |
| 3 | webhooks — live user-visible bug — Promote payment_method.attached (payment/stripe.go:932) and payment_method.updated (:1192) into domain.KnownWebhookEventTypes + docs/webhooks.md + dashboard EVENT_GROUPS picker — explicit subscriptions to events Hardcount actually emits are currently rejected with ‘Hardcount never emits it’; add the ratchet arch test (shrink-only allowlist of the 10 non-tx Dispatch sites + every emitted literal ∈ catalog). | SHIPPED #432 (catalog+constants+docs+picker; ratchet test deferred to the dispatch item) |
| 4 | attention banner residue — Rewrite invoice_attention.go:762 hasEmail=true copy to disposition form (‘the engine sends a setup link at finalize… check the activity feed to confirm delivery’) — current ‘has been emailed’ is false under suppression/DLQ; fix resendSetupLink (invoice/handler.go:595-608) returning 200 {“status”:“sent”} on the silent no-email no-op → typed 4xx, and map ErrRecipientSuppressed to a coded 409/422 (today a bare 500); add the ~15-line past-tense-effect-verb regex gate over classifier Messages. | SHIPPED #434 (typed NotifyOutcome; resend 409; disposition copy) |
| 5 | cancel line builder — silent mis-billing risk — Fail-loud parity in buildCancelLineItems: propagate the ListItemChangesInPeriod error (engine.go:3786) and the GetPlan segment-drop (3799-3802) exactly as the cycle analog does (2306/2330) — a transient DB failure currently silently mis-bills a changed sub’s final invoice. | SHIPPED #435 |
| 6 | threshold/preview coupling — Stamp PlanID+BaseBillTiming on base_fee PreviewLines (json:“-”) and match on them in threshold_scan.go:430-494 — deletes the baseFeeIdx positional parallel-array contract between a display surface and a money writer. | open |
| 7 | playbook gate-7 conformance — Stop swallowing the 12 _ = SetAutoChargePending(...) store-write errors (engine.go:3175,3256,3275,3650,3654,4379,4383,5466,5501,5522; threshold_scan.go:848,887) — WARN-log minimum; a failed set(true) is a class-G liveness sink (invoice invisible to RetryPendingCharges forever). |
SHIPPED #433 (all 13 sites WARN-logged) |
| 8 | typed effect outcome retrofit — NotifyNoPaymentMethod returns done|failed|skipped(reason); delete the false ‘Engine logs the warning’ comment (adapters.go:698-699) and the premature ‘notifying customer’ Info (engine.go:3271); engine logs branch on the actual outcome. Prospective rule + this one retrofit only. | SHIPPED #434 |
| 9 | engine interface hygiene — Delete the 7 dead coordinator-interface methods + adapter pass-throughs (InvoiceWriter.CreateInvoice/CreateLineItem/ApplyCreditAmount/FindBaseInvoiceForPeriod/LatestThresholdPeriodEnd; SubscriptionReader.ClearPauseCollection incl. its false ‘Called by the cycle scan’ comment; UsageAggregator.AggregateForBillingPeriod). Subscription’s own prorationInvoiceCreatorAdapter FindBaseInvoiceForPeriod copy is live and stays. If the EngineDeps migration doesn’t start immediately, also ship the ~30-line MustBeFullyWired() boot check + log lines on the 3 unlogged creditGranter-nil arms (4555/4671/5169) as PR-0. | SHIPPED #437 (7 methods + 5 pass-throughs deleted) |
| 10 | threshold default parity — subscription/service.go:2425-2426’s ‘matches Stripe’ claim is verified false (Stripe default = anchor unchanged). Flip the omitted-field reset_billing_cycle PATCH default to false (free at zero customers, breaking later; also routes silent-default operators off the ADR-066 §5 {in_advance × reset=true} under-bill) — or if the true-default is kept deliberately, fix the three false parity claims (service.go:2425, handler.go:795, domain/subscription.go:312-314) and record the divergence in an ADR-066 amendment; reword the §5 trigger either way. | SHIPPED #436 (default flipped to false; FE matched; tests updated) |
| 11 | usage — kill the ‘silent’ half of the revenue-leak attack — Emit a counter + slog WARN when an ingested event’s timestamp falls inside an already-finalized period (usage/service.go:214-226 names this option); true-up policy itself stays a DP decision. | open |
| 12 | branch + doc hygiene — Merge or explicitly close stranded ADR-084 (b567df0) and ADR-085 (6b6d9fe) — main’s docs/adr jumps 083→086, inviting a silent duplicate number; refresh playbook §6 (frozen 2026-07-02, predates ADR-086/sim-class/HA doc); fix stale ADR-074 citations → ADR-077 (subscription/handler.go:2286/2681, invoice.go:226-227); write the metering scale-ceiling doc (current single-table design + a measured number + partitioning/rollup trigger); update memory notes (attention-banner ‘~8 unswept’ refuted; ‘no Jan31→Feb28 clamp’ stale — advanceAnchored clamps; ‘EventDispatcher has no *sql.Tx’ superseded). | open |
Redesigns — owner decision required (all verifier-CONFIRMED, net-deleting)
Section titled “Redesigns — owner decision required (all verifier-CONFIRMED, net-deleting)”- [SHIPPED #431] [#1 Tax-fact propagation — CONFIRMED] domain.TaxFacts value type (13 fields) embedded in domain.Invoice / billing.TaxOutcome / InvoiceTaxRetryUpdate; the five build-time writers collapse to one struct assignment. DELETES: ProrationTaxResult DTO, prorationTaxApplierAdapter, five 13-line hand-copies (~150 lines net), and the add-a-tax-field N-site hand-edit — converts a 5×-recurred field-drop bug class into compile errors, ahead of the named VIES/multi-registration field-add pressure. BLAST RADIUS: compile-time only — no schema, no wire change, no migration; ~137 mechanical test-literal lines across 33 files; ~1 day. ADR must record adoption AND the rejection of the composeInvoiceHeader half.
- [SHIPPED 2026-07-11 — scope adjudicated by site census, ADR-087] [#4 Post-finalize collect policy — CONFIRMED] Executed as: the verbatim-copied PIPELINE (PM resolve → charge / queue + notify) extracted to one Engine.collectAfterFinalize called by the four engine sites; per-site GATES (credit-apply/creditApplyOK, pause, status splits) deliberately stay — an 8-agent census (18 divergence dimensions) verified the full-policy unification hits three money traps (credit-apply adoption = silent revenue change; $0-MarkPaid arm entangled with cycle-advance; decline retry-owner collision needing error classification no site has). Three error-path drift bugs fixed in the extraction (resolver-error false email D10, silent reload vanish D7, silent notify loss D9). NOT folded (recorded in ADR-087 with named follow-ups): manual-finalize handler (deliberate no-flag decline semantics + verified drift: predicate skips PM-ID, no timeout, transient dead-end), proration (sweep-mediated by design; no-PM email gap needs sweep-side dedup), tax-retry (collectless by design; threshold pre-plants the flag). The threshold no-PM silence + 2b3069f draft-strand by-products were already absorbed by #427/#428. Original spec: One Engine.CollectAfterFinalize(opts{applyCredits, collectionPaused, inlineChargeAllowed}) called by all seven finalize paths (billOnePeriod, FinalizeOnCreateInvoice, BillFinalOnImmediateCancel, fireThreshold, proration with inline=false, manual finalize via a consumer-defined PostFinalizeCollector, and tax-retry auto-finalize as the seventh caller per the verifier). DELETES: four hand-copied ~40-line collect blocks (~150 lines), 2 of 3 PM-readiness predicates on the money path, and the undocumented draft-enroll-via-charge-error accident (made an explicit draft→enroll invariant); fixes the threshold no-PM silence, the 2b3069f draft-strand, and proration’s missing no-PM setup email as by-products; makes the banner’s ‘auto-charges once a card is attached’ true. Evidence the model breeds its class: the careful fix for drift #1 introduced drift #3. BLAST RADIUS: billing/engine.go, threshold_scan.go, invoice/handler.go, subscription/handler.go (one call), api adapters/router; no schema; behavior-preserving except the defect fixes; supersedes/absorbs 2b3069f; must explicitly resolve the enroll-on-decline divergence. Stripe/Lago parity: one collection pipeline.
- [SHIPPED #442–#446 — mechanism amended] [#7 Engine dependency model — CONFIRMED] Shipped as boot-fail-closed
Engine.MustValidate()(#442: reflection over all 24 interface collaborators, panics naming every nil, drift-guarded) instead of a construction-time EngineDeps struct: the engine and its peer services are mutually referential (invoiceSvc needs the engine as clock resolver; the engine needs invoiceSvc as voider), so the setters exist because of the cycle and stay; the guarantee (no partially-wired engine can serve) is identical. All money-divergent nil arms then deleted per-dep: #443 creditGranter (3 arms), #444 creditNoteAdjuster+invoiceVoider (7 arms incl. the no-tax-reversal net grant, the leave-for-dunning pair, bothe.invoices == nilelse-arms and the creditUnusedPrebill helper), #445 dunningStarter+dunningResolver (4 arms), #446 creditHeadroom (the disabled #276-278 cap). Remaining charger/paymentSetups/noPMNotifier collect-gate arms fold into redesign #4 (CollectAfterFinalize) rather than being deleted twice. Original spec for reference: EngineDeps struct with all 24 collaborators required; NewEngine validates and boot fails closed naming every nil field; delete all 14 setters and ~10 prod-dead money-divergent nil arms (silent $0 cancel/swap refund credit, net-grant-with-no-tax-reversal, disabled #276-278 headroom cap, credits-skip recreating a documented overcharge shape, haveSrc fallback, allocator fixture arm; delete — not loud-fail — the CommitTax/ReverseTax nil guards once resolver is required). The silent-nil shape already caused one production wrong-money incident (zero-tax invoices) and was only half-migrated. DELETES: the optional-collaborator concept + ~120 lines of nil-semantics prose + the entire future class, before the HA split’s second composition root arms it. BLAST RADIUS: internal/billing + router.go + ~90 package-local test sites; zero prod behavior change; 3-PR staging with temporary setter shims, wireBaseTax precedent. Fallback scope if heavy: money-set-required only.
KEEP verdicts — defenses to record (each becomes an ADR amendment / doc)
Section titled “KEEP verdicts — defenses to record (each becomes an ADR amendment / doc)”- [Test-clock sim isolation → ADR-086 amendment] Reject the per-clock RLS/connection-layer scope dimension: the livemode analogy breaks at its load-bearing point (livemode is caller-derived and boundary-bindable; sim-ness is data-derived — operators must see sim+real together, badged), so the three-valued scope default is unwinnable in either direction; Stripe (docs verified 2026-07-10) runs exactly Hardcount’s model — sandbox cohabitation, customer pin, filter-based sweep carve-out, delete-tears-down-customers.
- [Attention-banner copy → ADR-009 amendment] Banner Messages may assert only input-provable facts and engine dispositions, never past-tense per-invoice effects; recorded effects render exclusively from ledger-backed surfaces (invoice timeline, Sent emails) which cannot lie; ledger-derived banner facts (alt A) recorded decided-but-deferred (~50-line adoption path pre-exists) with trigger = a DP reports banner-vs-timeline triage confusion; copy stays in internal/domain because Message is public wire contract (Stripe-parity server-authored envelope).
- [Dunning creation → ADR-064 amendment, rejected alternative 4] In-tx run creation in the fail-transition tx loses on derivability (run existence/schedule is a pure function of committed invoice state — the sweep is the projection-maintainer, not a patch), forces a forbidden cross-domain tx surface, inverts reliability (dunning failure would 500 the Stripe webhook), kills config-change self-healing, and deletes zero resolve sites; the 463429a one-day revert condemns edge-hooked persisted projections without a level leg — it validates this model. Resolution deliberately shares the same triggered-primary + derived-backstop grammar.
- [Proration peers → ADR-048 second amendment] A mid-period upgrade against a PAID in_advance base must produce a peer invoice: finalized-invoice immutability + invoice-anchored tax reversal (per-invoice caps ⇒ allocation) outrank single-funder; immediate-charge is the B2B norm (Stripe always_invoice, Orb new-invoice arm, Lago code-verified second in-period invoice; 7/7 platforms keep the anchor); the funding-set machinery is small, gated by a lint rule (now forbidigo in .golangci.yml), and the bug class has not recurred since 2026-06-15.
- [Threshold two-mode → ADR-065/066 amendment] A threshold fire is not an early cycle close: keep-anchor is the industry’s primary semantics (Stripe’s documented default; Lago/Orb/Metronome’s only mode — Lago ships the same watermark); the carve-outs are mode-inherent (dual-subtotal survives reset=true or a steady max-meter refires per tick; early close converts split-billing into customer-adverse per-window peak billing); ADR-031’s prepaid in_advance base pins the reset=false boundary. Revisit trigger: a charge-without-invoice/applied-balance primitive.
- [Line builders → recorded in the collect-policy ADR] Separate cycle/cancel/on-create/preview/threshold builders are deliberate: five ADR-anchored divergent window semantics; unification = 6-flag monolith re-risking ~15 individually-annotated fixes (SOLID audit already ruled the shape deliberate); the builders’ real bug class dies via the two surgical fix-nows instead.
- [Cold-read grill → docs/dev/hard-questions.md] Consolidate the layered-defense story (durable-state idempotency key + unknown-classifier + reconciler; response-cache middleware + domain unique constraints; BeginTx livemode refusal + strict-mode panic + fan-out asserts; boot-time PgBouncer topology probe) plus the two written scope cuts with triggers (late-event true-up, RBAC/2FA) — ~1 hour, most answers already exist as code comments; high hiring-reader value.
Triggered backlog (do NOT build before the named trigger)
Section titled “Triggered backlog (do NOT build before the named trigger)”- [Outbound events in-tx migration] Trigger: first external webhook consumer (DP or self-hoster issue) depending on an event still in the post-commit set → migrate that event’s FAMILY in-tx via the owning store’s existing OutboxEnqueuer seam (per-domain, not big-bang). Permanent post-commit exceptions: payment/stripe.go fireEvent and payment/settlement.go escalatePaymentAnomaly (no business tx; outer at-least-once via Stripe dedup-after-handler redelivery). Secondary trigger: second engineer joins. Third adjudication — record in docs/adr/README.md:149 row.
- [Ledger-derived banner send-facts (alt A)] Trigger: a design partner reports operator confusion from banner-vs-timeline send-state disagreement → plumb the outbox row into AttentionContext for that one banner (~50 lines; EmailEventLister + lazy-read pattern pre-exist).
- [Uniform is_simulated flag on remaining sim-bearing tables] Trigger (ADR-086’s recorded deferral): an operator hits a non-money live-clock gate in a real simulation.
- [Stripe attach-clock-to-existing-customer parity gap] Trigger: a design partner asks (Hardcount is create-only, customer/service.go:230-242; record one line in ADR-086/030).
- [ForClock twin-collapse refactor] Trigger: a NEW sweep family grows the 21-twin count again — otherwise textually-disjoint money SQL stays.
- [invoice.voided in-tx enqueue] Trigger: next time the void path is touched anyway — ride UpdateStatusWithReversal’s tx callback (seam already wired), zero new seams.
- [Backpressure/ordering + load/perf playbook class] Trigger: production cutover or first DP traffic — posture doc FIRST (ha-readiness precedent), no build before.
- [FE component/E2E harness] Trigger: first DP onboarding; pure-logic money/time display-helper tests (effectiveNow.test.ts pattern) are sanctioned NOW under class J.
- [Release/versioning discipline] Trigger: first external self-hoster or public launch → cut v0.2.0 that week (promotion mechanics already documented in CHANGELOG).
- [Sweep-as-sole-collector (birth-stamped auto_charge_pending)] Trigger: HA build forces re-examining inline charges — natural phase 2 of the collect policy only then (charge-latency parity regression today).
- [Security posture: CSRF token, 2FA, RBAC enforcement] Trigger (per ADR-081, reaffirmed by attack #9): first DP security questionnaire or any hosted/internet-exposed deployment.
Principle amendments (for the playbook / working rules)
Section titled “Principle amendments (for the playbook / working rules)”- [Playbook class J — rendered/asserted truth] Every surface that ASSERTS a money/effect fact (banner copy, log line, email, dashboard aggregate, README) derives it from the authoritative record or states engine behavior — never re-derives heuristically. Fix-kinds: plumb-the-value / stamp-at-write (m0142 pattern) / required-anchor type (ADR-086 §4 pattern). Detection: ‘every RENDERER of the state’ becomes item 6 of the §2 site-set. Test-lock: pure-logic display-helper tests (effectiveNow.test.ts seed). Closes the class (7+ recurrences) that structurally escaped the write-side-only site-set.
- [Containment trip-wire — sequences mechanized-invariant vs complexity-accretion] On a recurring bug shape, first ask what model change deletes the class; mechanize a gate only when the minimal complete model still permits the mistake AND the language can’t express the invariant (Go lint = legitimate; TS branded type = preferred dissolve). TRIP-WIRE: any PR shipping a SECOND gate/backstop/sweep-twin/honesty-flag on one shape must contain a written one-paragraph model trial in its ADR, pass or fail. (Three in-repo cascades — TZ ×4 ADRs, FE-time ×4 PRs, recipe gate + 2 stranded ADRs — would each have collapsed earlier.)
- [no-silent-fallbacks extension — typed effect outcomes] Effect interfaces return done|failed|skipped(reason); returning nil for a skipped effect is forbidden; a skipped effect leaves a queryable trace; no surface may assert an effect whose outcome it cannot read. PROSPECTIVE + exactly one retrofit (NotifyNoPaymentMethod) — explicitly NOT a repo-wide effect-interface migration.
- [no-belt-and-suspenders clarification] One LOAD-BEARING mechanism per requirement: a latency-primary over a correctness-backstop is not redundancy — two mechanisms both claiming correctness for the same failure mode is. Records ADR-064 (and the resolve grammar) as conforming, ending the doctrine ‘contradiction’.
The cold-read attack set (interview/due-diligence prep)
Section titled “The cold-read attack set (interview/due-diligence prep)”-
- ‘Your meter is an OLTP Postgres table — where does it die? Show me a number.’ — HOLDS with a named gap: deliberate self-host-simple design, bounded period+customer+meter scans, CONCURRENTLY-built indexes; but no written throughput ceiling exists. Fix = doc + a pgbench number (in fix_now), not Kafka/ClickHouse.
-
- ‘Late usage events after finalize are silently unbilled — a revenue leak in your flagship path.’ — HOLDS on policy (documented, industry-standard accept; immutable finalized invoices; Backfill exists), GAP on observability until the counter+WARN ships (fix_now); true-up policy is an honest named DP decision.
-
- ‘Crash between PaymentIntent-create and your DB write; justify the updated_at-derived idempotency key.’ — HOLDS, showcase-grade: durable-state key replay + unknown-vs-failed classification + reconciler ordered before auto-charge + webhook settle via PI metadata + ADR-059 in-flight guards. Volunteer the residual: >24h-stale retry needs webhook AND reconciler both dead for 24h; updated_at writer-set is closed by discipline, not a type.
-
- ‘RLS via a self-service GUC is not a security boundary — one SQLi and app.bypass_rls=on.’ — HOLDS only with the honest framing said unprompted: it contains APPLICATION bugs (forgotten WHERE tenant_id), not SQLi (parameterized queries are that defense); FORCE RLS + policy-enumeration test + non-superuser hardcount_app + enumerable bypass sites.
-
- ‘Field-by-field tax stamping across N writers breeds invoices that lie about tax — where is the machine?’ — LANDED (the one attack that found a real gap by the repo’s own doctrine): confirmed live defect on the proration writer. Answer after this program: hotfix shipped + domain.TaxFacts makes drop-a-field a compile error.
-
- ‘Livemode defaults to LIVE when unset.’ — HOLDS via the layered recital: BeginTx REFUSES TxTenant without explicit livemode, HARDCOUNT_LIVEMODE_STRICT panics in CI, WithRequiredLivemode at fan-out, live-then-test scheduler split. Residual to volunteer: non-DB Stripe-client resolution sits outside the BeginTx gate.
-
- ‘Session advisory locks + PgBouncer = silently halted billing.’ — HOLDS, showcase-grade: boot-time topology probe (backend-PID stability + cross-conn lock invisibility) refuses transaction-mode poolers, with the honest caveat documented in-code.
-
- ‘Your idempotency middleware caches responses, it doesn’t dedupe effects — and it fails open.’ — HOLDS only as the two-layer story told as one design: Stripe-shape middleware (reserve-before-act, fingerprint 422, 500s cached) + domain unique constraints/CAS leases underneath, so crash-after-commit re-runs INTO a constraint; fail-open is documented+metered, conflict path fails closed.
-
- ‘SameSite=Lax is your whole CSRF story; no 2FA; every teammate has full permissions.’ — DEFENSIBLE-WITH-CAVEAT (weakest of the ten; the prepared answer is scoping, not tech): self-host-in-VPC deployment model, ADR-081 written cut with roles recorded for the future split, trigger = first DP security questionnaire or hosted deployment. Unprepared (‘we have RLS’) would be damaging.
-
- ‘O(n) ledger SUM under an advisory lock, lock order in a comment, and a 5.8k-line engine.go.’ — HOLDS at the actual cardinality (invoice-cadence entries; empty-ledger phantom solved properly; real-Postgres concurrency tests; balance_after O(1) reads) + the #402 god-method decomposition and arch-test boundaries answer the file-size jab; cite the numbers or it sounds evasive.
Verdict
Section titled “Verdict”Overall design health is genuinely strong: six of eleven areas survived adversarial review outright, several with showcase-grade answers already written into the code (PgBouncer boot probe, charge idempotency layering), and every KEEP now has a verified defense worth committing as an ADR amendment so none of it gets re-litigated. The bad news is concentrated and money-shaped: one live defect (deferred-tax proration invoices permanently invisible to the auto-retry reconciler) and one about-to-ship regression (2b3069f’s Finalized gate strands tax-pending threshold invoices) — do those two first, today; 2b3069f is verified still unmerged so the amendment is free. The three confirmed redesigns share one signature — hand-copied fact/effect propagation breeding drift — and all three net-delete code with no schema or behavior change, so they are unusually cheap for their payoff; recommended order is TaxFacts (~1 day, absorbs the hotfix), then CollectAfterFinalize (absorbs 2b3069f and fixes three defects as by-products), then EngineDeps (3 staged PRs, should land before any HA second-composition-root work). The strongest single pattern across all eleven reviews: censuses kept proposing structural machinery (RLS scope dimensions, unified builders, tx-required-only signatures) where the verified failures were actually one-string, one-field, or one-block copy drift — the razor held, and the refuted appendix is the proof to cite next time. The principles corpus needs its three amendments (rendered-truth class J, containment trip-wire, typed effect outcomes) because the repo demonstrably dissolves wrong models but 2-4 containment layers too late; these are doc-level plus one small interface retrofit, not a build program. Nothing in the triggered backlog should be built now — every item has a precise, checkable trigger, and writing those triggers down is what converts scope cuts from skeletons into decisions an expert evaluator respects.