swaits/wanderbooks
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(store,core): a refused command never reaches the journal; reversal rules

Browse Source 
Max-testing pass, step 2 (rank 1 regressions + a rank 2 law). Two ways
to make the books unopenable, both found by test, both fixed:

1. Reversing into a closed period: the store journaled `entry.reversed`,
   then the engine refused it at post. Replay could never apply that
   line, so Store::open failed ("period 1 is closed"). Regression:
   reversal_into_a_closed_period_is_refused_before_journaling.
2. An oversized invoice (or any command whose hand-written store
   pre-checks lagged the engine, here the new line range) journaled,
   then failed in the engine. Found by the new property
   refused_commands_never_reach_the_journal within 4s.

The fix is structural, not another hand-written check: every
journal-first store command now rehearses on a scratch clone of the
ledger (`Store::rehearse`), so whatever the engine refuses is refused
before the journal, now and after any future engine change. The
create_entry rehearsal uses the checked constructor: no draft is ever
dated in a closed period (the Lean invariant Inv.draftsOpen, which the
store had bypassed).

The property (1,000 random command sequences, heavy on refusals)
asserts, after every command: a refusal leaves the journal
byte-identical; every posted entry validates; the trial balance
balances; no draft sits in a closed period; and the books reopen and
verify at the end. Shown to have teeth: removing the create_entry
rehearsal makes it fail and shrink to open-period, close, draft.

Engine (spec confirmed by Stephen, 2026-10-13), Ledger::check_reversal,
a pure pre-check that reverse_entry and the store share:
- an entry is reversed at most once (EntryAlreadyReversed); reversing
  the reversal re-instates it;
- a closing entry is never reversed (ClosingEntryIrreversible);
- a refused reversal leaves no trace (it used to leave a draft and
  burn an id).

