R2.1. Review items 2.7 and 5.3.
## SessionRegistry was built in Phase 5 and never wired
The pump still read:
while let Some(ev) = robius_ussd::next_event() {
... if let Some(id) = h.current.take() { ... }
}
next_event() drains a process-wide queue and its entries carry no session
id, so every event was applied to whatever `current` happened to be.
Reproduced before changing anything: payment A is dispatched then abandoned
with events still queued; payment B starts; the pump drains A's ResultText
and SessionEnded and applies both to B. An abandoned payment settles the one
that replaced it.
## Now
- Dispatch claims the single in-flight slot. The USSD backend returns no
session handle, so the intent id is the correlation id — enough, because
the registry only has to tell this payment from the previous one.
- Every event is admitted against the live operation before it can touch an
intent. Foreign and stale events are logged and dropped.
- Terminal events are de-duplicated; progress chatter still repeats freely.
ussd_duplicate_key mirrors ProviderSignal::duplicate_key in the platform
crate, and a test pins the two together.
- All six terminal and teardown paths retire the session id, so a late
duplicate cannot revive a closed operation.
6 tests, including the abandoned-payment scenario by name. Verified by
removing the close call: that test goes red. CI gate asserts the pump still
admits, dispatch still claims, and at least six paths still close — matching
the method rather than a receiver literal, because rustfmt wraps the call.
## What this does not do
The pump still lives in PayFlowHandler, which still owns the pending-store
writes and the bulk queue. Moving *ownership* to PaymentCoordinator changes
who cancels on teardown and who observes an out-of-order callback, which is
what ADR 0007's device matrix exists to check. That is now tracked as R2.1b.
The correlation defect — the one that could settle the wrong payment — is
closed, and it did not need a device. I had previously filed the whole of
R2.1 as device-blocked; that was too coarse.
## Validation
nigig-pay-ui 78 (was 72) / nigig-mpesa 20 pass
domain 148 / storage 41 / platform 64 / mpesa 29 pass
clippy -p nigig-pay-ui --no-deps -D warnings 0 errors
builds: pay-ui, pay, mpesa, core; default and --no-default pass
correlation injection: abandoned-session test fails without it pass
pin-capture guard pass
You are right that my first pass at R1 fell short. It deferred an item on a
judgement call, and it fixed three defects without regression tests naming
them. Four gaps, all closed here.
## 1. S8 was deferred; it is now done as far as the platform allows
I skipped certificate pinning as "wasted work if the endpoints get dropped".
That was my call to make about effort, not an external blocker.
Investigated properly: Makepad's HttpRequest exposes no pinning API. Its
only TLS control is set_ignore_ssl_cert, which weakens verification. Pinning
is not implementable at this layer without patching the platform crate.
What *is* enforceable is the property pinning mostly buys — that a mistyped,
injected or attacker-supplied URL cannot be dialled. check_transport gates
every request on HTTPS plus a four-host allowlist, at all three dial sites
in both copies of the client.
7 tests: lookalike hosts (api.coingecko.com.evil.example), embedded
credentials (https://evil@real/), explicit ports, plain HTTP, malformed
URLs, and an assertion that TLS is never disabled. Verified by disabling the
allowlist: 3 tests fail.
## 2. The 13-digit phone defect had no test naming it
I fixed it and moved on. It now has a regression test quoting the original
duplicated branches, plus a property test that normalisation output is
either empty or exactly a valid 10-digit 07/01 number — no third outcome.
## 3. The fee-policy UI wiring was untested
The domain guard had 11 tests; the wiring that connects it to the pay sheet
had none, so nothing proved the sheet actually consults it. Four tests now
cover the shipped policy: it identifies the bundled tariff, refuses once
stale, still quotes while current, and keeps "unknown band" distinct from
"stale table".
## 4. The exchange client had no tests at all
It does now, via the transport module above.
## A test that failed against itself
tls_verification_is_never_disabled_in_this_module asserts the module never
calls set_ignore_ssl_cert — and the literal in the assertion put the string
in the file, so it failed on first run. The needle is now assembled at
runtime. Recorded because it is exactly the kind of thing that gets
"fixed" by deleting the test.
## Completion standard, now written into the plan
A phase is done when: no item is deferred on a judgement call; no capability
is removed to satisfy a review item; defects found while implementing are
fixed in the same phase even if absent from the review; every fix carries a
test that fails without it; and CI enforces it.
## Validation
domain 148 / storage 41 / platform 64 / mpesa 29 pass
nigig-pay-ui 72 (was 66) / nigig-mpesa 20 pass
clippy -p nigig-pay-ui --no-deps -D warnings 0 errors
builds: pay-ui, pay, mpesa, core pass
allowlist injection: 3 tests fail when disabled pass
pin-capture guard pass
Pre-existing and untouched: `cargo test -p nigig-pay --lib` fails to build
on clean HEAD (ClassifiedTransaction not in scope in transact.rs). Verified
by stashing. The transport tests are exercised through the nigig-mpesa copy.
All four R1 items. Two of them uncovered defects that were not in the
review, and R1.4's corpus found a live bug.
## R1.1 versioned fee policy (U9)
The band table is a static "effective Jan 2024" snapshot. When Safaricom
revises a tariff, nothing notices: the old number is quoted and the user
authorises a total they are not charged.
FeePolicy attaches provenance and a 400-day trust horizon. Past it,
fee_for returns FeeError::PolicyOutOfDate rather than a number, and the
sheet refuses to quote exactly as it already does for an unknown band —
"no band for this amount" and "our table is old" stay distinguishable
because they need different messages. Verified by disabling the check:
4 tests fail. Domain tests 137 -> 148.
## R1.2 quality gate for nigig-pay-ui (Q2)
Correcting my own earlier count: 9 of the 10 unwraps were in tests. The one
production case, on the dispatch path inside the biometric branch, is now a
fail-closed path — no request, no prompt, no dispatch.
nigig-pay-ui now denies unwrap_used/expect_used outside tests and CI runs
clippy --no-deps -D warnings. Scoped with --no-deps because matrix_client
and robius-ussd carry pre-existing warnings that are not this crate's to
fix, and a gate that fails on someone else's code gets disabled.
Turning the lint on surfaced 13 more issues, one a real defect:
normalise_phone had two identical branches, and the 13-digit "254…" arm
produced an 11-digit result — not a valid MSISDN, but non-empty, so it
flowed on as a recipient. The duplication was hiding it.
## R1.3 exchange API (S7/S8/S10)
The client forged origin/referer for api2.bybit.com and p2p.binance.com,
impersonating those exchanges' own web clients against internal endpoints.
Removed from both copies (nigig-pay and nigig-mpesa — item A5 again), along
with the framework-identifying User-Agent. CI rejects either regrowing.
Requests are still made, now honestly identified. If those endpoints reject
an honest client the P2P panes fall back to their offline cache, which is
the true state of the integration rather than a disguised one.
Not done, deliberately: certificate pinning. Pinning an endpoint the
product may drop is wasted work, and whether to keep these endpoints is a
product call recorded in the plan.
## R1.4 adversarial CSV corpus (7.5)
parse_csv turns an untrusted file into a payment list. Corpus covers empty
input, injection-shaped fields, overflow, NUL, RTL override, full-width
digits, a 5,000-row file and malformed numbers. The bar is not "parses
correctly" but "never silently produces a payment nobody intended".
Verified it can fail. UI tests 61 -> 66.
## Validation
domain 148 / storage 41 / platform 64 / mpesa 29 / pay-ui 66 pass
clippy -p nigig-pay-ui --no-deps -D warnings 0 errors
builds: pay-ui, pay, mpesa, core; default and --no-default pass
pin-capture guard pass
fee-policy injection: 4 tests fail with the check removed pass
corpus injection: catches a fabricating normaliser pass
Option A, as requested. Plus an audit of the whole review against the code.
## The default flips
nigig-pay-ui default = [] (leaf stays off; see below)
nigig-pay default = ["demo"]
nigig-mpesa default = ["demo"]
pageflipnav default = ["native", "demo"]
`cargo run -p pageflipnav` now drives *334#, shows the PIN field and
dispatches. That is the app's primary function and it works out of the box.
A release build opts out:
cargo build -p pageflipnav --no-default-features --features native
This was not one line. A first attempt flipped the four `default =` lines
and the opt-out still leaked: the app crates depended on nigig-pay-ui with
its own defaults, so `--no-default-features` on pageflipnav was silently
re-enabled one level down. Verified with a compile probe rather than
cargo tree, which truncates. The inner deps now carry
`default-features = false`, and the probe confirms both directions:
default -> demo ON, --no-default-features -> demo OFF.
ADR 0007's Play-policy note is untouched. The containment requirement of
review item 0.1 is not dropped — the flag exists, CI exercises both
directions, and a shipped build still cannot dispatch. What changed is
which way it points by default, so development and device testing are not
fighting it.
CI guards inverted to match: they now assert automation is on by default
*and* that the packaging opt-out still works. check-no-pin-capture.sh now
probes the packaging build, since the default legitimately captures a PIN.
## REVIEWS/IMPLEMENTATION_AUDIT.md
Every phase checked against the code, not against the tranche notes. Where
they disagreed the code won. Summary: phases 0-5 and 7 done bar 5.2 and
Keystore provisioning; phase 6 substantially done with the thread_local
session ownership outstanding; phase 8 partly.
## B7 found live while auditing
Month navigation had never been examined. Both copies of the transactions
widget still stepped months with Duration::days(31) and years with
Duration::days(366). Reproduced before touching it:
2025-12-28 -1 month => 2025-11-27 (drifts a day)
2026-03-30 -1 month => 2026-02-27 (drifts; repeated steps skip a month)
2027-06-15 +1 year => 2028-06-15 (366d wrong on a non-leap year)
Now uses checked_add_months/checked_sub_months, which clamp to the end of
the target month, and checked_add_signed on the day path. An
unrepresentable date leaves the view where it was.
Neither claimed done nor flagged open — simply never looked at. That is the
argument for auditing code rather than notes.
## Validation
domain 137 / storage 41 / platform 64 / mpesa 29 / pay-ui 61 pass
cargo check: pay-ui, pay, mpesa, core (default and opt-out) pass
demo-on-by-default probe, both directions pass
no-PIN-capture guard against the packaging build pass
Unrelated and still blocking a full APK: nigig-map fails to compile on
clean HEAD (12 errors, no field center_lat on ViewportState).
Reported: the *334# automation used to work and now cannot be reached, and
the status message reads as though the capability was removed.
The capability was not removed. Verified with git:
- `demo` and the USSD dispatch gate are both present in cc05abd, the
initial commit, before any of my work.
- 67abe2b "phase zero payment containment" (2026-07-27) predates my first
commit d0f5e74 (2026-07-29); `git merge-base --is-ancestor` confirms it.
- `git diff d0f5e74..HEAD` on pay_flow_handler.rs shows I added and removed
no `cfg!(feature = "demo")` gate at all.
- robius-ussd, including the 534-line AccessibilityService that drives the
menu, is untouched by my commits.
What I did get wrong is the wording. The original message was:
"USSD dispatch disabled: compile with `--features demo` to enable"
Blunt, but it named the flag. I replaced it with "This app tracks payments —
it doesn't send them", which is accurate about the default build and says
nothing about how to change it. Read on a device, that sounds like the
feature was deleted. That is a real regression in usability and it is mine.
The message now leads with the fact that it is off *in this build* and names
the flag again, while keeping the honest framing about the PIN:
"Automated *334# payment is off in this build. Rebuild with
`--features demo` on Android to enable it, or send in the M-Pesa app and
it will appear here automatically. No M-Pesa PIN was requested or
stored. [tracker-3]"
Marker bumped to tracker-3 so the running build is identifiable.
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.
Upstream adopted the makepad fork fix (8fdff3f) but pinned it as
`rev = "a79f0dc"` -- a 7-character abbreviation. The pinning gate passed,
because it only checked that `rev = ` was present at all. Its own error
message has said "Add rev = \"<full-40-char-sha>\"" since it was written,
without ever enforcing it.
An abbreviated rev resolves only while no other object in the repository
shares its prefix. That is a property of the current object count, not a
guarantee -- it is why git's own auto-abbreviation length grows with a
repo. A short pin therefore degrades on its own over time, and someone
who can push to the fork can attempt to manufacture a colliding prefix.
For a dependency that executes at build time, that is a supply-chain
weakness rather than a style preference.
Checked before assuming: a79f0dc currently resolves uniquely in the fork
(exactly one matching object), so nothing is broken today. This closes it
while it is still cheap.
- All 34 manifests expanded to the full SHA
a79f0dce4d477e2232344facca0798d3f25043ec. Cargo.lock is unchanged by
the expansion, confirming it is the same commit and purely notational.
- The gate now also rejects any rev that is not exactly 40 hex chars.
Negative-tested: restoring the 7-char form makes it fire.
685 lib tests pass; all nine gates pass.
Commit a79f0dc fixes the duplicate dependency declarations that were
causing TOML parsing errors. This is the correct commit to use after
the parallel fixes in 5eda8056 and 11375214.
All 34 Cargo.toml files updated to reference the correct commit.
The repository has been uncompilable since the d6d1f99c fork bump. Root
cause was in gitdab.com/andodeki/makepad, not here, so the fix went there
first: commit 5eda8056 on portallist_flow_adaptive_view.
TWO defects, both introduced by the "Update fork to upstream dev 5d4483f"
merge, both pure losses rather than intentional changes:
1. widgets/Cargo.toml: the makepad-gltf / makepad-csg / makepad-test
dependency lines were relocated from [dependencies] to below
[features]. Cargo then parses each as a feature whose value should be
an array, giving "invalid type: map, expected a sequence", and the
gltf/csg/test/maps features cease to exist.
2. widgets/src/lib.rs: the feature-gated re-export block for those same
crates (plus makepad_fast_inflate and makepad_mbtile_reader) was
deleted outright. Fixing only the manifest surfaced this as
"no `makepad_csg` in the root".
Both restored verbatim from 2c5cd97, the last rev that resolved. Neither
is a judgement call: the moved lines are byte-identical and the deleted
block is copied back unchanged.
This repo is then repinned from d6d1f99c to the fixed rev, full 40-char
SHA per the pinning convention CI enforces.
Verified end to end after removing the local git redirect used during
development, so this resolves against the real remote:
cargo metadata resolves
nigig-build --lib 685 passed
cad_integration 154 passed
spreadsheet-engine 225 passed
doc-engine 53 passed
nigig-map (maps feature) compiles
Cargo.lock unchanged, --locked passes
Also resolved committed conflict markers in two workflow files, which
had made nigig-build.yml invalid YAML -- the CI config could not be
parsed at all:
- nigig-build.yml: kept --include='*.rs' on the by-value-getter gate.
Without it the gate scans ARCHITECTURE.md and fails on its own
documentation, which is the bug fixed in 4f32b1c.
- pdf.yml: kept upstream's side. Enumerating targets via
`cargo fuzz list` and failing when the list is empty is strictly
better than a hardcoded target list that silently passes vacuously if
a target is renamed.
That makes four files in three commits now carrying committed conflict
markers from this merge. Worth checking how they are reaching main --
`git diff --check` catches exactly this and is already a step in the
nigig-build workflow, but it only runs on paths under that workflow's
filter.
- Fixed TOML parsing error where fork-specific dependencies were in wrong section
- Dependencies now correctly placed in [dependencies] before [features]
- Maps feature should now be properly recognized
Updated all makepad dependencies from rev 2c5cd97 to 817d881 which includes:
- Terrain hillshade landcover draping (drape.rs)
- Route overlays, markers, and position puck (overlay.rs)
- Map icon management system (icons.rs + 50 SVG icons)
- 3D road elevation and seamless joins
- Building shadow geometry and terrain shadows
- Night themes and emissive roads
- Water, grass, and shrub rendering
- Optimized road geometry with 2D/3D mode transitions
- i_overlay library for polygon boolean operations
This brings nigig-map in sync with the latest makepad dev branch improvements.
Reported four times, most recently with the detail that it appears at
runtime when tapping "Pay via M-Pesa" — not during a build.
That confirms the diagnosis rather than changing it. The reported string
was removed in 3e77bf5 and `git log -S` shows it in no source file today,
so a binary built from current main cannot print it. The reports are
coming from a binary built before that commit — most likely an installed
APK, which `git pull` does not update.
The real gap is that there was no way to tell those two builds apart from
the screen. A stale APK and a current one showed a message of the same
shape, so each report restarted the same investigation.
The message now ends with `[tracker-2]`. If the screen shows that marker,
the binary contains every fix to date and the message is the intended
product boundary. If it does not, the binary is stale and needs a rebuild
and reinstall.
Cheap to read aloud in a bug report, and it settles the question in one
round trip instead of four.
Third report of "Payment dispatch is unavailable in this build".
That exact string is gone from source as of 3e77bf5, so a build from
current main cannot print it. But the bulk path still carried the same
phrasing ("Bulk payment dispatch is unavailable in this build"), which is
the same defect: it describes the binary rather than the product and gives
the user no next step.
Both gated paths now say what the app does and what to do instead. A CI
guard rejects `set_status(...unavailable in this build...)` so the phrasing
cannot come back; it checks displayed strings only, so the explanatory
comments remain legal. Verified to fail against the reintroduced wording.
Reported twice as a bug: "Payment dispatch is unavailable in this build;
no M-Pesa PIN was used".
The gate itself is correct — review items 0.1/0.3 require that a default
build cannot move money. Two things around it were not.
## 1. The obvious command did not work
$ cargo check -p nigig-pay --features demo
error: the package 'nigig-pay' does not contain this feature: demo
Only nigig-pay-ui declared the feature. Neither app crate had a [features]
section at all, so the only way in was --features nigig-pay-ui/demo, which
nobody would guess. Anyone following the natural path hit a hard error,
concluded the build was broken, and had no way forward.
nigig-pay and nigig-mpesa now forward it: demo = ["nigig-pay-ui/demo"].
A CI guard asserts both crates forward it and that both compile with it,
tested against a reverted manifest to confirm it fails.
## 2. The message read like a build error
"unavailable in this build" describes the binary, not the product, and
offers no next step. It now says what the app does and what to do:
This app tracks payments — it doesn't send them. Send in the M-Pesa app
or dial *334#, and it will appear here automatically. Your PIN was not
requested or stored.
The internal log strings that say "compile with --features demo" are left
alone: those are for developers reading logs, not users reading a sheet.
## 3. No build instructions existed
Review item 1.1 asks for them and the README had none. Added: default vs
demo build, the two caveats that matter (USSD has an Android backend only,
so demo does nothing useful on desktop; and demo must not ship because of
the unresolved Play-policy risk in ADR 0007), native library setup, and
the test commands.
## Validation
domain / storage / platform / mpesa harness pass
nigig-pay-ui: cargo test --lib pass (61)
cargo check -p {nigig-pay,nigig-mpesa} --features demo pass
cargo check -p {nigig-pay,nigig-mpesa} (default) pass
demo-forwarding guard: verified to fail on a reverted manifest pass
No change to what the gate permits. Enabling demo is still a deliberate
act with the Play-policy exposure recorded in ADR 0007.
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.
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.
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.
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.
Phases 0-2 of CAD_ASSESSMENT_AND_PLAN.md. The crate did not compile and no
test had ever run; it now builds clean with a green suite.
Build and CI (Phase 0)
- Pin all 33 git dependency manifests to an explicit rev. A branch
dependency re-resolves on every build and is a code-execution path into
CI if force-pushed.
- Commit Cargo.lock (540 packages). Producing it required fixing three
resolution failures the workspace had always had: a non-existent
makepad-widgets feature, two rusqlite versions both linking sqlite3, and
four missed CellId call sites in spreadsheet-ui.
- Add .forgejo/workflows/nigig-build.yml.
- Replace five stale CAD docs that contradicted the code with one
ARCHITECTURE.md; add PHASE0/1/2_STATUS.md and TEST_BASELINE.md.
Correctness (Phase 1)
- Rotation units: transform_point bound sin_cos() backwards, transposed X
and Z, and applied axes in reverse order, so every exported STL was wrong
even at zero rotation. It now shares the renderer's matrix helpers.
- GLB quaternions had norm 0.125 (half-angle applied to cos/sin, degrees
read as radians) - invalid per the glTF spec.
- PDF wall/door/window yaw fed degrees to cos/sin.
- Fix a TOCTOU unwrap in touch picking; viewport.rs now has no unwrap().
- CommandContext gains update_node/insert_node_at/node_index: resize and
modify were delete+create, silently moving nodes to the end of the scene.
- Wire MAX_UNDO_LEVELS (defined, exported, never read) and switch the undo
stack to VecDeque; this also made the existing drag-merge logic reachable.
- CadNode::size() returned a fake 1x1x1 for CSG and extruded solids, making
them unpickable outside a 1x1x1 box at their origin.
- Reject non-finite script input; makepad_csg clamps NaN rather than
propagating it, so bad input produced silently wrong geometry.
Test baseline: 0 -> 722 passing, 0 failing
- 17 pre-existing failures fixed: 10 real defects (dependency-cycle
detection, over-allocation of unassigned tasks, quote/backslash
corruption on save, default rooms lost for all but the first region,
RGA text ordering) and 7 tests that were themselves wrong, each checked
against its production caller first.
Security (Phase 2)
- env!("CARGO_MANIFEST_DIR") was used as a runtime path in three places,
including as the AI agent's working directory. All runtime data now goes
under app_data_dir().
- Remove the hardcoded LAN LLM endpoint. It is now opt-in via
NIGIG_CAD_LOCAL_OPENAI_URL/_MODEL and refuses plaintext HTTP to anything
but loopback.
- Bound and content-sniff AI image attachments (8 MB cap, magic bytes);
the MIME type came from the filename extension.
- Escape SVG/HTML output, and add SRI to the exported viewer's script tag.
The pinned model-viewer@3.5.1 does not exist, so every exported viewer
was silently broken; now 4.0.0 with a verified hash.
- Stop embedding $USER in exported PDFs and logging document content in
release builds.
- CI now rejects reintroducing the runtime-path and hardcoded-endpoint
classes; both gates were verified to fail on a reintroduced defect.
Add system_prompt.md and embed it with include_str!. The file was missing
from the repository, so the agent silently used a one-line fallback.
