diff --git a/examples/folio/sanderling/predicates.ts b/examples/folio/sanderling/predicates.ts index db36849..27bee6d 100644 --- a/examples/folio/sanderling/predicates.ts +++ b/examples/folio/sanderling/predicates.ts @@ -249,16 +249,54 @@ export function acrossRelaunch(lastAction: ObservedAction | null): boolean { // well have landed. Leaving it out is what convicted a healthy app: // committedTransactionsExceedSubmits saw a transaction rise of one against a // window of zero and called it a double submit. +// +// A submit the app must have refused does not count, for the mirror reason: it +// cannot have committed anything, so the bound it would raise is slack the app +// can hide a real double submit behind. See submitCouldCommit for what "must +// have refused" is allowed to mean. export function countSubmitsInWindow(args: { previousCount: number; lastAction: ObservedAction | null; + amountText?: string; fresh: boolean; }): { reported: number; next: number } { const { previousCount, lastAction, fresh } = args; - const reported = previousCount + (isTxnSubmitTap(lastAction) ? 1 : 0); + // A relaunch is the one thing that can put a form state on screen other than + // the one the tap read, so the field it draws proves nothing about it. + const refused = !acrossRelaunch(lastAction) && !submitCouldCommit(args.amountText); + const reported = previousCount + (isTxnSubmitTap(lastAction) && !refused ? 1 : 0); return { reported, next: fresh ? 0 : reported }; } +// Could the app have committed anything for that submit? The amount field as +// the LANDING frame shows it is the form state the tap read: the tap changes +// nothing about it, and one action runs per step, so nothing else could have. +// Off the transaction screen there is no field to read, and undefined is +// unknown, which counts. +// +// False only where Folio's own code must have refused. parseCents takes +// `^\d+(\.\d{1,2})?$` with commas stripped and refuses everything else, and +// AddTransactionViewModel refuses a parsed zero on top of that. An empty field +// never even reaches the parser: TxnSubmit is +// clickable(enabled = amount.isNotBlank()), so the click does not fire. +// +// This is the difference between a bound and a useless one. The window is an +// upper bound on the transactions the interval could hold, and a bound inflated +// by taps that commit nothing is a bound the app can never exceed: the iOS run +// in #78 read a rise of 15 transactions against a window of 37 submits and had +// nothing to say. Measured over four recorded android runs, 19, 11, 25 and 25 +// of 35, 26, 42 and 42 submit taps landed with the amount field empty. +// +// An amount too large for a Kotlin Long is refused by the app too, and still +// counts here: over-counting can only cost a detection, and the reading that +// would have to prove the overflow is a float that cannot hold the number. +export function submitCouldCommit(amountText: string | undefined): boolean { + if (amountText === undefined) return true; + const trimmed = amountText.trim().replace(/,/g, ""); + if (!/^\d+(\.\d{1,2})?$/.test(trimmed)) return false; + return /[1-9]/.test(trimmed); +} + // Parses formatCents output like "$5.00", "-$1,234.56", "+$0.50" back to // integer cents. Anything that is not a complete amount is null, not 0: a // balance we could not read is unknown, and reading it as zero silently moves diff --git a/pkg/spec/test/folio-submit-window.test.ts b/pkg/spec/test/folio-submit-window.test.ts index 6e19281..4774165 100644 --- a/pkg/spec/test/folio-submit-window.test.ts +++ b/pkg/spec/test/folio-submit-window.test.ts @@ -67,6 +67,96 @@ test("a second submit with no Home reading between them counts two", () => { ); }); +// The window is a budget: an upper bound on the transactions the interval could +// hold. A tap the app's own parser must have refused spends none of it, and on +// android it does not even reach the parser, because TxnSubmit is +// clickable(enabled = amount.isNotBlank()). Measured over four recorded android +// runs, 19, 11, 25 and 25 of 35, 26, 42 and 42 submit taps landed on the +// transaction screen with the amount field empty, so more than half the budget +// was being spent on taps that cannot commit anything. +test("a submit the app must have refused does not spend the window's budget", () => { + for (const amountText of ["", " ", "0", "0.00", "00", "5.", "abc"]) { + assert.deepEqual( + countSubmitsInWindow({ + previousCount: 0, + lastAction: { kind: "Tap", on: submitOn }, + amountText, + fresh: false, + }), + { reported: 0, next: 0 }, + `amount ${JSON.stringify(amountText)} was counted as a possible commit`, + ); + } +}); + +// The field as the landing frame shows it, which is the form state the tap read: +// nothing between the two changes it. Anywhere but the transaction screen there +// is no field to read, and unknown has to count. +test("an amount that could commit, or that nobody could read, spends the budget", () => { + for (const amountText of ["5", "0.01", "1,000", "999999999999999999999", undefined]) { + assert.deepEqual( + countSubmitsInWindow({ + previousCount: 0, + lastAction: { kind: "Tap", on: submitOn }, + amountText, + fresh: false, + }), + { reported: 1, next: 1 }, + `amount ${JSON.stringify(amountText)} was dropped from the budget`, + ); + } +}); + +// The one thing that can put a different form state on screen than the one the +// tap read: the runner restarting the app, which the tap survives and the typed +// amount does not. The field a fresh process draws is empty whatever was +// submitted, so it proves nothing and the submit keeps its place in the budget. +test("a submit across a relaunch spends the budget whatever the field shows", () => { + assert.deepEqual( + countSubmitsInWindow({ + previousCount: 0, + lastAction: { kind: "Tap", on: submitOn, applied: true, relaunched: true }, + amountText: "", + fresh: false, + }), + { reported: 1, next: 1 }, + ); +}); + +// What the budget costs the counting invariant, in the shape of the iOS run in +// #78: a stretch of the walk that never went Home, most of it taps on a submit +// button with nothing typed into the form, and one double tap that committed +// twice. Counting the refused taps hands the app five transactions of slack it +// never used, and two rows against six actions is no violation. +test("refused submits used to hide a double submit behind their own budget", () => { + const frames = [ + { amountText: "", lastAction: { kind: "Tap", on: submitOn } }, + { amountText: "", lastAction: { kind: "Tap", on: submitOn } }, + { amountText: "", lastAction: { kind: "Tap", on: submitOn } }, + { amountText: "", lastAction: { kind: "Tap", on: submitOn } }, + { amountText: "", lastAction: { kind: "Tap", on: submitOn } }, + { amountText: undefined, lastAction: { kind: "DoubleTap", on: submitOn } }, + ]; + let budget = 0; + for (const frame of frames) { + budget = countSubmitsInWindow({ + previousCount: budget, + lastAction: frame.lastAction, + amountText: frame.amountText, + fresh: false, + }).next; + } + assert.equal(budget, 1); + assert.equal( + committedTransactionsExceedSubmits({ + countsBefore: { Checking: 3 }, + countsAfter: { Checking: 5 }, + submitsInWindow: budget, + }), + true, + ); +}); + // The two traces the freshness rule exists to tell apart, driven step by step // through the same pair of carriers the spec holds. function run(steps: { route: string | null; totalText?: string; lastAction: unknown }[]) {