Repository navigation
feat(ledger): immutable BRL ledger core - #354
Merged
Merged
Conversation
sonikro
marked this pull request as ready for review
October 11, 2026 03:13
sonikro
force-pushed
the
feat/ledger-core
branch
from
October 13, 2026 00:29
da29115 to
a18e8a6
Compare
The repository names modules after their primary export: PascalCase for classes and types (Region.ts, SQLiteServerRepository.ts), camelCase for function modules. The ledger stack shipped snake_case files, which is the convention written down in AGENTS.md and the code-style skill but not the one the codebase actually follows. Rename the ledger domain, repository, service and use-case modules, and update the core, domain and provider barrel exports plus every relative import that points at them. No behaviour change. The ledger_constants and seed_ledger_opening modules are left untouched here: they are removed by later work in this stack.
The monthly Discord bot infrastructure cost was a constant in source (ledger_constants.ts), so changing it meant a code change and a release, and the report formatter carried its own copy of the same literal. Move the value into config/default.json under a top-level money block as a decimal string plus its currency, and parse it once in a new core MoneyConfig module that exposes the amount in minor units. The settlement use case now takes the parsed config as a required dependency instead of an optional cost override, and the monthly report formatter will read it from the same place. The parser rejects a missing block, a non-decimal amount and a currency that is not BRL or USD. A segment that fails to settle is now reported as failed rather than skipped, so a permanently failing month can no longer be announced as already recorded. The settlement date range moves to the UTC basis of the entry it writes, so the queried period matches the recorded one.
The ledger migration is unreleased: it exists only on this unmerged stack and has never run in any environment, so it can still be corrected in place. The balance column held centavos while being named amount_brl, which reads as a BRL amount and invites a unit mistake in every later query. Rename it to amount_brl_cents, require created_at, and constrain the enumerated columns (type, currency_original, fx_source) so a typo cannot reach the ledger. An explicit integer check keeps a fractional value from being stored as a real in the cents column. Cover the down/up round trip, the integer storage and each rejected value.
The donation dedupe key embedded the occurrence timestamp, which is generated at call time, so every call produced a distinct key and the unique index could never catch a repeated registration. The use case also never consulted the repository before inserting, so a duplicate surfaced as a raw SQLite error. Drop the timestamp from the key, check it before doing any I/O, and reject an identical donation with a message naming the amount, currency and source so the owner knows to vary the source when a second donation really is intended. The exchange rate is only fetched once the donation is known to be new. The repository now translates the dedupe-key unique violation into a UserError as well, so a double submission racing past the pre-check still produces a clear message instead of a database error. Other constraint violations are rethrown untouched.
The opening balance is a one-time decision an owner makes, not a deploy-time step, and exposing it as an npm script meant anyone with shell access and a copy of the database could run it repeatedly. The owner-only opening command added later in this stack replaces it, with the ledger's own emptiness as the one-shot guard.
AGENTS.md and the code-style skill both told agents to use snake_case file names, but the codebase names class and type modules in PascalCase (Region.ts, SQLiteLedgerRepository.ts) and function modules in camelCase (maskIp.ts, commandErrorHandler.ts); snake_case appears only in migrations. That written rule is why the ledger stack shipped snake_case files, so it is the root cause rather than a detail. State the convention the repository actually follows.
SQLite writes CURRENT_TIMESTAMP as a UTC string without a zone, and parsing it directly treated it as host-local time, so a created_at timestamp read back three hours off on a non-UTC host. Nothing consumes it yet, which is exactly why it would have gone unnoticed until something did. The money config parser also accepted an amount whose minor-unit result is not a safe integer. Reject it, so a nonsense config value fails loudly at startup instead of producing a corrupted cost.
The default five second test timeout is not enough for the suite under parallel load: the HTTP tests, the timer test and the file-backed migration round trip each exceed it intermittently, so CI goes red on tests that pass in isolation. The failures are genuine timeouts caused by machine load rather than hanging logic, and a longer ceiling changes no assertion: a hanging test still fails, just later.
Every case but the migration round trip was creating a temporary database on disk and removing it afterwards, which made this the heaviest source of real filesystem I/O in the suite for no benefit: the cases assert repository behaviour, not persistence. Keep the file-backed database only for the down/up round trip, which is the one case actually about persistence. Same cases, same assertions.
This was referenced Oct 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack 1/4 — Ledger core (base:
main)An immutable BRL ledger that replaces manual
#moneyaccounting. Every entry isappend-only; the balance is a sum over integer centavos, so no float can reach it.
What this PR adds
ledger_transactionstable (Knex migration): integeramount_brl_cents,CHECKconstraints on
type,currency_original,fx_sourceand on the cents column being aninteger, a unique
dedupe_key, and a requiredcreated_at.LedgerTransaction) with the money boundaries (toBrlCents,toMajorUnits), thededupe-key builders and the settlement date helpers.
LedgerRepository(core interface) andSQLiteLedgerRepository(Knex/SQLite).RegisterDonation,RegisterCost(idempotent by dedupe key),GetBalance,MonthlyCostSettlement(per-region segments plus the bot infrastructure segment, isolatedper segment, counting inserted / already-recorded / no-cost / failed separately).
config/default.json(
money.fixedMonthlyInfrastructureCost), parsed once by a new coreMoneyConfigmodule andexposed through
ConfigManager. It is no longer a constant in source, and the parser rejectsa missing block, a non-decimal amount, a currency that is not BRL or USD, and an amount whose
minor-unit value is not a safe integer.
type modules), and
AGENTS.mdplus thecode-styleskill were corrected: they had beentelling agents to use snake_case, which is what produced the wrong names in the first place.
Behaviour worth knowing
rate is fetched; the unique index is the real guarantee, and a losing double submission is
translated from the SQLite constraint error into a domain error.
that replaces it arrives in feat(ledger): owner-only money commands and routine wiring #356.
Testing
CI=true npx vitest run→ 104 files, 747 tests, 0 failures.npm run build:backend→ clean.down/upround trip on a file-backed database, plus rejected invalid values foreach
CHECK.test-and-buildgreen on the pushed head.Notes / follow-ups
amount_originalis still a float column. It is informational; the balance is the integercents column. Moving it to minor units is a follow-up migration.
LedgerRepository.listRecenthas no production caller yet.in memory: under parallel load the 5s default made CI non-deterministic across several
pre-existing tests.
Stack
Next: #355 (
feat/ledger-fx→ this branch).