Commit graph

13 commits

Author SHA1 Message Date
775eec4424 fix(pay): remove PIN capture from default builds rather than hiding it (0.3)
Some checks failed
Payment domain, storage, platform and UI / isolated-payment-tests (push) Has been cancelled
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
Asked in review: "does the latest code have the PIN input field?" It did.

## Hiding was weaker than it looked

pin_input, form_pin and the reveal toggle were all present, and the default
build hid them at runtime in on_after_new. Three problems:

1. The DSL declared the control visible and Rust hid it afterwards, so
   anything re-applying the UI definition — a hot reload, a re-instantiated
   sheet — brought it back. Defect B11 already names this class.
2. Hidden is not absent. The TextInput stayed in the widget tree, and a
   hidden input can still be focused or filled programmatically.
3. form_pin was compiled into every build, so any path reaching it could
   populate it.

Item 0.3 asks for removal, not concealment.

## The field no longer exists without `demo`

form_pin, pin_visible, the eye-toggle handler, the text-input handler, both
PIN checks, the request construction and try_build_ussd_request are all
#[cfg(feature = "demo")]. A default build has no field to write to.

The DSL now declares visible: false on the PIN input and its reveal button,
so hidden is the default state rather than a runtime correction; demo
unhides them on init. That closes the reload path in (1).

One runtime call remains as belt-and-braces: if a hot-reloaded definition
surfaces the input, the non-demo arm wipes what was typed. It has no
form_pin to clear, because there isn't one.

## Proving absence rather than asserting it

A grep for form_pin proves nothing — it passes just as happily against a
field still present behind a runtime if. tools/check-no-pin-capture.sh is a
compile probe: it references the field outside any cfg block and requires
the default build to fail with "no field `form_pin`" while demo succeeds.
The script restores the file on every exit path.

Verified both directions: passes on current code, exits 1 when the field is
re-exposed un-gated.

## Validation

  cargo check -p {nigig-pay-ui,nigig-pay,nigig-mpesa,nigig-core}  pass
  cargo check -p {nigig-pay,nigig-mpesa} --features demo          pass
  nigig-pay-ui: cargo test --lib                                  pass (61)
  domain / storage / platform / mpesa harness                     pass
  no-PIN-capture guard: verified to fail on a re-exposed field    pass

## What 0.3 still leaves open

The Java-side KEY_PIN scrubbing was already done, so 0.3 is complete for the
default product. A demo build still captures a PIN by design — that path
exists for authorised device testing and is covered by the unresolved
Play-policy decision in ADR 0007. If that decision goes against the USSD
rail, the demo path and its PIN capture are deleted with it.
2026-08-02 07:31:44 +00:00
b5e38825fa fix(pay): validate money at the persistence boundary (B6)
Some checks failed
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
Payment domain, storage, platform and UI / isolated-payment-tests (push) Has been cancelled
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
B6 is the last unfinished P0 on the review's priority table.

## Why the boundary and not the format

The obvious reading of B6 is "change f64 to Money on disk", which is a data
migration and belongs with the SQLite move. But measuring first shows the
stored representation is not where the damage is:

- The f64 round trip is exact. 1500.0, 1500.5, 0.1+0.2, 1e20 and
  12345678.995 all write and re-read bit-identically.
- The read path is the damage. parts[2].parse().unwrap_or(0.0) turned any
  unreadable amount into a confident KSh 0 row.
- And NaN gets in. "NaN" and "inf" both parse as f64 and the writer emits
  them back verbatim, so they survive a round trip. One corrupt SMS then
  poisons every total it enters, permanently — once a NaN is in a sum,
  every comparison against that sum is false.

## What changed

validate_money refuses three classes, on parse, on load and on save:
non-finite; negative (direction lives in TransactionType, so a negative
amount is a contradiction); and beyond 2^53, where f64 can no longer
represent consecutive shillings.

A row whose amount cannot be trusted is skipped with a log line rather than
zeroed. Dropping a row is visible and recoverable by rescanning the inbox;
a silent KSh 0 is neither. balance and cost are optional context, so they
degrade to None rather than discarding the row, but can no longer be NaN.

The writer refuses to persist an invalid amount, which is what stops a
poisoned value becoming permanent.

Smaller parse fix: "Ksh ,5" used to read as 5. A separator before any digit
means the text is not the expected shape, and guessing is worse than
declining.

## The parser had no tests

parser.rs — the file deciding what every observed amount is — had zero
tests. It now has 13: validation, extraction, end-to-end parsing, and
unicode/NUL bodies that must not panic on a byte-index slice.

M-Pesa harness: 7 -> 24 tests.

## Verifying the tests can fail

validate_money was reduced to Some(value) and the harness re-run: 4 tests
failed. A guard that cannot fail is decoration.

