diff --git a/src/domain/fold.test.ts b/src/domain/fold.test.ts index 1426888..63e94a7 100644 --- a/src/domain/fold.test.ts +++ b/src/domain/fold.test.ts @@ -346,16 +346,33 @@ describe("the anchored (lastCharge) track", () => { expect(january.expected).toBe(0); // nothing left uncharged in January itself }); - test("with no recurring costs, projected still equals remaining", () => { - // The anchored model adds a second walk (`anchoredOccurrences`), but a - // dataset that never defines a recurring cost must never see it: both - // walks are keyed off `dataset.recurring`, which is empty here. - const data = dataset([post("food", 500)], [spend("a", "food", 300, "2026-01-05")]); - const fold = foldBalances(data, "2026-03"); - for (const month of ["2026-01", "2026-02", "2026-03"]) { - const figures = figuresFor(fold, "food", month); - expect(figures.expected).toBe(0); - expect(figures.projected).toBe(figures.remaining); - } + // REVIEW ROUND 1 / M1: the three-charges test above pins that January's + // `expected` is 0, but that assertion holds no matter WHICH charge the walk + // anchors from — with a 30-day cycle, even the earliest of the three + // (2026-01-05) steps to 2026-02-04, past January, so `charges[0]` in place + // of `charges[charges.length - 1]` in `anchoredOccurrences` would still + // pass it. This test uses a cycle short enough (10 days, matching the gap + // between each charge) that anchoring on the wrong end changes the answer: + // stepping from the EARLIEST charge (2026-01-05) lands on 2026-01-15 and + // then 2026-01-25 — both still inside January, both unconfirmed by + // definition (an anchored occurrence is never matched against an existing + // Purchase) — which would double-count 600 as still owed in a month that + // in fact has nothing left uncharged. Anchoring on the LATEST charge + // (2026-01-25) instead steps to 2026-02-04, outside January, leaving + // nothing expected there. Verified by swapping `charges[length - 1]` for + // `charges[0]` in `anchoredOccurrences` and watching this fail before + // reverting. + test("expected in January depends on anchoring off the LATEST charge, not the earliest", () => { + const data = datasetWithAnchoredPlan(); + data.recurring[0]!.recurrence = { kind: "everyNDays", n: 10 }; + data.purchases.push( + { id: "p1", date: "2026-01-05", description: "Phone plan", total: { amount: 300, currency: "DKK" }, splitMode: "percent", splits: [{ postId: "phone", value: 100, absorbsRemainder: true }], schedule: null, source: { recurringId: "plan" } }, + { id: "p2", date: "2026-01-15", description: "Phone plan", total: { amount: 300, currency: "DKK" }, splitMode: "percent", splits: [{ postId: "phone", value: 100, absorbsRemainder: true }], schedule: null, source: { recurringId: "plan" } }, + { id: "p3", date: "2026-01-25", description: "Phone plan", total: { amount: 300, currency: "DKK" }, splitMode: "percent", splits: [{ postId: "phone", value: 100, absorbsRemainder: true }], schedule: null, source: { recurringId: "plan" } }, + ); + + const january = figuresFor(foldBalances(data, "2026-01"), "phone", "2026-01"); + expect(january.charges).toBe(900); // 3 x 300, all real money already moved + expect(january.expected).toBe(0); // correct anchor steps to 2026-02-04, outside January }); }); diff --git a/src/export/json.test.ts b/src/export/json.test.ts index ce223e3..eaef723 100644 --- a/src/export/json.test.ts +++ b/src/export/json.test.ts @@ -606,6 +606,30 @@ describe("the occurrenceDate / anchoring pairing", () => { test("an anchored charge without occurrenceDate is accepted", () => { expect(() => parseDatasetJson(withCostAndCharge("lastCharge", { recurringId: "r1" }))).not.toThrow(); }); + + // REVIEW ROUND 1 / I1: a calendar charge with a genuinely MISSING + // occurrenceDate used to be reported with `String(undefined)`, which prints + // the literal word "undefined" — so someone restoring a backup would + // search their file for that text and find nothing, since the file has no + // such string anywhere. Missing and malformed are different problems, and + // the missing case now names the cost instead. + test("a calendar charge missing occurrenceDate names the cost, not the word 'undefined'", () => { + let message = ""; + try { + parseDatasetJson(withCostAndCharge("calendar", { recurringId: "r1" })); + } catch (error) { + message = (error as Error).message; + } + expect(message).toMatch(/Bill/); // the cost's name, from withCostAndCharge's fixture + expect(message).not.toMatch(/undefined/); + }); + + // The malformed case (present but not a real date) keeps its old shape. + test("a calendar charge with a malformed occurrenceDate still names the bad value", () => { + expect(() => + parseDatasetJson(withCostAndCharge("calendar", { recurringId: "r1", occurrenceDate: "nope" })), + ).toThrow(/invalid occurrence date "nope"/); + }); }); describe("startDate granularity under the anchored model", () => { diff --git a/src/export/json.ts b/src/export/json.ts index 0fed439..ff6537a 100644 --- a/src/export/json.ts +++ b/src/export/json.ts @@ -450,9 +450,21 @@ export function parseDatasetJson(text: string): Dataset { ); } } else { - if (!PURCHASE_DATE.test(source.occurrenceDate as string)) { + // Split from the "malformed" case below: `String(undefined)` prints + // the literal word "undefined", which sends someone restoring a + // backup searching their file for text that appears nowhere in it. + // Missing and malformed are different problems and need different + // messages, and this one names the cost the way the anchored-side + // message above already does. + if (source.occurrenceDate === undefined) { throw new ImportValidationError( - `${label} has an invalid occurrence date "${String(source.occurrenceDate)}"`, + `${label} has no occurrence date, but "${cost.name}" is calendar-anchored and a charge ` + + `against it settles one slot`, + ); + } + if (!PURCHASE_DATE.test(source.occurrenceDate)) { + throw new ImportValidationError( + `${label} has an invalid occurrence date "${source.occurrenceDate}"`, ); } } diff --git a/src/store/actions.test.ts b/src/store/actions.test.ts index 596fbeb..3836940 100644 --- a/src/store/actions.test.ts +++ b/src/store/actions.test.ts @@ -947,3 +947,164 @@ describe("confirmOccurrence by anchoring", () => { expect(p.date).toBe("2026-09-01"); }); }); + +// REVIEW ROUND 1 / I2: `addPurchase`/`updatePurchase` used to accept an +// arbitrary `source` unchecked — only `confirmOccurrence` built a coherent +// one. A later task adds a control that hand-builds a `source` and passes it +// straight to `addPurchase`, so this closes it here rather than leaving that +// task to carry the validation. Mirrors the pairing `json.ts` enforces at +// import: a source naming a calendar cost requires `occurrenceDate`; one +// naming an anchored cost forbids it; one naming a nonexistent cost throws. +describe("addPurchase/updatePurchase enforce the source/occurrenceDate pairing", () => { + function seeded() { + const d = createSeedDataset("2026-09"); + const post = d.posts[0]!; + const calendarCost = actions.addRecurringCost(d, { + name: "Rent", archived: false, + amount: { amount: 8000, currency: "DKK" }, + startDate: "2026-09", + recurrence: { kind: "everyNMonths", n: 1 }, + anchoring: "calendar", splitMode: "percent", + splits: [{ postId: post.id, value: 100, absorbsRemainder: true }], + }); + const anchoredCost = actions.addRecurringCost(d, { + name: "Phone", archived: false, + amount: { amount: 300, currency: "DKK" }, + startDate: "2026-09-01", + recurrence: { kind: "everyNMonths", n: 1 }, + anchoring: "lastCharge", splitMode: "percent", + splits: [{ postId: post.id, value: 100, absorbsRemainder: true }], + }); + return { d, post, calendarCost, anchoredCost }; + } + + function purchaseInput( + postId: string, + source?: { recurringId: string; occurrenceDate?: string }, + ) { + return { + date: "2026-09-15", + description: "Test", + total: { amount: 100, currency: "DKK" }, + splitMode: "percent" as const, + splits: [{ postId, value: 100, absorbsRemainder: true }], + schedule: null, + source, + }; + } + + test("addPurchase refuses a source naming an unknown recurring cost", () => { + const { d, post } = seeded(); + expect(() => actions.addPurchase(d, purchaseInput(post.id, { recurringId: "ghost" }))) + .toThrow(/Unknown recurring cost/); + }); + + test("addPurchase requires occurrenceDate when source names a calendar cost", () => { + const { d, post, calendarCost } = seeded(); + expect(() => actions.addPurchase(d, purchaseInput(post.id, { recurringId: calendarCost.id }))) + .toThrow(/occurrence date/i); + }); + + test("addPurchase accepts a calendar source WITH occurrenceDate", () => { + const { d, post, calendarCost } = seeded(); + expect(() => + actions.addPurchase(d, purchaseInput(post.id, { recurringId: calendarCost.id, occurrenceDate: "2026-09" })), + ).not.toThrow(); + }); + + test("addPurchase forbids occurrenceDate when source names an anchored cost", () => { + const { d, post, anchoredCost } = seeded(); + expect(() => + actions.addPurchase( + d, + purchaseInput(post.id, { recurringId: anchoredCost.id, occurrenceDate: "2026-09-01" }), + ), + ).toThrow(/occurrence date/i); + }); + + test("addPurchase accepts an anchored source with NO occurrenceDate", () => { + const { d, post, anchoredCost } = seeded(); + expect(() => + actions.addPurchase(d, purchaseInput(post.id, { recurringId: anchoredCost.id })), + ).not.toThrow(); + }); + + test("addPurchase with no source at all is unaffected", () => { + const { d, post } = seeded(); + expect(() => actions.addPurchase(d, purchaseInput(post.id, undefined))).not.toThrow(); + }); + + test("updatePurchase checks the RESOLVED pair against a source it newly sets", () => { + const { d, post, anchoredCost } = seeded(); + const purchase = actions.addPurchase(d, purchaseInput(post.id, undefined)); + expect(() => + actions.updatePurchase(d, purchase.id, { + source: { recurringId: anchoredCost.id, occurrenceDate: "2026-09-01" }, + }), + ).toThrow(/occurrence date/i); + }); + + test("updatePurchase re-validates the STORED source when an unrelated field changes", () => { + const { d, post, calendarCost } = seeded(); + const purchase = actions.addPurchase( + d, + purchaseInput(post.id, { recurringId: calendarCost.id, occurrenceDate: "2026-09" }), + ); + expect(() => actions.updatePurchase(d, purchase.id, { description: "Renamed" })).not.toThrow(); + expect(d.purchases[0]!.description).toBe("Renamed"); + }); +}); + +// REVIEW ROUND 1: pre-existing asymmetries with json.ts, confirmed clean +// against the baseline — neither `requireRecurrence` nor `addRecurringCost`/ +// `updateRecurringCost` validated `recurrence.kind`/`anchoring` against their +// known sets; both relied on TypeScript's unions alone, which do not survive +// a hand-edited object or an `as` cast reaching this file directly. +describe("runtime guards beyond what TypeScript's unions promise", () => { + function draft(): Dataset { + const data = createSeedDataset("2026-01"); + data.posts[0]!.id = "housing"; + return data; + } + + const billInput = { + name: "Bill", archived: false, + amount: { amount: 100, currency: "DKK" }, startDate: "2026-01-01", + recurrence: { kind: "everyNMonths" as const, n: 1 }, + anchoring: "calendar" as const, splitMode: "percent" as const, + splits: [{ postId: "housing", value: 100, absorbsRemainder: true }], + }; + + test("addRecurringCost refuses an unknown recurrence kind bypassing the type checker", () => { + const data = draft(); + expect(() => + actions.addRecurringCost(data, { + ...billInput, + recurrence: { kind: "everyFullMoon", n: 1 } as any, + }), + ).toThrow(/recurrence/i); + }); + + test("addRecurringCost refuses an unknown anchoring bypassing the type checker", () => { + const data = draft(); + expect(() => + actions.addRecurringCost(data, { ...billInput, anchoring: "whenever" as any }), + ).toThrow(/anchoring/i); + }); + + test("updateRecurringCost refuses an unknown anchoring the same way", () => { + const data = draft(); + const cost = actions.addRecurringCost(data, billInput); + expect(() => + actions.updateRecurringCost(data, cost.id, { anchoring: "whenever" as any }), + ).toThrow(/anchoring/i); + }); + + test("updateRecurringCost refuses an unknown recurrence kind the same way", () => { + const data = draft(); + const cost = actions.addRecurringCost(data, billInput); + expect(() => + actions.updateRecurringCost(data, cost.id, { recurrence: { kind: "everyFullMoon", n: 1 } as any }), + ).toThrow(/recurrence/i); + }); +}); diff --git a/src/store/actions.ts b/src/store/actions.ts index 27414ab..6926dda 100755 --- a/src/store/actions.ts +++ b/src/store/actions.ts @@ -204,6 +204,7 @@ export function addPurchase( draft: Dataset, purchase: Omit, ): Purchase { + requireSourcePairing(draft, purchase.source); const created: Purchase = { ...purchase, id: newId(), @@ -228,6 +229,12 @@ export function updatePurchase( ): void { const purchase = requirePurchase(draft, purchaseId); const resolved: Partial> = { ...changes }; + // The RESOLVED pair, same `??` style `updateRecurringCost` uses for + // anchoring/recurrence above: the incoming `source` if this update sets + // one, else the one already stored — so touching an unrelated field still + // re-validates whatever source is already there, and a `source` this + // update DOES set is checked against the cost it now names. + requireSourcePairing(draft, changes.source ?? purchase.source); if (changes.total) { resolved.total = roundMoneyValue(draft, changes.total); } @@ -375,6 +382,66 @@ function requireRecurringCost(draft: Dataset, id: RecurringCostId): RecurringCos return cost; } +/** + * REVIEW ROUND 1 / I2: `addPurchase`/`updatePurchase` used to accept an + * arbitrary `source` unchecked — only `confirmOccurrence` built a coherent + * one. That let the store write a dataset that would fail its own + * re-import, which is exactly the drift the boundary-agreement requirement + * exists to close. A later task adds a control that hand-builds a `source` + * and passes it straight to `addPurchase`, so this has to be closed here, at + * the store layer, rather than becoming a validation that control's own task + * has to carry. + * + * Mirrors the pairing `json.ts` enforces at the import boundary: a `source` + * naming a calendar cost REQUIRES an `occurrenceDate` (a charge against a + * fixed grid settles one slot); a `source` naming a `lastCharge` cost + * FORBIDS one (an anchored cost has no grid, so a slot id would describe a + * schedule this app will not compute). `undefined` (no source at all) is + * always fine and skips this entirely. + */ +function requireSourcePairing(draft: Dataset, source: Purchase["source"]): void { + if (source === undefined) return; + const cost = requireRecurringCost(draft, source.recurringId); + if (cost.anchoring === "lastCharge") { + if (source.occurrenceDate !== undefined) { + throw new Error( + `"${cost.name}" is anchored to its last charge and has no slots to settle, so a purchase ` + + `referencing it must not carry an occurrence date`, + ); + } + } else if (source.occurrenceDate === undefined) { + throw new Error( + `"${cost.name}" is calendar-anchored, so a purchase referencing it must carry the occurrence ` + + `date (slot) it settles`, + ); + } +} + +/** + * REVIEW ROUND 1: pre-existing asymmetry with `json.ts`'s `RECURRENCE_KINDS` + * check — this file relied on the `Recurrence` union alone, which only binds + * a caller that goes through the type checker. A hand-edited object or an + * `as` cast reaches this function directly (this is the store's write path, + * not the JSON one), so the three known kinds are checked here too now. + */ +const RECURRENCE_KINDS = new Set(["everyNMonths", "everyNDays", "everyNWeeks"]); + +/** Mirrors `json.ts`'s `ANCHORINGS` check for the same reason: the `Anchoring` + * union only binds a typed caller. */ +const ANCHORINGS = new Set(["calendar", "lastCharge"]); + +/** + * Guards against a value TypeScript's `Anchoring` union would otherwise + * guarantee — but only for a caller the type checker actually saw. A + * hand-edited object or an `as` cast reaches this function directly. + */ +function requireAnchoring(anchoring: Anchoring): Anchoring { + if (!ANCHORINGS.has(anchoring)) { + throw new Error(`Anchoring must be "calendar" or "lastCharge", not "${anchoring}"`); + } + return anchoring; +} + /** * The projection walk in `domain/occurrences.ts` terminates only if every step * strictly advances, and `n` is what guarantees that. The JSON importer checks @@ -382,6 +449,9 @@ function requireRecurringCost(draft: Dataset, id: RecurringCostId): RecurringCos * fold can hang. */ function requireRecurrence(recurrence: Recurrence): Recurrence { + if (!RECURRENCE_KINDS.has(recurrence.kind)) { + throw new Error(`A recurrence must be "everyNMonths", "everyNDays", or "everyNWeeks", not "${recurrence.kind}"`); + } if (!Number.isInteger(recurrence.n)) { throw new Error(`A recurrence interval must be a whole number, not ${recurrence.n}`); } @@ -442,13 +512,15 @@ export function addRecurringCost( draft: Dataset, cost: Omit, ): RecurringCost { + const anchoring = requireAnchoring(cost.anchoring); const recurrence = requireRecurrence(cost.recurrence); - requireStartDateGranularity(recurrence, cost.startDate, cost.anchoring); + requireStartDateGranularity(recurrence, cost.startDate, anchoring); const created: RecurringCost = { ...cost, id: newId(), order: draft.recurring.length, recurrence, + anchoring, amount: roundMoneyValue(draft, cost.amount), splits: roundSplits(draft, cost.splits, cost.splitMode, cost.amount.currency), }; @@ -473,20 +545,16 @@ export function updateRecurringCost( const cost = requireRecurringCost(draft, id); const resolved: Partial> = { ...changes }; - if (changes.recurrence) { - resolved.recurrence = requireRecurrence(changes.recurrence); + if (changes.anchoring) { + requireAnchoring(changes.anchoring); } - // Runs on every update, not just one that touches recurrence or startDate: - // either field alone can break the pairing (a kind flip against an - // untouched month-only startDate is exactly how this bug was reached), and - // an anchoring flip against an untouched month-only startDate is the same - // shape of bug one level up. - requireStartDateGranularity( - resolved.recurrence ?? cost.recurrence, - resolved.startDate ?? cost.startDate, - changes.anchoring ?? cost.anchoring, - ); + // REVIEW ROUND 1 / M2: checked BEFORE requireStartDateGranularity below. + // A charged cost's anchoring change is refused outright, no matter what + // else is wrong with the request — checking the date pairing first meant + // flipping a charged month-only calendar cost to lastCharge reported the + // date problem, the owner "fixed" the date and resubmitted, and only then + // learned the change was never allowed at all. if (changes.anchoring && changes.anchoring !== cost.anchoring) { // Switching calendar -> lastCharge could strip the now-forbidden slot ids // mechanically, but the reverse cannot: there is no honest way to invent @@ -502,6 +570,20 @@ export function updateRecurringCost( } } + if (changes.recurrence) { + resolved.recurrence = requireRecurrence(changes.recurrence); + } + // Runs on every update, not just one that touches recurrence or startDate: + // either field alone can break the pairing (a kind flip against an + // untouched month-only startDate is exactly how this bug was reached), and + // an anchoring flip against an untouched month-only startDate is the same + // shape of bug one level up. + requireStartDateGranularity( + resolved.recurrence ?? cost.recurrence, + resolved.startDate ?? cost.startDate, + changes.anchoring ?? cost.anchoring, + ); + if (changes.amount) { resolved.amount = roundMoneyValue(draft, changes.amount); }