ap and invoices: the PR 78, 89 and 90 follow ups

[futurebuildai]

The follow ups of PR 78 (C4-1b), PR 89 and PR 90: five findings from the reviews, each fixed once, red first (a test that fails on fa9e13e, shown in the commits), the GL ties asserted after every money act in the new tests (every entry balanced; 2010 equals the open bills' amount_open).

Fix round 2 (the r2 Opus review and the r1 leftovers) follows the same rule: each finding red first on ed8346a, one commit per finding, the money ties asserted by ledger sums after every act, per currency where the finding is about currency (sections 6 to 10).

Fix round 3 (the r4 Opus review and r3's P3s) follows the same rule: each finding red first on 61b4deb (the red shown by reverting that one fix locally for the run), one commit per finding, the money ties asserted by ledger sums after every act (sections 11 to 15). The rule this round completes: a bill's credit is live exactly when the POSTED entries of its VENDOR_INVOICE entry and that entry's whole reversal chain leave a net credit on 2010; every AP act decides on that, never on one step of the chain.

Fix round 4 (the r6 Opus review's P2 and P3, and r5 Sonnet's P3s) follows the same rule: each finding red first on d76b9c1 (the race and the later findings red on d76b9c1 plus only the earlier findings' fixes, none of which touch the path under test), one commit per finding, the money ties asserted by ledger sums after every act, per currency (sections 16 to 19). Out of scope, left for the GL item the reviews named: GL voids and reverses that race or follow an AP act (the entry voided beside the void's own reversal, the credit reinstated under a VOIDED bill), and the payment consulting no ledger.

1. PR 78 r8 P3-1: a pending bill carrying amount_paid approved whole (f81265e)

A legacy pending bill with amount_paid <> 0 (an unknown status migration 106 made PENDING, or a hand edited row) approved whole: the entry credited 2010 with the entire total while amount_open stood at the migration's clamp (zero on a bill paid past its total, the rest on a partly paid one), so 2010 held money no bill owed and no route cleared it.

Choice: the approve refuses it (a 409 invalid_state_transition with the has_paid_amount blocker naming the paid cents), not a migration 108 reset. Why: a reset of open to the total would make the desk pay the already paid part again (a migration writing money), a reset to total minus paid is what 106 already computes and leaves the tie broken by exactly the paid amount at the approve, and the refusal keeps 2010 and open moving together by construction while the notices of 106 already name the odd rows for the operator to mend. New data cannot reach the shape: the pay applies only to an approved or partial bill.

Proof: TestWire_ApproveRefusesLegacyPaidAmount seeds both shapes (paid 9.00 on a 4.00 bill with open 0; paid 3.00 on a 10.00 bill with open 7). Red on fa9e13e: the approve answered 200 and posted the total against open 0. Green: each approve is a 409 has_paid_amount, the bill stays PENDING at revision 1, no entry is written, every entry balances and control equals open after each refusal. The fragment, docs/modules/ap.md and a CONTRACT-CHANGES row state it.

2. PR 78 r8 P3-2: the void's no reversal arm was keyed on the total (224d884)

A legacy bill hand edited to a zero total after its approve voided with nothing reversed, stranding its posted credit in 2010 under a VOIDED bill; a bill whose entry never posted was refused instead.

The arm now asks the ledger: repo.HasPostedInvoiceEntry (the predicate migration 106 links by: a posted, unreversed VENDOR_INVOICE entry stands for the bill). A bill that holds a live entry it is not linked to is refused entry_not_linked whatever its total says now; a bill with no live entry (one that prices to zero, or a legacy bill whose posting was swallowed or whose entry was already reversed) holds no credit and voids with nothing to reverse; a linked live entry still voids reversed whole.

Proof: TestWire_VoidHandEditedZeroTotalBill seeds the hand edited shapes. Red on fa9e13e: the unlinked zero total bill over a live one leg credit voided 200 with the 10.00 stranded. Green: it is a 409 entry_not_linked with nothing written and control and open untouched; the linked zero total bill voids 200 with its entry reversed once, control down exactly 1000 and only the seeded legacy entry unbalanced. The unlinked case of TestWire_VoidLegacyApprovedBill (entry deleted) now voids 200 owing nothing, control and open both 0: red on fa9e13e (it answered 409), green here, and TestWire_VoidZeroBill still passes untouched. The fragment, docs/modules/ap.md and a CONTRACT-CHANGES row state the new rule.

3. PR 89 r1 P2-1: the currency.default setting was never read on the route (66c68cf, 6b1e955)

Parse set an omitted currency to USD before the service saw it, so the resolve that reads currency.default never read it on the route. Parse now leaves an omitted currency empty and the create reads the setting (USD where it is absent, as the COALESCE always said); a named currency is still validated as an ISO 4217 code of three capital letters.

Proof: TestWire_CreateCurrencyFromSetting sets the setting to EUR. Red on fa9e13e: a create with the currency omitted answered USD. Green: it answers and stores EUR; a named CAD wins; a lowercase usd is a 400 naming currency; every entry balances and control equals open. The test borrows the row migration 091 seeds and puts it straight back (a parallel package's order queries CROSS JOIN it; the first cut deleted the row and failed three packages, fixed in 6b1e955 before the push). docs/modules/ap.md now says the setting is read on the route (r2 P3-a), gives the local curl body inline (r2 P3-b) and tells a repeated bill id in one payment from a repeated payment caught by the Idempotency-Key (r2 P3-d). A CONTRACT-CHANGES row states it.

4. PR 90 r1 F3: the write off fragment named a blocker the code never answers (6df4a2e)

core/api/fragments/invoice.yaml said the write off route refuses with invoice_not_open; the code answers a plain invalid_state_transition and the string exists nowhere in Go.

Choice: the fragment is made true to the code, not a new blocker added in code. Why: ADR 0005 section 12's route table names no blocker for the wrong state on this route (its siblings, the credit memo apply and refund rows, name a bare invalid_state_transition too), and PR 90 already dropped the claim from docs/modules/invoices.md on the same ruling. No behaviour changes, so no CONTRACT-CHANGES row. invoice_not_open no longer occurs anywhere in the repository; openapi.yaml and schema.d.ts regenerated (merge -check fresh).

5. PR 78 r8 P3-3 and r7 P3-2: two cosmetics (d05a587)

TestWire_VoidZeroBill sat between TestWire_VoidLegacyApprovedBill's RULE comment and its func: the RULE paragraph now sits directly above the legacy test. The redundant unit price <= maxUnitPriceTenThousandths in Parse's Extend guard (the price check above it already records that error; the r7 mutant survived) is dropped.

6. r2 P1-1: a reversal of any status unlinked the credit (692efdb)

HasPostedInvoiceEntry counted a reversal row of any status as an undo, but the trial balance sums POSTED entries alone: an unlinked bill whose entry's reversal was voided (the GL void route voids any posted entry, a reversal included) voided 200 with nothing reversed, stranding the live credit in 2010 under a VOIDED bill. A reversal now counts only when it is POSTED.

Red on ed8346a: TestWire_VoidRefusesUnlinkedBillWhoseReversalIsVoid built the L05 shape through the public GL routes (approve, reverse, void the reversal, unlink by hand) and got void = 200, want 409. Green: 409 entry_not_linked, nothing written, the bill stays APPROVED, one live credit still stands for it, control and open untouched. The fixture now mounts the GL routes so tests reach the ledger through them, not hand SQL.

Migration 106's link predicate has the same textual blind spot and is left as it is on purpose: the flaw only unlinks (POSTED is required on its own, so it never links a dead entry), and an entry whose reversal row stands can never be reversed again while that row holds the one reversal an entry may have (the 077 unique index is not status filtered), so an unlinked bill is the safer side: its void is refused rather than pointed at an entry it cannot reverse, and 106's "no journal entry to link" notice counts it for the operator.

7. r2 P2-1: the linked arm answered 500 when the GL routes had undone the entry (da56fc6)

Choice: check the linked entry against the ledger and void with nothing reversed when it is not live (not POSTED, or a POSTED reversal points at it, or it was deleted by hand); refuse with the 409 reversal_not_posted blocker when it is live while a not POSTED reversal row points at it, because that credit stands in 2010 and no new reversal can post while the row holds the one reversal an entry may have; never a 500. The refusal beats a silent void there: voiding would strand the credit the P1-1 fix protects.

Red on ed8346a: TestWire_VoidLinkedEntryAlreadyUndoneByGL drove all three shapes on new data through the GL routes; the reversed and the voided linked entry each answered 500 internal_error, the voided reversal shape 500 too. Green: the first two void 200 with nothing reversed (no entries written, control untouched, open closes, the tie the GL act broke is mended), the third answers 409 reversal_not_posted with nothing written. ErrAlreadyReversed is mapped to the same 409 as a race belt. The fixture's control balance now sums POSTED entries only, as the trial balance does.

8. r2 P2-2: a payment posted in the setting's currency, not the bills' (5832e3d)

PayVendor posted its entry with no Currency, so it took the currency.default setting's code whatever the bills carried, and 2010 tied in sum but not per currency (the trial balance never sums across two). The entry now posts in the bills' currency, and a payment naming bills of more than one currency is refused with the 409 currency_mismatch blocker naming the bills that differ, nothing written.

Red on ed8346a: TestWire_PaymentCurrencyFollowsTheBills paid a bill entered under EUR after the setting went back to USD; the payment entry posted USD (0 entries in the bill's EUR) and the per currency ties broke (EUR control 10000 against open 0, USD control -10000 against open 0); the mixed USD and CAD payment went through. Green: the entry posts EUR, every per currency tie holds after each act, and the mixed payment answers 409 currency_mismatch naming both bills with no payment, application or entry written. The settings row is borrowed under the shared lock every wire test holds and put straight back.

9. r2 P3-1: a double posted bill voided with its duplicate credit live (e51700f)

A bill holding a live VENDOR_INVOICE entry beside its linked one (the base read the status outside its transaction, so a double click could post it twice; only the newest is linked) voided 200 reversing the linked entry alone. The void is now refused with the 409 entry_not_linked blocker naming the other live entry by number and id.

Red on ed8346a: TestWire_VoidRefusesDoublePostedBill seeded the duplicate and got void = 200, want 409. Green: 409 naming the duplicate, nothing written, no reversal, control and open untouched.

10. r1 P3-1, P3-2 and P3-3: the comment, the pending case, the parked row (57d030e, 831f422)

The duplicate invoice ids RULE comment moved back down to its own test. TestWire_VoidLegacyApprovedBill gained the pending case (a pending bill holding a live unlinked entry voids refused entry_not_linked, nothing written); the arm is status agnostic, so this is coverage, not a behaviour change, and it was green on ed8346a as the review said. CONTRACT-CHANGES states the limit of a legacy pending bill carrying a paid amount: it can be neither approved (has_paid_amount) nor voided (has_payments runs first) and no route edits amount_paid, so the row is parked for the operator to mend by hand. 831f422 is a wording and ordering straightening of the void's doc comment, no behaviour change.

11. r4 P1-1: liveness is the net of the whole reversal chain (321f289)

HasPostedInvoiceEntry, LiveInvoiceEntryOtherThan and LinkedEntryLedger each looked one level down the reversal chain: a POSTED reversal pointing at the bill's entry read as dead even when that reversal's own reversal (posted through the public GL reverse route, a reinstatement) had put the credit back, so the void answered 200 with nothing reversed and left the bill VOIDED with its full credit live in 2010, linked or not (a regression this PR introduced; the base answered 500 or entry_not_linked, each writing nothing).

Choice: the net, not the parity of the POSTED chain depth. Why: the trial balance sums 2010 as an amount, so "the bill's credit stands" is an amount question; a chain can hold a voided middle, a reinstatement behind it and legs that price it partially at once, and only the net answers every mix (parity alone would call a chain that nets zero live when the entry priced to zero, and would miss a voided middle counting twice). Liveness now walks reverses_entry_id recursively through entries of any status and sums the POSTED entries' 2010 amount: nonzero means live. All three predicates use it, the same lens for the linked and the unlinked arm and for the double posted check. A live linked entry whose one reversal row is already taken (of any status, so no new reversal can post under ux_gl_entry_single_reversal) is refused with the 409 reversal_not_posted blocker naming the shape, nothing written.

Proof: TestWire_VoidRefusesReinstatedEntry (the review's reproducer as written) and TestWire_VoidDecidesOnTheWholeReversalChain (chains of depth 0 to 3 with reversal positions voided, linked and unlinked). Red with the fix reverted locally: the reproducer voided 200 in both arms with 1600 cents of the credit live in 2010 under a VOIDED bill, and the depth table failed on the reinstatement shapes. Green: every live chain with its reversal row taken is a 409 (reversal_not_posted linked, entry_not_linked unlinked) with nothing written and the bill untouched; every chain netting zero voids 200 with nothing to reverse; the plain bill voids 200 reversing whole; the tie is asserted after every act.

12. r4 P3-2: the approve refuses a bill that already holds a live entry (440b980)

A legacy PENDING bill whose posting ran before the status moved (the base's sync could post first; migration 106 links only APPROVED or PARTIAL, so the row carries its entry unlinked) approved into a second credit: 2010 held the total twice while the bill owed it once, and a whole payment later left the surplus under a PAID bill. The approve now decides on the ledger like every AP act: LiveInvoiceEntryOtherThan finds a live entry of the bill under the chain lens, and the 409 entry_not_linked blocker names it by number and id, nothing written.

Proof: TestWire_ApproveDecidesOnTheWholeReversalChain walks the depth 0 to 3 chains, linked and unlinked, over a hand set PENDING bill. Red with the fix reverted locally: 12 arms approved 200 posting a second credit (control 4200 against an owed 2100). Green: every live chain is refused naming the entry with nothing written and the bill stays PENDING at its revision; a chain netting zero approves into the one credit it owes, and the tie holds after every act.

13. r4 P3-1, the race r3 read but did not drive: a lost reversal insert race maps to ErrAlreadyReversed (54cb126)

When an AP void and a GL reverse of the same entry race, the loser's insert meets the one reversal index ux_gl_entry_single_reversal (a pre-check cannot see an uncommitted contender), and PostEntry passed the duplicate key error up unmapped, so the AP void answered 500 on a money route section 7's ErrAlreadyReversed belt claim covered. PostEntry now maps a unique violation on a reversal insert to ErrAlreadyReversed, which the void answers with the same 409 reversal_not_posted as the belt, nothing written.

Proof: TestWire_VoidReverseRaceNever500, 20 rounds of one AP void against one GL reverse under -race. Red with the fix reverted locally: in 19 of the 20 rounds the AP void answered 500 internal_error with the duplicate key log line. Green: neither side ever answers 5xx, exactly one reversal of the entry stands each round, the bill either voids or stays voidable (the retry voids 200 with nothing to reverse), every entry balances and control equals open after every round.

14. r4 P3-3: a reversal posts in the entry's own currency (be9293d)

The void handed PostReversal the bill's current currency, and PostReversal took the currency it was handed, so a bill whose currency had been hand edited after its approve (no route changes a bill's currency; legacy only) voided with the reversal in the bill's new currency and neither currency's 2010 tied alone (the trial balance never sums across two). GetJournalEntry now reads the entry's currency and PostReversal posts in it, the input's currency kept only as the fallback for an entry that carries none.

Proof: TestWire_VoidReversalKeepsTheEntryCurrency builds the review's C5 shape by hand (a CAD bill over a USD entry). Red with the fix reverted locally: the reversal was not USD, and after the void the USD control stood at 2500 and the CAD at -2500. Green: the reversal posts USD and both currencies tie at zero after the void.

15. r3's P3s: the untested linked entry shape pinned (9e2f171)

The linked arm's not POSTED branch had no test shape of its own (the tested voided entry held no reversal row). The case r3 named, the entry VOID by the GL void route with its reversal VOID beside it, lands as a fourth shape in TestWire_VoidLinkedEntryAlreadyUndoneByGL: neither POSTED entry counts in 2010, the chain nets nothing, and the void answers 200 with nothing to reverse, the tie kept. It was already correct at 61b4deb under both lenses, so it pins the branch rather than turning a bug (no red exists for it); the race r3 read is section 13's test. Out of scope and left for its own GL item: the GL void route voids an entry that already holds a reversal, or a reversal itself (r4's note; the base behaves the same).

Gates

go build ./..., go vet ./... clean; migration 106 down (0 of the 6 columns), up (6) and a second up (idempotent, exit 0) on a scratch database (no new migration in this PR; fix round 2 adds none either: the 106 ruling is stated in section 6; fix round 3 adds none); CI=true GABLE_TEST_REQUIRE_DB=1 go test -race -count=1 ./... 70 packages ok twice in fix round 2 and twice again in fix round 3 after the base merge, none failed any run (pkg/outbox did not flake; the characterization goldens passed both round 3 runs); gofmt clean; no new files; make contract-go fresh (319 paths, 406 operations, 511 schemas, 0 pending, 1289 golden steps); bash scripts/check-shape.sh OK; python3 .github/scripts/reuse_gate.py passed; openapi.yaml and schema.d.ts regenerated from the changed fragment.

16. r6 P2-1: a non default currency bill whose entry the GL route reversed voided 200 with its CAD credit stranded (c1585e8)

A regression this PR introduced. The GL reverse route built its reversal with no currency, so it posted in the currency.default setting's code instead of the entry's, and the chain liveness summed 2010 across currencies: a CAD entry plus the route's USD reversal netted zero as a whole, so the AP void read the chain dead, answered 200, reversed nothing and left the bill VOIDED with its CAD credit live in the CAD ledger (on the base the same shape answered 500 with nothing written).

Both halves fixed. ReverseJournalEntry posts its reversal in the original entry's own currency (gl.yaml and a CONTRACT-CHANGES row state it), and both liveness reads (chainLive behind HasPostedInvoiceEntry and LiveInvoiceEntryOtherThan, and LinkedEntryLedger's own walk) net per currency, calling the chain live exactly when any one currency nets nonzero, so no mixed currency chain, however it arose (legacy rows included), can read as dead.

Proof: the review's reproducer as written, red on d76b9c1 ("void = 200 and the bill is VOIDED with CAD control 2000 against CAD open 0: the CAD credit is stranded"), green with the route fix alone (the chain becomes one currency and nets zero, the tie restored); and a chain holding two currencies built directly in the ledger (the shape no route can build any more), red on d76b9c1 and still red with the route fix alone, green with the per currency net: the linked void refused reversal_not_posted, the unlinked void and the legacy approve refused entry_not_linked, nothing written, every entry balanced, the per currency ties asserted after every act (CAD control against CAD open; the USD gap named as the hand edit's and untouched by every refusal).

17. r6 P3-1: the AP void answered 500 when a GL void of the linked entry committed mid request (2ad596e)

PostReversal's refusal to reverse an entry that is no longer POSTED was a plain error the void mapped nowhere: the void's ledger read had seen the entry POSTED, the GL void committed beneath it, and the act answered 500 internal_error on a money route (nothing written by the loser).

The refusal is now the typed gl.ErrNotPosted, mapped beside ErrAlreadyReversed to the same 409 reversal_not_posted, nothing written; ap.yaml, docs/modules/ap.md and a CONTRACT-CHANGES row state it. Proof: TestWire_VoidVoidRaceNever500, fifty rounds under -race of an AP void against a GL void of the linked entry. On the unfixed tree the test is red, with rounds answering 500 internal_error in the race; with the fix, two clean runs of the fifty rounds never answer 500 on either side, the lost rounds answer the 409 with nothing written (the entry count unchanged), the retry voids 200 with nothing to reverse, every entry balances, and the refused rounds' per currency ties hold as deltas against their own snapshot.

18. r5 P3-1: a net negative chain was refused with words that said a credit was live (8ea80a1)

A chain whose original entry was reversed and then voided holds the reversal's debit alone on 2010, yet every liveness refusal said a credit was live. The refusal was right and wrote nothing; only the words lied, on the void's linked arm, its unlinked arm and the legacy approve.

The messages now name the net amount standing in accounts payable, as a credit or as a debit, whichever way the chain nets; ap.yaml, the module docs and a CONTRACT-CHANGES row state the rule. Proof: TestWire_VoidRefusalWordsMatchANetNegativeChain builds the shape (the chain net of -1600 asserted), red on the unfixed messages in all three arms ("still holds its credit live in accounts payable", "could leave its credit in accounts payable", "would post a second credit in accounts payable"), green after: each refusal's message names the net and never says a credit is live, nothing written, the debit untouched, every entry balanced.

19. r5 P3-2 and P3-3: the reinstated entry test asserts the ties; the reversal race map keys on the index's own name (ecd4eb8, 6d8d098)

P3-2: TestWire_VoidRefusesReinstatedEntry asserted the status and the chain net but never the ledger sums. It now asserts, in both arms, that every entry balances and the control account holds exactly what the open bills owe after the acts that build the shape, and that nothing moved after the refused void (the sums unchanged, the bill still approved). The assertions bite: a mutant GL reverse route posting a stray balanced entry beside the reversal, outside the chain but pointing at the same bill, keeps the status and the chain net yet breaks the tie, and the strengthened test fails on it while the old one passed; the mutant was reverted and never committed.

P3-3: PostEntry mapped any unique violation on a reversal insert to ErrAlreadyReversed on the SQLSTATE alone; the map now also tests the violated constraint's name (ux_gl_entry_single_reversal), so a later unique index passes through unmapped instead of reading as a lost reversal race. Proof: TestPostEntryMapsOnlyTheReversalIndexViolation builds a probe partial unique index on the memo and collides a reversal carrying insert on it, red on the unfixed code (the loss "mapped to ErrAlreadyReversed"), green after (the loss stays a plain unique violation naming the probe index); the existing twenty round race test still proves the real index's violation maps to the same 409, never a 500. The module docs state the keying.

Round 4 gates, on the round's head 9f42561 with the round's own postgres:16-alpine container and a private GOCACHE: go build, go vet and the migrate rerun clean; CI=true GABLE_TEST_REQUIRE_DB=1 go test -race -count=1 ./... twice, both runs 70 packages ok and none failed (pkg/outbox did not flake in either run); gofmt clean (the reproducer's one comment alignment fixed in 9f42561); no new files, so no new SPDX headers; make contract-go fresh (319 paths, 406 operations, 511 schemas, 0 pending, 1289 golden steps); bash scripts/check-shape.sh OK; python3 .github/scripts/reuse_gate.py passed; openapi.yaml and schema.d.ts regenerated from the changed fragments (the api client types embed the description text; drift and typecheck pass in web/packages/api-client).

🤖 Generated with Claude Code