## Validation

  mpesa harness: 24 tests                                        pass
  domain  : 137 tests --locked, fmt, clippy -D warnings, bench   pass
  storage : 36 + 41 sqlcipher --locked, fmt, clippy              pass
  platform: 56 + 64 ussd --locked, fmt, clippy, mock guard       pass
  nigig-pay-ui: cargo test --lib                                 pass (61)
  nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core: check     pass
  authorization / batch / settlement-tick guards                 pass
  defect injection: 4 tests fail with validation removed         pass

## What B6 still leaves open

MpesaTransaction::amount is still f64 on disk. That is deliberate and now
bounded: nothing untrustworthy can enter or leave the store, so the
remaining exposure is precision within the validated range, which for
whole-shilling amounts under 2^53 is none. Converting the field means
rewriting the PSV format and migrating existing files. ADR 0002 records B6
as partially addressed with the migration deferred to the SQLite move.
2026-07-29 04:48:50 +00:00
cf878d9e3e feat(pay): coordinator entry point for an externally granted authorization
Some checks failed
Payment domain, storage, platform and UI / isolated-payment-tests (push) Has been cancelled
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
The API change tranche 10 scoped, in REVIEWS/adr/0007.

## dispatch_with_authorization

Tranche 10 established that Android cannot implement BiometricAuthorizer
without reopening defect S2, and built AuthorizationAttempt as the
replacement. It named the remaining work: a coordinator entry point taking
an already-granted attempt instead of calling biometric.authenticate().

  coordinator.dispatch_with_authorization(&id, &mut attempt)

It does not prompt, does not wait, and does not consult the injected
BiometricAuthorizer at all. That is asserted rather than assumed: one test
injects an authorizer reporting no hardware that also errors, and shows a
granted attempt still dispatches. If the coordinator ever fell back to it,
that test fails.

Three refusals, each with a test that fails when the gate is removed:

- An unanswered prompt does not dispatch. PromptShown plus SensorEngaged
  is not consent — the exact shape the Android adapter would produce.
- A grant belongs to one payment. A foreign grant is refused before
  anything else, so a stray callback cannot even fail the payment on
  screen; the victim intent stays in AwaitingUserAuthorization rather
  than being transitioned to Failed by a stranger.
- A grant is spent on use. One authorization, one dispatch (B3).

Denied, still-prompting and already-spent attempts all fail the intent
closed and never reach the gateway.

## Verifying the tests can fail

The gate was removed (if !attempt.consume() -> if false) and the suite
re-run: 5 tests failed. A fail-closed test that passes against fail-open
code is worthless, so this is the check that matters.

Worth recording: a_grant_cannot_dispatch_twice still passed with the gate
removed, because the existing duplicate-dispatch budget caught it
independently. Two unrelated mechanisms refuse the second dispatch. That
is defence in depth working, and the reason that test is not sufficient
evidence on its own.

## Validation

  domain  : 137 tests --locked, fmt, clippy -D warnings, bench    pass
  storage : 36 + 41 sqlcipher --locked, fmt, clippy               pass
  platform: 56 + 64 ussd --locked, fmt, clippy, mock guard        pass
  nigig-pay-ui: cargo test --lib                                  pass (61)
  nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core: check      pass
  authorization / batch / settlement-tick guards                  pass
  fail-open injection: 5 tests fail with the gate removed         pass

Domain tests 129 -> 137.

## What remains

The authorization half is done and verified. PayFlowHandler still owns the
USSD session lifecycle, the pending-store writes that shadow the
coordinator's repository, and the bulk queue plumbing. Moving those makes
the coordinator own the gateway session, which changes who cancels on
teardown and who observes an out-of-order callback — ADR 0007's device
matrix, which cannot be exercised here.
2026-07-29 04:10:59 +00:00
cbafa92269 fix(pay): close the fail-open biometric the coordinator migration would open
Some checks failed
Payment domain, storage, platform and UI / isolated-payment-tests (push) Has been cancelled
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
Review defect S2, plus an amendment to ADR 0007.

## Attempting the migration found a defect in the plan

The stated next step was replacing PayFlowHandler with PaymentCoordinator,
which means writing a BiometricAuthorizer adapter for Android. It cannot
be written correctly, and why is the substance of this change.

The trait contract is blocking: "authenticate must be a blocking call that
waits for user action. Returns Ok(()) on success."

robius_fingerprinting::authenticate does not do that. Reading through
sys/android/prompt.rs: it calls the Java authenticate static method and
returns Ok(()) as soon as the prompt is on screen. The user's answer
arrives later through next_event().

So an adapter can either return Ok(()) when the prompt opens — and
authorize_and_dispatch then sends money before the user has touched the
sensor, which is defect S2 reintroduced through the type system — or
block, and deadlock the thread that must pump the callback.