Specs go in doc comments as a ```text GOAL/INVARIANTS/SCENARIOS block.
New dev dependency: proptest for ledger-store (already the workspace's
property library).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GWVo7x3UHw5kwCUaWxiejX
This commit is contained in:
swaitsandClaude Opus 5.5 committed 2026-10-13 11:40:50 -06:00
1 parent 3609bf065e
commit a09c1fb218
7 files changed
+478 -6
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/ledger-store/Cargo.toml
+1
View File
@@ -19,6 +19,7 @@ hex = "0.4"
[dev-dependencies]
tempfile = "3"
proptest = "1"
rust_decimal = "1"
[lints]
crates/ledger-store/src/store.rs
+88 -3
View File
@@ -397,6 +397,7 @@ impl Store {
parent: Option<AccountId>,
placeholder: bool,
) -> Result<AccountId, StoreError> {
self.rehearse(|l| l.add_account(code, name, account_type, parent, placeholder))?;
// Pure pre-validation replicating ChartOfAccounts::add, so the
// journal write below cannot be followed by a failed mutation.
let chart = self.ledger.chart();
@@ -439,6 +440,9 @@ impl Store {
date: chrono::NaiveDate,
description: &str,
) -> Result<EntryId, StoreError> {
// The checked constructor: no draft is ever dated in a closed
// period (the Lean invariant `Inv.draftsOpen`).
self.rehearse(|l| l.try_create_entry(date, description))?;
let id = self.ledger.next_entry_id();
let line = self.append_journal(
"entry.created",
@@ -481,6 +485,7 @@ impl Store {
/// Validate, journal, and post a draft entry.
pub fn post_entry(&mut self, entry: EntryId) -> Result<(), StoreError> {
self.rehearse(|l| l.post_entry(entry))?;
let snap = self
.ledger
.get_entry(entry)
@@ -518,9 +523,9 @@ impl Store {
.get_entry(entry)
.ok_or(LedgerError::UnknownEntry(entry))?
.clone();
if orig.status() != EntryStatus::Posted {
return Err(LedgerError::EntryNotPosted(entry).into());
}
// Every refusal before the journal: a journaled reversal the engine
// then refuses could never be replayed, and the books would not open.
self.ledger.check_reversal(entry, date)?;
let new_id = self.ledger.next_entry_id();
let flipped: Vec<Value> = orig.lines().iter().map(flip_line_json).collect();
// Reversals journal a single entry.reversed line (no separate
@@ -548,6 +553,7 @@ impl Store {
/// Register a fixed asset. Journaled as `asset.registered`.
pub fn register_asset(&mut self, new: NewAsset) -> Result<AssetId, StoreError> {
self.rehearse(|l| l.register_asset(new.clone()))?;
// Pure pre-validation so the journal write cannot be followed by a
// failed mutation.
self.ledger.validate_new_asset(&new)?;
@@ -757,6 +763,7 @@ impl Store {
proceeds_account: AccountId,
gain_loss_account: AccountId,
) -> Result<EntryId, StoreError> {
self.rehearse(|l| l.dispose_asset(asset_id, date, proceeds))?;
let asset = self
.ledger
.get_asset(asset_id)
@@ -924,6 +931,7 @@ impl Store {
start: NaiveDate,
end: NaiveDate,
) -> Result<PeriodId, StoreError> {
self.rehearse(|l| l.create_period(name, start, end))?;
// Pure pre-validation so the journal write cannot be followed by a
// failed mutation.
ledger_core::periods::validate_period_range(start, end)?;
@@ -1059,6 +1067,20 @@ impl Store {
/// account path when missing. The ledger's internal ensure_account
/// then no-ops, which keeps replay deterministic: every account
/// creation is an `account.added` event.
/// Rehearse a command on a scratch copy of the ledger before anything
/// is journaled, so every refusal the engine would make surfaces first.
/// A journaled command the engine then refused could never be replayed,
/// and the books would not reopen (found by the refusal property in
/// `tests/store.rs`, 2026-10-13). Hand-written pre-checks drift from
/// the engine; the engine itself cannot.
fn rehearse<T>(
&self,
op: impl FnOnce(&mut Ledger) -> Result<T, LedgerError>,
) -> Result<(), StoreError> {
op(&mut self.ledger.clone())?;
Ok(())
}
fn ensure_control_account(
&mut self,
code: &str,
@@ -1128,6 +1150,7 @@ impl Store {
/// Add a customer. Journaled as `customer.added`.
pub fn add_customer(&mut self, name: &str) -> Result<CustomerId, StoreError> {
self.rehearse(|l| l.add_customer(name))?;
// The engine's only failure mode is an empty name; replicate it so
// the journal write cannot be followed by a failed mutation.
if name.trim().is_empty() {
@@ -1143,6 +1166,7 @@ impl Store {
/// Add a vendor. Journaled as `vendor.added`.
pub fn add_vendor(&mut self, name: &str) -> Result<VendorId, StoreError> {
self.rehearse(|l| l.add_vendor(name))?;
if name.trim().is_empty() {
return Err(LedgerError::EmptyAssetDescription.into());
}
@@ -1164,6 +1188,11 @@ impl Store {
due_date: NaiveDate,
lines: Vec<DocumentLine>,
) -> Result<InvoiceId, StoreError> {
self.rehearse(|l| {
ensure_on(l, "1200", "Accounts Receivable", AccountType::Asset)?;
ensure_on(l, "4000", "Revenue", AccountType::Revenue)?;
l.create_invoice(customer_id, date, due_date, lines.clone())
})?;
// Pure pre-validation mirroring Ledger::create_invoice, so the
// journal writes below cannot be followed by a failed mutation.
// (The auto-posted entry balances by construction and its accounts
@@ -1223,6 +1252,11 @@ impl Store {
due_date: NaiveDate,
lines: Vec<DocumentLine>,
) -> Result<BillId, StoreError> {
self.rehearse(|l| {
ensure_on(l, "6999", "General Expenses", AccountType::Expense)?;
ensure_on(l, "2000", "Accounts Payable", AccountType::Liability)?;
l.create_bill(vendor_id, date, due_date, lines.clone())
})?;
if self.ledger.get_vendor(vendor_id).is_none() {
return Err(LedgerError::UnknownVendor(vendor_id).into());
}
@@ -1274,6 +1308,10 @@ impl Store {
amount: Decimal,
cash_account: AccountId,
) -> Result<EntryId, StoreError> {
self.rehearse(|l| {
ensure_on(l, "1200", "Accounts Receivable", AccountType::Asset)?;
l.receive_payment(invoice_id, date, amount, cash_account)
})?;
// Pure pre-validation mirroring Ledger::receive_payment, plus the
// account checks post_entry would perform: a failed post after the
// journal writes would leave the journal ahead of the ledger.
@@ -1332,6 +1370,10 @@ impl Store {
amount: Decimal,
cash_account: AccountId,
) -> Result<EntryId, StoreError> {
self.rehearse(|l| {
ensure_on(l, "2000", "Accounts Payable", AccountType::Liability)?;
l.pay_bill(bill_id, date, amount, cash_account)
})?;
if amount <= Decimal::ZERO {
return Err(LedgerError::InvalidPaymentAmount(amount).into());
}
@@ -1512,6 +1554,12 @@ impl Store {
lines: Vec<PayrollLineInput>,
cash_account: AccountId,
) -> Result<PayrollRunId, StoreError> {
self.rehearse(|l| {
for (code, name, account_type) in PAYROLL_ACCOUNTS {
ensure_on(l, code, name, account_type)?;
}
l.run_payroll(date, lines.clone(), cash_account)
})?;
// Pure pre-validation mirroring Ledger::run_payroll, so the journal
// writes below cannot be followed by a failed mutation.
self.ledger.validate_payroll(date, &lines, cash_account)?;
@@ -1589,6 +1637,7 @@ impl Store {
date: NaiveDate,
cash_account: AccountId,
) -> Result<EntryId, StoreError> {
self.rehearse(|l| l.remit_payroll_taxes(date, cash_account))?;
// Pure pre-validation mirroring Ledger::remit_payroll_taxes.
if self.ledger.chart().get(cash_account).is_none() {
return Err(LedgerError::UnknownAccount(cash_account).into());
@@ -1663,6 +1712,7 @@ impl Store {
/// Add a class (division) for line-level P&L tracking. Journaled as
/// `class.added`.
pub fn add_class(&mut self, name: &str) -> Result<ClassId, StoreError> {
self.rehearse(|l| l.add_class(name))?;
// The engine's only failure mode is an empty name; replicate it so
// the journal write cannot be followed by a failed mutation.
if name.trim().is_empty() {
@@ -1690,6 +1740,7 @@ impl Store {
line_index: usize,
class_id: Option<ClassId>,
) -> Result<(), StoreError> {
self.rehearse(|l| l.set_line_class(entry_id, line_index, class_id))?;
// Draft-only check via the ledger, before anything is journaled.
let entry = self
.ledger
@@ -1802,6 +1853,7 @@ impl Store {
&mut self,
new: ledger_core::NewSchedule,
) -> Result<ledger_core::ScheduleId, StoreError> {
self.rehearse(|l| l.register_schedule(new.clone()))?;
ledger_core::schedules::validate_schedule(&new, self.ledger.chart())?;
let id = self.ledger.next_schedule_id();
let line = self.append_journal(
@@ -1873,6 +1925,7 @@ impl Store {
&mut self,
new: ledger_core::NewRecurring,
) -> Result<ledger_core::RecurringId, StoreError> {
self.rehearse(|l| l.register_recurring(new.clone()))?;
ledger_core::schedules::validate_recurring(&new, self.ledger.chart())?;
let id = self.ledger.next_recurring_id();
let lines: Vec<Value> = new
@@ -1938,6 +1991,7 @@ impl Store {
category: Option<&str>,
deductible_percent: Option<Decimal>,
) -> Result<(), StoreError> {
self.rehearse(|l| l.set_account_tax(account, category, deductible_percent))?;
self.ledger
.check_account_tax(account, category, deductible_percent)?;
let line = self.append_journal(
@@ -1962,6 +2016,7 @@ impl Store {
form: Option<ledger_core::Form1099>,
tin_on_file: bool,
) -> Result<(), StoreError> {
self.rehearse(|l| l.set_vendor_1099(vendor, form, tin_on_file))?;
self.ledger
.get_vendor(vendor)
.ok_or(LedgerError::UnknownVendor(vendor))?;
@@ -1981,6 +2036,7 @@ impl Store {
entry: EntryId,
vendor: Option<VendorId>,
) -> Result<(), StoreError> {
self.rehearse(|l| l.set_entry_vendor(entry, vendor))?;
// Validate first, so nothing invalid is journaled.
self.ledger
.get_entry(entry)
@@ -2823,6 +2879,35 @@ fn get_str<'a>(p: &'a Value, key: &str) -> Result<&'a str, StoreError> {
/// Replay one journal line into a ledger, asserting recorded ids. Used by
/// hydration, `verify`, and `rebuild`: the journal is the script, the engine
/// is the actor.
/// [`Store::ensure_control_account`] on a bare ledger, for rehearsals.
fn ensure_on(
l: &mut Ledger,
code: &str,
name: &str,
account_type: AccountType,
) -> Result<AccountId, LedgerError> {
match l.chart().find_by_code(code) {
Some(a) => Ok(a.id),
None => l.add_account(code, name, account_type, None, false),
}
}
/// The payroll control accounts, in the order [`Store::run_payroll`]
/// creates them.
const PAYROLL_ACCOUNTS: [(&str, &str, AccountType); 7] = [
("6100", "Salary & Wage Expense", AccountType::Expense),
("6110", "Payroll Tax Expense", AccountType::Expense),
(
"2110",
"Federal Withholding Payable",
AccountType::Liability,
),
("2120", "State Withholding Payable", AccountType::Liability),
("2130", "SS Tax Payable", AccountType::Liability),
("2140", "Medicare Tax Payable", AccountType::Liability),
("2150", "Other Deductions Payable", AccountType::Liability),
];
pub(crate) fn replay_line_into_ledger(
ledger: &mut Ledger,
line: &JournalLine,
crates/ledger-store/tests/store.rs
+245
View File
@@ -752,3 +752,248 @@ fn a_draft_before_a_posted_entry_verifies_and_reopens() {
let mut store = Store::open(&dir).unwrap();
assert_eq!(store.verify_and_heal().unwrap().healed, None);
}
/// Regression (found by the max-testing pass, 2026-10-13): reversing into a
/// closed period journaled the reversal *before* the engine refused it, so
/// the journal held an event replay could never apply and the books would
/// not reopen. Oracle (rank 1, spec R-3 + closed periods): a refused
/// command leaves no journal line, and the books always reopen.
#[test]
fn reversal_into_a_closed_period_is_refused_before_journaling() {
let dir = test_dir("reverse-closed");
let mut store = build_books(&dir);
let jan = store
.create_period(
"2026-01",
NaiveDate::from_ymd_opt(2026, 1, 1).unwrap(),
NaiveDate::from_ymd_opt(2026, 1, 31).unwrap(),
)
.unwrap();
store.close_period(jan).unwrap();
let posted = store.ledger().posted_entries().next().unwrap().id();
let journal = std::fs::read_to_string(dir.join("journal.jsonl")).unwrap();
let err = store.reverse_entry(
posted,
NaiveDate::from_ymd_opt(2026, 1, 15).unwrap(),
"into a closed month",
);
assert!(err.is_err());
assert_eq!(
std::fs::read_to_string(dir.join("journal.jsonl")).unwrap(),
journal,
"a refused reversal must not reach the journal"
);
drop(store);
let reopened = Store::open(&dir).expect("the books still open");
reopened.verify().unwrap();
}
// ---------------------------------------------------------------------------
// Refusals never reach the journal (rank 2: a law over every command
// sequence). Found by the max-testing pass, 2026-10-13: a reversal into a
// closed period was journaled before the engine refused it, and the books
// would not reopen.
// ---------------------------------------------------------------------------
mod refusals {
use super::*;
use ledger_core::{DocumentLine, Form1099};
use proptest::prelude::*;
/// One store command, with arguments drawn to hit both the happy path
/// and every refusal: small id spaces so unknown, duplicate, posted,
/// closed, and already-reversed cases come up constantly.
#[derive(Clone, Debug)]
enum Op {
AddAccount(u8, u8, Option<u64>, bool),
CreateEntry(u32),
AddLine(u64, u64, bool, i64),
Post(u64),
Reverse(u64, u32),
CreatePeriod(u32, u32),
Close(u64),
AddCustomer(bool),
AddVendor(bool),
Invoice(u64, u32, u32, i64),
Bill(u64, u32, u32, i64),
ReceivePayment(u64, u32, i64, u64),
PayBill(u64, u32, i64, u64),
AddClass(u8),
SetLineClass(u64, usize, Option<u32>),
SetEntryVendor(u64, Option<u64>),
SetVendor1099(u64, bool),
SetAccountTax(u64, u8),
}
fn day(n: u32) -> NaiveDate {
NaiveDate::from_ymd_opt(2026, 1, 1).unwrap() + chrono::Days::new(u64::from(n % 120))
}
/// Cents, biased toward the edges: zero, negative, tiny, huge.
fn arb_cents() -> impl Strategy<Value = i64> {
prop_oneof![
4 => 1i64..200_000,
1 => Just(0i64),
1 => -50_000i64..0,
1 => Just(i64::MAX / 2),
]
}
fn arb_op() -> impl Strategy<Value = Op> {
let id = 0u64..12;
prop_oneof![
(0u8..8, 0u8..5, prop::option::of(0u64..10), any::<bool>())
.prop_map(|(c, t, p, h)| Op::AddAccount(c, t, p, h)),
(0u32..120).prop_map(Op::CreateEntry),
(id.clone(), 0u64..10, any::<bool>(), arb_cents())
.prop_map(|(e, a, d, c)| Op::AddLine(e, a, d, c)),
id.clone().prop_map(Op::Post),
(id.clone(), 0u32..120).prop_map(|(e, d)| Op::Reverse(e, d)),
(0u32..120, 0u32..120).prop_map(|(a, b)| Op::CreatePeriod(a, b)),
(0u64..5).prop_map(Op::Close),
any::<bool>().prop_map(Op::AddCustomer),
any::<bool>().prop_map(Op::AddVendor),
(0u64..4, 0u32..120, 0u32..120, arb_cents())
.prop_map(|(c, d, due, a)| Op::Invoice(c, d, due, a)),
(0u64..4, 0u32..120, 0u32..120, arb_cents())
.prop_map(|(v, d, due, a)| Op::Bill(v, d, due, a)),
(0u64..4, 0u32..120, arb_cents(), 0u64..10)
.prop_map(|(i, d, a, c)| Op::ReceivePayment(i, d, a, c)),
(0u64..4, 0u32..120, arb_cents(), 0u64..10)
.prop_map(|(b, d, a, c)| Op::PayBill(b, d, a, c)),
(0u8..3).prop_map(Op::AddClass),
(id.clone(), 0usize..4, prop::option::of(0u32..4))
.prop_map(|(e, i, c)| Op::SetLineClass(e, i, c)),
(id.clone(), prop::option::of(0u64..4)).prop_map(|(e, v)| Op::SetEntryVendor(e, v)),
(0u64..4, any::<bool>()).prop_map(|(v, t)| Op::SetVendor1099(v, t)),
(0u64..10, 0u8..4).prop_map(|(a, k)| Op::SetAccountTax(a, k)),
]
}
const CODES: [&str; 8] = [
"1000", "1200", "2000", "3000", "3999", "4000", "6000", "6100",
];
const TYPES: [AccountType; 5] = [
AccountType::Asset,
AccountType::Liability,
AccountType::Equity,
AccountType::Revenue,
AccountType::Expense,
];
const TAX: [Option<&str>; 4] = [
None,
Some("advertising"),
Some("no-such-category"),
Some(""),
];
fn apply(store: &mut Store, op: &Op) -> Result<(), StoreError> {
let usd = |c: i64| Money::new(Decimal::new(c, 2), Currency::USD);
let line = |c: i64| {
vec![DocumentLine {
description: "work".into(),
amount: Decimal::new(c, 2),
}]
};
match *op {
Op::AddAccount(c, t, p, h) => store
.add_account(CODES[c as usize], "acct", TYPES[t as usize], p, h)
.map(drop),
Op::CreateEntry(d) => store.create_entry(day(d), "e").map(drop),
Op::AddLine(e, a, d, c) => {
let side = if d { Side::Debit } else { Side::Credit };
store.add_line(e, a, side, usd(c))
}
Op::Post(e) => store.post_entry(e),
Op::Reverse(e, d) => store.reverse_entry(e, day(d), "undo").map(drop),
Op::CreatePeriod(a, b) => store.create_period("p", day(a), day(b)).map(drop),
Op::Close(p) => store.close_period(p).map(drop),
Op::AddCustomer(blank) => store
.add_customer(if blank { " " } else { "Acme" })
.map(drop),
Op::AddVendor(blank) => store.add_vendor(if blank { "" } else { "Vend" }).map(drop),
Op::Invoice(c, d, due, a) => {
store.create_invoice(c, day(d), day(due), line(a)).map(drop)
}
Op::Bill(v, d, due, a) => store.create_bill(v, day(d), day(due), line(a)).map(drop),
Op::ReceivePayment(i, d, a, c) => store
.receive_payment(i, day(d), Decimal::new(a, 2), c)
.map(drop),
Op::PayBill(b, d, a, c) => store.pay_bill(b, day(d), Decimal::new(a, 2), c).map(drop),
Op::AddClass(n) => store.add_class(["A", "B", ""][n as usize]).map(drop),
Op::SetLineClass(e, i, c) => store.set_line_class(e, i, c),
Op::SetEntryVendor(e, v) => store.set_entry_vendor(e, v),
Op::SetVendor1099(v, t) => store.set_vendor_1099(v, Some(Form1099::Nec), t),
Op::SetAccountTax(a, k) => store.set_account_tax(a, TAX[k as usize], None),
}
}
/// The Lean model's `Inv` (verify/lean/Wanderbooks/Ledger.lean), checked
/// on the real store: every posted entry is acceptable, the books net
/// to zero, and no draft is dated in a closed period.
fn check_invariants(store: &Store) -> Result<(), TestCaseError> {
let l = store.ledger();
for e in l.posted_entries() {
prop_assert!(
e.validate(l.chart()).is_ok(),
"posted entry {} invalid",
e.id()
);
}
let tb = ledger_core::reports::trial_balance(l);
prop_assert_eq!(tb.total_debits, tb.total_credits);
for id in 1..l.next_entry_id() {
if let Some(e) = l.get_entry(id)
&& e.status() == ledger_core::EntryStatus::Draft
{
prop_assert!(
l.closed_period_containing(e.date()).is_none(),
"draft {} is dated {} inside a closed period",
id,
e.date()
);
}
}
Ok(())
}
proptest! {
#![proptest_config(ProptestConfig {
cases: 1000,
failure_persistence: None,
..ProptestConfig::default()
})]
/// A refused command leaves the journal byte-identical, and after
/// any sequence the books reopen and verify.
#[test]
fn refused_commands_never_reach_the_journal(
ops in prop::collection::vec(arb_op(), 1..40),
seed in any::<u64>(),
) {
let dir = test_dir(&format!("refusals-{seed}"));
let mut store = Store::create(&dir, Currency::USD).unwrap();
let journal = dir.join(ledger_store::JOURNAL_FILE);
for op in &ops {
check_invariants(&store)?;
let before = std::fs::read_to_string(&journal).unwrap();
if let Err(e) = apply(&mut store, op) {
let after = std::fs::read_to_string(&journal).unwrap();
prop_assert!(
after == before,
"refused {:?} ({}) but the journal grew by:\n{}",
op,
e,
after.strip_prefix(&before).unwrap_or(&after)
);
}
}
check_invariants(&store)?;
drop(store);
let reopened = Store::open(&dir);
prop_assert!(reopened.is_ok(), "books failed to reopen: {:?}", reopened.err());
reopened.unwrap().verify().unwrap();
let _ = std::fs::remove_dir_all(&dir);
}
}
}