Had the migration been done without noticing, the result would have been a
correct-looking refactor that silently reopened the review's most serious
security finding.

## AuthorizationAttempt

Authorization is a state machine, not a function call:
Requested -> Prompting -> Granted | Denied, advanced by inbound signals.
One rule, enforced by the type: only an explicit success grants dispatch.

- A displayed prompt does not authorise. A touched sensor does not
  authorise. A non-match keeps the prompt up and stays retryable.
- A grant is bound to one intent, so a callback for an abandoned payment
  cannot authorise the current one.
- A grant is spent on use, so one fingerprint cannot authorise two
  dispatches (B3), and a replayed success cannot re-arm it.
- A late success after a cancel is ignored, not resurrecting the payment.
- Backgrounding mid-prompt denies; it never silently allows.

dispatch_ussd now consumes a grant before dispatching and refuses without
one. auth == None means no biometric gate was configured, which is
deliberately distinct from an ungranted one. Both cancel paths abandon the
grant.

A CI guard asserts the gate exists and was tested with the check disabled
to confirm it fails.

The trait keeps its blocking contract for synchronous authorizers and test
doubles, and now documents that Android must not use it.

## Validation

  domain  : 129 tests --locked, fmt, clippy -D warnings, bench    pass
  storage : 36 + 41 sqlcipher --locked, fmt, clippy               pass
  platform: 56 + 64 ussd --locked, fmt, clippy, mock guard        pass
  nigig-pay-ui: cargo test --lib                                  pass (61)
  nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core: check      pass
  authorization guard: verified to fail with the gate removed     pass
  batch-counter and settlement-tick guards: still passing         pass

Domain tests 117 -> 129.

## Where the migration stands

The blocker is no longer unknown. PaymentCoordinator needs an
authorization path that does not assume a blocking authorizer, and
AuthorizationAttempt is that path, built and tested. What remains is a
coordinator entry point taking an already-granted attempt instead of
calling biometric.authenticate() itself, then moving USSD session
ownership across.

That is an API change that should be designed against ADR 0007's device
matrix — permission denial, cancellation, backgrounding, app restart,
out-of-order callbacks — none of which can be exercised here.
2026-07-29 03:57:07 +00:00
b4ad9348d3 fix(pay): make batch progress bounded and derived, not counted
Some checks failed
Payment domain, storage, platform and UI / isolated-payment-tests (push) Has been cancelled
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
Review items 6.6 / U7, and B3 at batch scope.

## A progress bar that could read 3/2

While working toward retiring the thread_local handler, the bulk
accounting turned out to hold a defect worth fixing on its own.

bulk_done was a bare usize incremented at eight independent match arms,
none bounded, and bulk_current_idx added one more while an item was in
flight. Nothing tied the counter to the batch contents.

The USSD pump does not guarantee one terminal event per item — an Error
can be followed by a SessionEnded for the same session, and each arm
increments separately:

  one item, two terminal events -> bulk_done=2/2
  BulkItemDispatched reports        3/2   <-- exceeds total

That was reproduced before changing any code, so this answers a
demonstrated defect rather than a suspected one.

## PaymentBatch

Progress is now a function of the batch contents, not of how many code
paths ran. Each item carries exactly one BatchItemOutcome and settle()
refuses a second instead of counting it, which makes finished > total
unrepresentable rather than merely unlikely. Every former bulk_done += 1
routes through one settle_bulk_item helper.

A CI guard rejects direct increments and was tested against the
reintroduced bug to confirm it fails, same as the tick guard in tranche 8.

Three properties follow from putting this somewhere testable:

- finished is not succeeded. confirmed counts only provider
  confirmations, so a batch that ended with unknown outcomes reports
  "Batch finished — N confirmed, M awaiting confirmation. Do not
  re-send" and never carries a tick. Only all-confirmed may be ticked.
- A pending item is never offered for re-run. safe_to_recreate returns
  only NotSent and Rejected; replaying a possibly-settled item is B3.
- Cancelling does not rewrite history. cancel_remaining marks only items
  that never got an outcome, because cancelling a batch does not recall a
  request the provider may already hold.

A stray callback for an unknown item id is refused rather than counted —
the batch-level counterpart of ADR 0007's correlation rule.

## Validation

  domain  : 117 tests --locked, fmt, clippy -D warnings, bench    pass
  storage : 36 + 41 sqlcipher --locked, fmt, clippy               pass
  platform: 56 + 64 ussd --locked, fmt, clippy, mock guard        pass
  nigig-pay-ui: cargo test --lib                                  pass (61)
  nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core: check      pass
  batch-counter guard: verified to fail on the reintroduced bug   pass

Domain tests 104 -> 117.

## On the thread_local, again

Still there; this change did not remove it. What changed is that the state
carrying correctness risk — the batch accounting — no longer lives in it.
PayFlowHandler now delegates every outcome to PaymentBatch, so the
thread-local holds queue plumbing and platform handles rather than the
numbers a user is shown.

That is a smaller and more honest claim than "Phase 6 wiring complete".
Replacing PayFlowHandler with PaymentCoordinator moves ownership of the
USSD session and the biometric prompt, and belongs in its own change with
ADR 0007's device matrix, not folded into one that also touches
accounting.
2026-07-29 02:55:49 +00:00
a6e8f4c04e fix(pay): remove the last false-success labels, and fix B6 where it is real
Some checks failed
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
Payment domain, storage, platform and UI / isolated-payment-tests (push) Has been cancelled
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
Continues Phase 6 (REVIEWS/adr/0008) and addresses defect B6.

## Two labels were still claiming settlement

Tranche 7 noted the sheet's remaining status strings were composed
inline. Two of them were not untidiness, they were the Phase 6 violation
still shipping:

  ObservedEvidence     -> "✓ SMS received — ref: {code} …"
  BulkItemObserved     -> "✓ {current}/{total} sent"

A tick is a settlement claim. The first put one beside an SMS, which
proves nothing and can be forged. The second counted dispatches as
confirmations, so five unknown outcomes rendered as "✓ 5/5 sent".

FlowStage now owns the vocabulary next to the presentation types, with
one rule: a tick requires a provider confirmation. Evidence reads
"Message received (ref X) — not yet confirmed. Do not send again until
this is resolved." A finished batch reads "Batch finished — N sent,
awaiting confirmation", which is what actually happened.

Nine call sites derive their text instead of composing it. The two
remaining ✓ in that file are fingerprint-sensor feedback — a real local
event, not a settlement claim.

A CI guard backs this, and it was tested against the old code to confirm
it fails when the regression returns. A guard that cannot fail is
decoration.

## B6, scoped to where it is real

Each f64 site was measured before changing it, because "replace f64
everywhere" is easy to claim and easy to get wrong.

format_amount was checked exhaustively from 0.00 to 2000.00 against an
exact integer reference: zero mismatches. It also rounds 1234.567 and
2.675 correctly. It is not the defect and rewriting it would be churn.

Accumulation is the defect. 10,000 realistic amounts showed no visible
drift, but f64 silently discards additions past 2^53 and offers no
overflow signal at all, so a corrupt record yields a confident wrong
total with nothing to indicate it.

So the fix went to the accumulators. update_summary in nigig-mpesa and
nigig-pay — exact duplicates of each other, review item A5 — now build a
PeriodSummary from exact minor units with checked arithmetic, and render
"—" rather than a wrapped figure when overflowed is set. The conversion
from stored f64 is explicit and validated.

store.rs::totals() has no callers; it is now #[deprecated] with the
reason rather than deleted, since nigig-core has no domain dependency and
that is a separate change.

Two regression tests state it executably: f64 silently swallows 2^53 + 1
while the exact path keeps both entries, and an impossible total is
reported rather than shown.

## Validation

  domain  : 104 tests --locked, fmt, clippy -D warnings, bench   pass
  storage : 36 + 41 sqlcipher --locked, fmt, clippy              pass
  platform: 56 + 64 ussd --locked, fmt, clippy, mock guard       pass
  nigig-pay-ui: cargo test --lib                                 pass (61)
  nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core: check     pass
  settlement-tick guard: verified to fail on the old code        pass

Domain tests 95 -> 104.

## Not claimed complete

The thread_local PayFlowHandler still exists. The status vocabulary is now
domain-owned, which was the part carrying correctness risk, but the
widgets still do not talk to PaymentCoordinator. That remains the Phase 6
architectural item. The parser's stored f64 amount/balance/cost are
unchanged: that is a migration, not an edit, and belongs with SQLite.
2026-07-29 02:45:32 +00:00
750d856668 feat(pay): Phase 6 truthful payment states, and run UI tests in CI
Some checks failed
Payment domain, storage, platform and UI / isolated-payment-tests (push) Has been cancelled
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
Phase 6 and item 7.3 of REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md.
Rationale in REVIEWS/adr/0008.

## The rule, made into a type

Phase 6's exit criterion is that the UI cannot call a transaction
successful without trusted confirmation, or failed without known
rejection. The old code violated it structurally: the sheet set its
status inline in about a dozen places, each from whatever local signal
was nearest, so there was nowhere to put the rule.

The worst instance is U4. A payment whose confirmation SMS had not
arrived in five minutes was marked Failed, the sheet closed, and the user
saw "✗ Payment failed" — with a retry button — while the money was gone.

payment_view_state.rs makes the rule a type. PaymentPresentation has no
Success variant reachable from untrusted evidence and no Failed variant
reachable from a missing SMS. VerificationExpired, Unknown and
UnknownNeedsReconciliation all present as PendingConfirmation: not
success, not failure, and no retry offered. Two defence-in-depth rows:
Confirmed without a provider reference reads as unknown rather than
settled, and an unclassified failure is not evidence nothing was sent.

6.2: ConfirmationSummary makes every required term a mandatory field, so
a confirmation missing the fee or total is not constructible. It is built
from the same quote that gets dispatched. material_digest() binds consent
to the exact terms shown and is re-checked on OK — if anything material
moved in between, the authorisation is void and the screen is shown
again. The modal previously showed only recipient and amount.

6.3: must_stay_open() keeps pending payments visible, and the status line
begins with the exact required wording, asserted by a test.

6.5: evidence rows are exposed and labelled untrusted; pending payments
offer receipt, problem-reporting and data-deletion actions.

## Three defects found by writing the tests

1. Bulk quotes silently under-charged. compute_bulk_costs used filter_map
   over the fee lookup, so a contact outside the tariff was dropped from
   the fee total and the user was quoted less than they would pay.
2. The batch total could overflow — .sum() panics in debug, wraps in
   release. A wrapped total is a quote for the wrong amount.
3. The cost preview showed unknown fees as free via unwrap_or(0).

All three are B5/B6 territory. Item 3 is B5 resurfacing in the preview
path after tranche 1 fixed it in the dispatch path: fixing a defect at one
call site is not the same as fixing the defect.

## A pre-existing failing test, diagnosed rather than deleted

money::tests::ksh_rounds_to_nearest_cent asserted format_ksh(1.005) ==
"1.01" and had been failing on every run — confirmed pre-existing by
stashing this work and re-running clean.

The expectation is impossible, not the formatter wrong: 1.005 has no
binary representation, the nearest f64 is 1.00499999999999989..., so the
correctly rounded result is 1.00. This is defect B6 at its smallest. The
test now says so, with a companion showing Money handling it exactly.
Deleting it would have hidden a live argument for finishing B6.

## 7.3 UI tests in CI

The NIGIG_TEST_PAY gate was already gone; what blocked CI was the Makepad
link step. tools/makepad-native-libs.sh listed the libraries needed to
compile but not libasound2-dev, libpulse-dev and libssl-dev, which are
needed to link a test binary — cargo check succeeds and then
"unable to find library -lasound" appears much later. The helper now
installs and checks them, and a payment-ui-tests job runs the UI tests
plus cargo check on nigig-pay-ui, nigig-pay and nigig-mpesa.

## Validation

  domain  : 95 tests --locked, fmt, clippy -D warnings, bench   pass
  storage : 36 + 41 sqlcipher --locked, fmt, clippy             pass
  platform: 56 + 64 ussd --locked, fmt, clippy, mock guard      pass
  nigig-pay-ui: cargo test --lib                                pass (61)
  nigig-pay-ui / nigig-pay / nigig-mpesa: cargo check           pass

Domain 75 -> 95 tests. UI 55 -> 61, with the long-standing failure fixed.
Every new CI step was run locally before commit.

## Not claimed complete

Phase 6 wiring is partial. PaymentViewState exists and is tested, and the
confirmation path uses ConfirmationSummary, but the sheet's remaining
status strings are still set inline and the thread_local PayFlowHandler
still exists. The type makes that migration mechanical; it does not
perform it. 6.6 bulk stays demo-gated pending ADR 0007's unresolved 5.2.
2026-07-29 02:27:44 +00:00
d0f5e74435 feat(pay): Phase 5 platform gateway boundary, and verify Phase 4
Some checks failed
nigig-map / test (push) Has been cancelled
Payment domain, storage and platform / isolated-payment-tests (push) Has been cancelled
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
Phase 5 of REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md. Design and the one
item engineering cannot close are in REVIEWS/adr/0007.

## Phase 4 is now verified, not just written

Tranche 5 implemented the draw_walk fixes and said plainly that the
nigig-pay widget edits were unbuilt. That blocker was environmental:
installing the packages tools/makepad-native-libs.sh already lists makes
the UI graph compile. Both commands the status doc listed as required
now pass:

  cargo check -p nigig-pay      pass
  cargo check -p nigig-pay-ui   pass

No code changed for this; the claim is now evidence rather than assertion.

## Phase 5: new nigig-pay-platform crate

ADR 0002 reserved this crate and marked it "not yet created".

5.1 One crate owns the seam. It is the only crate in the payment stack
that may name a platform SDK. CI enforces both directions: payment crates
may not import Makepad, and domain/storage may not import jni or the
robius platform crates.

5.3 Correlation is mandatory and single-flight. SessionRegistry admits an
event as Accepted, Duplicate or Ignored; PlatformEvent cannot be built
without a CorrelationId. Closed session ids are retired permanently, so
an abandoned session's confirmation cannot settle the payment that
replaced it. There is a test named after exactly that scenario.

2.8/5.3 Progress decides retry safety, not error kind. classify_failure
takes the failure and the DispatchProgress reached before it, and
progress is the authority. The same TemporarilyUnavailable is safely
retryable before the dial and ambiguous once the menu is being driven —
the distinction the old code could not make, which is defect B3's
mechanism. A property test asserts across the whole failure space that
nothing which may have reached the provider authorises a fresh attempt.

5.4 Fakes cannot ship. MockGateway is cfg-gated, is a compile_error! in a
release build unless allow-mock-in-release is named explicitly, and
stamps every session id with MOCK-. CI asserts the release build fails.

5.5 No unsafe, no PIN. The crate is #![forbid(unsafe_code)] so the JNI
surface stays in robius-ussd. UssdGateway is !Send/!Sync by construction,
making the main-thread requirement a compile error. The adapter leaves
the pin field empty and a test asserts it.

5.6 The web claim is withdrawn. No browser API can drive USSD and a
Daraja credential must never reach a browser, so WebGateway refuses every
call and maps to Fatal — "never sent" — which owes no reconciliation.

7.5 Adversarial SMS corpus. StrictMpesaSms is the payment-boundary
reader, deliberately separate from nigig-core's permissive tracker parser
(ADR 0007 explains why this is not the duplication ADR 0002 forbids). It
requires an exact 10-char code, exact sender-ID match so MPESA-REFUNDS
and FAKE-MPESA are refused, rejects fractional shillings instead of
rounding, and caps body length. Corpus covers spoofing, forged code
shapes, out-of-range amounts, unicode and NUL injection, and replay. The
closing test asserts the honest limit: a well-crafted forgery is still
only evidence, because the output type has no settled state to reach.

## 5.2 is not done and is not closeable here

The AccessibilityService Play-policy review is a business decision. ADR
0007 records it as blocking, states the termination exposure, and names
what must happen before the rail is enabled. USSD dispatch stays behind
the default-off demo feature. If the review fails, ADR 0001's
tracker/launcher position applies and only the dispatch adapter is lost.

## Validation

  platform: 56 tests, 64 with --features ussd, fmt, clippy -D warnings
            (both feature sets), cargo-deny, mock-in-release guard
            asserted to fail                                      pass
  domain:   75 tests --locked                                     pass
  storage:  36 + 41 tests --locked, incl. sqlcipher                pass
  nigig-pay, nigig-pay-ui: cargo check                            pass

cargo-deny reports advisories/bans/licenses/sources ok. The isolated
runner gained a `platform` target and it runs in CI on every push that
touches the crate.
2026-07-29 02:07:19 +00:00
ecaac4bcb7 feat(pay): complete Phase 3, start Phase 4
Some checks failed
Payment domain and storage / isolated-payment-tests (push) Has been cancelled
Phase 3 (secure repository) is now complete.

3.3 retention and erasure — new retention.rs:
- Data minimisation and financial record keeping pull opposite ways, so the
  policy states both: evidence expires 90 days after settlement, settled
  records are held for a five-year floor, never-dispatched intents expire in
  30 days, and anything awaiting reconciliation is never swept. Deleting an
  unresolved payment would destroy the only record of money that may have
  left the account.
- erase_payment_on_request honours a user deletion request without
  destroying the accounting record: it normally erases the parsed copy of
  the user's inbox and retains the financial record, erases fully only past
  the floor, and refuses outright while in flight. Refusals carry a reason.

3.4 backup, restore and corruption reporting:
- integrity_check and foreign_key_check surface damage as StorageError
  ::Corrupt rather than letting a damaged ledger be trusted.
- backup_to uses SQLite's online backup API instead of copying the file, so
  a concurrent writer cannot produce a torn copy, and refuses to back up a
  database that fails its integrity check.
- restore_from validates the candidate in a separate connection before
  touching the live database, so a failed restore cannot destroy a working
  ledger. Tested with both a missing and a corrupt backup.

Storage tests 20 -> 36.

Phase 4 (blocking I/O) — 4.1 to 4.3 implemented:
- 4.1 expenses/mod.rs called reload(), a full parse of the store from disk,
  as the first statement of draw_walk; scrolling re-read the whole ledger
  every frame. Now loaded once, with invalidate()/reload_if_stale() on the
  event path.
- 4.2 the SMS scan was a wall-clock check inside draw_walk, so the branch ran
  every frame and the scan landed mid-paint. Now a cx.start_interval timer on
  the event path, with the interval named rather than inline.
- 4.3 update_summary walked the filtered list and rewrote labels every frame.
  Now invalidated only in refresh_filtered and applied in handle_event, fully
  out of the draw path rather than guarded inside it.
- 4.4 already satisfied by PaymentStorageWorker since tranche 1.

Supporting domain work: view_state.rs adds PeriodSummary and
TransactionsViewState so the summary arithmetic is testable outside a widget.
The tests immediately pin two things the inlined code got wrong: totals now
report overflow instead of wrapping, and a net outflow returns None rather
than a clamped zero. Money gains Default (zero).

Domain tests 66 -> 75.

Validated on rustc 1.97.1: domain and storage each pass test --locked, fmt,
clippy -D warnings and cargo-deny; storage also --features sqlcipher; domain
benches run; 7 M-Pesa store tests pass; 49 manifests parse.

NOT VERIFIED: the nigig-pay widget edits for 4.1-4.3 are not compiled here.
That crate needs the git-hosted Makepad fork. cargo check -p nigig-pay on a
full developer checkout is required before Phase 4 can be called complete.
2026-07-27 15:34:38 +00:00
c6e122c3fc feat(pay): complete Phase 1 and fix M-Pesa store defects B1/B4
Some checks failed
Payment domain and storage / isolated-payment-tests (push) Has been cancelled
Phase 1 of NIGIG_PAY_CONSOLIDATED_REVIEW.md is now closed.

1.1 build governance:
- Declare license = "MIT" on both payment crates. cargo-deny correctly
  reported them as unlicensed, which would block any distribution review.
- Version-pin the nigig-pay-domain path dependency; a bare path dependency
  is a wildcard requirement.

1.2 quality gates:
- Add deny.toml and a CI job running cargo-deny over both payment crates.
  Advisories, bans, licences and sources all pass. The config bans the
  makepad-* crates outright and restricts sources to crates.io.

1.3 canonical ownership (ADR 0002):
- Record the domain/storage/platform/UI layering and its one-way deps.
- The review's A5 "fork farm" table is stale: one copy each of parser.rs,
  store.rs, pending_store.rs and pay_flow_handler.rs, not three.
- Fix B1: store.rs parsed category, sub_category, status and confidence
  from disk then overwrote them with Default::default(), losing every user
  categorisation on reload. Persistence also wrote display names, which are
  not reversible, so this adds stable storage tokens with a legacy-display
  fallback so existing rows still load.
- Fix B4: clean/restore mapped '|' to '~' and reversed every '~', so
  "JOHN~DOE" loaded as "JOHN|DOE". Replaced with bijective backslash
  escaping covering the separator, newlines and carriage returns.
- B6 (f64 money) deliberately deferred to Phase 6: it is a type change that
  ripples into UI consumers.

These were previously recorded as untestable because nigig-core is not a
workspace member. That was wrong: the three files involved need only serde,
chrono, one log! macro and one app_data_dir() helper. The new
tools/test-mpesa-store-clean.sh supplies those shims in a throwaway crate
and runs 9 tests, two of which reproduced the defects before the fix.

1.4 boundary: enforced twice, by a CI manifest/import check and by the
deny.toml ban list.

1.5 shims: three re-exports in nigig-pay/src/lib.rs had zero callers and are
deleted. The remaining four carry a caller count and a named migration
target so they have a deletion plan rather than an open-ended lifetime.

1.6 scope (ADR 0001): accepted that Nigig Pay is a read-only tracker and
launcher, not a payment processor, until an authorised provider integration
exists. This is the decision the review required before further UI work.

SECURITY: a live Cloudflare API token was found committed in README.md,
present since the initial commit and pushed to a public remote. Removed and
recorded as R-SEC-001 in PAYMENT_RISK_REGISTER.md. Redaction does not revoke
it; it remains in history and must be rotated by the owner.

Validated on rustc 1.97.1: domain and storage each pass test --locked, fmt,
clippy -D warnings and cargo-deny; storage also passes --features sqlcipher;
domain benches run; 9 M-Pesa store tests pass. 49 manifests parse.
2026-07-27 15:23:20 +00:00
01bc055d1e feat(pay): complete the partially-done review phases
Some checks failed
Payment domain and storage / isolated-payment-tests (push) Has been cancelled
No new phase was started. This finishes every item tranches 1-2 left
partial, so Phases 1, 3, 7 and 8 are complete for the two pure crates or
explicitly blocked on crates that cannot be built here.

Phase 1 (build governance):
- Check in Cargo.lock for both payment crates and build with --locked.
  .gitignore excluded them, which would have made item 1.1 a false claim.
- tools/test-rust-clean.sh now reads the channel from rust-toolchain.toml
  instead of a hardcoded default that had already drifted, and gates on
  fmt, clippy -D warnings, the sqlcipher feature and the benchmarks.
- CI enforces lockfile presence/freshness and a Makepad-boundary check that
  inspects manifests and use/extern lines rather than prose (1.4).

Building on the declared 1.97.1 toolchain surfaced five lints 1.85 missed;
all are fixed. The two Default impls are annotated rather than derived
because each encodes a security or state-machine decision.

Phase 3 (secure repository):
- 3.1 encryption at rest: new encryption.rs and an opt-in sqlcipher feature.
  PRAGMA key is applied first and verified by a forced read, so a wrong key
  fails as KeyRejected rather than as corruption. DatabaseKey redacts its
  Debug and zeroes on drop. No key is ever derived or persisted here, and
  there is no unencrypted fallback. A test asserts the recipient MSISDN is
  absent from the raw database bytes.
- 3.5: preferences.rs replaces the ANDROID_DATA marker file whose existence
  was the value; every field fails safe.
- 3.6: redact.rs. PaymentIntent's derived Debug leaked a customer MSISDN
  into any log line; it now masks phone, name and ids, with a regression
  test. StorageError no longer prints a full intent id.

Phase 7:
- 7.7 benchmarks over money, validation, fees, the intent lifecycle and
  evidence handling, using the stable harness so they run on the pinned
  toolchain. BASELINE.md records measured output.
- 7.1/7.2/7.3 verified already satisfied; the review text is stale.

Phase 8:
- 8.1 simulator.rs with a scripted gateway and biometric, no clock or I/O.
- 8.2 property tests over every permutation of a representative event set:
  no ordering dispatches twice, untrusted events never settle a payment,
  ambiguity never becomes failure, Confirmed is terminal, duplicate
  confirmations are idempotent, and replayed SMS cannot fake a conflict.

Validated on rustc 1.97.1 outside the incomplete workspace graph:
domain 66 tests, storage 20 tests, storage+sqlcipher 25 tests, fmt clean,
clippy -D warnings clean on both crates and both feature sets, benches run.
49 manifests parse; git diff --check clean.

Still unclaimed and blocked on uncompilable crates: UI wiring, Keystore key
provisioning, Phase 4 draw_walk I/O, Phase 5 adapters and legal review, the
Java PIN scrub, and B1/B4/B6 in nigig-core.
2026-07-27 15:07:28 +00:00
70eddf42c7 feat(pay): add evidence, event correlation and explicit reconciliation
Some checks failed
Payment domain and storage / isolated-payment-tests (push) Has been cancelled
Implements the review phases left open by the previous tranche in the two
pure crates, per REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md.

nigig-pay-domain:
- evidence.rs: ObservedEvidence/EvidenceLog make SMS and USSD output an
  untrusted observation that can raise a reconciliation flag but can never
  settle a payment (0.4, B2). Replays are de-duplicated by a deterministic
  body digest; only the digest is retained, never the raw body (0.6).
- reconciliation.rs: explicit queue with typed reasons and attributed
  resolutions (2.8, 6.3). SafeToRecreate never mutates or re-dispatches the
  original intent. The user-facing message says "do not retry" and never
  "failed" (U4).
- coordinator.rs: operation_id -> intent index so an unmatched callback
  changes nothing (2.7); hard duplicate-dispatch guard where only a
  never-started dispatch refunds the budget (2.9, B3);
  expire_local_confirmation_window replaces Submitted -> Failed on timeout
  and cannot disturb a settled payment.

nigig-pay-storage:
- schema migration 2 adds payment_intent_evidence with a unique digest
  constraint and a durable payment_reconciliation_queue (3.2, 3.3).
- legacy import now durably queues unprovable records.
- fault-injection tests for unwritable path, corrupt file, unusable worker
  path and audit-insert rollback (3.4); restart-equivalence tests (8.2).

Both crates deny unwrap/expect/panic/indexing outside tests (7.8). That
ratchet caught PaymentStorageWorker::spawn aborting on thread-spawn
failure; it is now fallible and returns StorageError.

Validated in an isolated toolchain over copies of the two pure crates:
cargo fmt --all --check clean, cargo clippy --workspace --all-targets
-D warnings clean, 43 domain tests and 17 storage tests passing, and all
49 workspace manifests parse. The full workspace is not asserted buildable
here: external Makepad/Robius sibling path dependencies are absent, so the
Makepad UI wiring, SQLCipher/Keystore encryption and the Java PIN scrub
remain explicitly unclaimed.
2026-07-27 14:39:15 +00:00
f6750c8259 feat(pay): add domain coordinator and sqlite storage foundation 2026-07-26 18:42:02 +00:00