108 commits

Author	SHA1	Message	Date
nigig-ci	afe7c518b3	docs(ci): nigig-map is green; record what is deliberately not gated ... 16 of 17 jobs now pass. Updates the board and adds two sections: - Known-not-gated: the nigig-map unit-test ratchet (9 real logic failures), the three test/bench targets that do not compile, and the report-only fmt/clippy steps. Written down so nobody reads a green tick as "this crate is healthy". - Writing a ratchet step: the `bash -e` trap that made this workflow fail at exactly its own baseline, and the `|| status=$?` fix. Cheap to record, expensive to rediscover. Also corrects the cad-module note: `cargo fmt -p nigig-build` is 1,559 diffs across 89 files spanning doc, project_management and cost_estimator, not just CAD.	2026-08-04 05:22:14 +00:00
nigig-ci	833181faec	ci(map): fix the ratchet aborting before it could evaluate anything ... First real run of nigig-map.yml reported failure at 530 passed / 9 failed -- exactly the baseline it was supposed to allow. The step ran `out="$(cargo test ...)"` under the runner's `-e` shell. cargo test exits 101 while any test fails, and a failing command substitution in a plain assignment aborts the step immediately, so neither the parse nor the comparison ever executed. The `set -o pipefail` I had added made it worse, not better. `|| status=$?` puts the assignment inside a tested compound command, which -e exempts, so the script keeps control and decides for itself. Verified against the same `bash -e` the runner uses: at baseline 530 passed / 9 failed -> exit 0, "OK" regressed 527 passed / 12 failed -> exit 1, "12 failing ... baseline is 9" restored 530 passed / 9 failed -> exit 0 My bug, introduced in de698b1. The rest of that workflow was sound: the same run proved checkout, native deps, the pinned-toolchain install and the build gate all pass, which is the first time this crate has ever built in CI.	2026-08-04 05:15:00 +00:00
nigig-ci	de698b1a64	ci(map): make the workflow runnable, and cover the code it now guards ... nigig-map.yml has never executed a single step. It used actions/setup-rust@v1, which does not exist on data.forgejo.org, so every run died in "Set up job" with "repository not found" and cancelled all seven steps -- the same class of defect as android-actions/setup-android in sms.yml. Replaced with the inline rustup install already used by pay-domain.yml. That action also requested `toolchain: stable`, contradicting the 1.97.1 pin in rust-toolchain.toml. The replacement reads the channel out of rust-toolchain.toml, so CI and developers use one compiler. Added the native GL/wayland dependencies; Makepad does not build without them. Gates, scoped to what is honestly true today now that the crate compiles: - Build is a hard gate. This is the regression that matters: until the previous commit the crate did not compile at all. - Unit tests are a RATCHET at 9, not a hard gate. 535 unit tests existed and had never run; 526 pass and 9 fail on real logic (4 mvt_parser, 1 overpass_parser, 4 sprite classification). Failing the build on those would mean a permanently red job that everyone learns to ignore. The ratchet fails the moment a tenth appears. - `cargo test` with no filter is NOT used: two of the four test targets and the criterion bench do not compile (tests/ui.rs imports makepad_widgets::makepad_test; tests/makepad_visual_tests.rs and benches/tile_decode_bench.rs import pub(crate) modules, and criterion is not a declared dev-dependency). Separate defects. - fmt and clippy report without gating, matching doc-engine.yml and sms.yml. rustfmt could not parse view.rs while the crate was broken so it skipped all of src/; there are now 392 visible pre-existing diffs and 132 clippy warnings. A step that always fails is worse than no step. Also added four unit tests for center_lat() and meters_per_pixel(). Both were introduced in the compile fix and had zero coverage: I verified that by regressing center_lat() by +1.0 degree and watching the ratchet stay green at 9. It now fails at 12. The tests round-trip the projection across eight latitudes, pin the equator to zero, check hemisphere sign, and assert the ground scale ratio between 0 and 60 degrees is cos(60) = 0.5 -- the position puck's accuracy circle is sized from that, so an inversion would be wrong by 2x at Nordic latitudes. Ratchet negative-tested both ways: perturbing lon_lat_to_normalized takes it 9 -> 12 and fails; at HEAD it reports 530 passed, 9 failed and passes.	2026-08-04 05:06:48 +00:00
nigig-ci	ea7788673d	docs(ci): record the repo-hygiene scheduling bug and the current board ... Adds the fourth defect the first real runs exposed -- the mapping form of `on:` not being scheduled on this instance -- and a table of the latest result for all 17 jobs, so "is CI green" has an answer that is not someone's memory. 14 pass. The two failures, cad-module formatting and nigig-map, are pre-existing source problems rather than CI plumbing.	2026-08-02 10:38:06 +00:00
nigig-ci	4d627496d2	ci: use the list form of on: so repo-hygiene actually runs ... repo-hygiene.yml is the one workflow with no path filter. Its whole purpose is to run on every commit, because the other six are scoped with `paths:` and a commit touching only unfiltered files otherwise gets no checks at all. The file's own header comment explains this, citing commit 8c9ccb9, which pushed 30 conflict-marker lines into two workflow files and silently disabled the CAD gates. It has never run. Not once. Of the first 47 task records after a runner was registered, every other workflow appears and this one does not, across pushes that touched .forgejo/, tools/, crates/ and Cargo.lock. The cause is the mapping-with-null-values form: on: push: pull_request: Valid YAML, both keys parse as None, and it is the spelling GitHub documents for "all branches". This instance does not schedule it. The list form does. So the workflow that exists to catch silently-disabled checks was itself a silently-disabled check.	2026-08-02 10:09:28 +00:00
nigig-ci	89f7c1df2a	docs(ci): how to register a runner, and what the first runs found ... Not in the repo root -- this lives next to the workflows it describes. Covers registering a forgejo-runner against gitdab (Gitea 1.22), the ubuntu-latest label every job depends on, the docker-vs-host tradeoff, and how to read job logs given that this instance's REST API 404s on the logs endpoint. The important section is action resolution: Forgejo resolves `uses:` against data.forgejo.org with no github.com fallback, and a missing action fails the job in "Set up job" before any step runs -- which reads like an infrastructure blip rather than a config error. Records which actions currently resolve and which do not. Also records the four defects the first real runs exposed, three now fixed, so the next person understands why these workflows look the way they do.	2026-08-02 09:58:08 +00:00
nigig-ci	bda0124992	ci(pay): pass --config to cargo-deny's check subcommand, not the binary ... The dependency audit step has never audited anything. cargo-deny 0.18.6 exits 2 immediately: error: unexpected argument '--config' found tip: 'check --config' exists --config belongs to the `check` subcommand, and its path resolves relative to the manifest rather than the working directory, so it also has to be absolute. nigig-build.yml already gets both right; this invocation predates that fix. This step is the last one that runs in isolated-payment-tests, so its failure also skipped the three gates behind it: - Payment crates must not depend on Makepad - Domain and storage must not reach the platform SDK - Mock gateway must not compile into a release build Verified with cargo-deny 0.18.6 against all three crates: nigig-pay-domain advisories ok, bans ok, licenses ok, sources ok nigig-pay-storage advisories ok, bans ok, licenses ok, sources ok nigig-pay-platform advisories ok, bans ok, licenses ok, sources ok Found by running the workflow on a real runner for the first time.	2026-08-02 09:42:46 +00:00
nigig-ci	892471c73f	ci: commit tools/*.sh executable, and gate the mode ... Five of the six scripts under tools/ were committed mode 100644. Every one of them is invoked with a leading ./ from pay-domain.yml or pdf.yml, so those steps could only ever fail: ./tools/makepad-native-libs.sh: Permission denied ./tools/test-mpesa-store-clean.sh: Permission denied Both are real failures from run 349, the first time a runner existed to execute pay-domain.yml at all. They fail at the job's first substantive step, so payment-ui-tests did no work whatsoever and isolated-payment-tests skipped its last seven gates -- including the dependency audit, the "payment crates must not depend on Makepad" check, and the mock-gateway-in-release guard. The mode is a property of the index, so a local chmod that is never staged does not fix it. Marked all five executable with `git update-index --chmod=+x` and added a hygiene gate that fails if any tracked tools/*.sh is not 100755. repo-hygiene.yml is the right home: it has no path filter, needs no toolchain, and already exists to validate CI configuration itself. Gate negative-tested: reverting one script to 100644 fails it with "tools/makepad-native-libs.sh is mode 100644, expected 100755"; restoring the bit passes.	2026-08-02 09:28:52 +00:00
andodeki	1d3e6ab72a	fix(pay): correlate USSD callbacks to the payment that asked for them ... 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	2026-08-02 09:21:45 +00:00
Arena Agent	015cf44422	fix(cad): remove the reachable panics; gate unwrap/expect at 5 ... Phase 6, scoped to what is provable rather than a blanket -D warnings. Measured the CAD module first: ~200 clippy warnings, but the panic family -- the part the plan actually cared about, copying the pay crates' ratchet -- was only 8: 2 unwrap, 6 expect, 0 panic!. Three were real, five are genuine constructor invariants. Fixed: - code_editor.rs x2. `lazy_init_session(); self.session.as_mut().unwrap()` in both draw_walk and handle_event. Correct today, but the guarantee lived across a function boundary the compiler cannot see, so an unwrap sat on a widget draw path waiting for a third caller to forget the prologue -- and a panic there kills the editor with unsaved work in it. Added `editor_and_session()`, which splits the borrow and returns Option, so both sites take an early return instead. I first tried folding init into `get_or_insert_with`. That silently dropped the `keep_cursor_in_view = Once` side effect, which only happens on the create path. Caught it by grepping for the field rather than trusting the refactor; reverted. - cad_scene.rs x1. MeshCache::get_or_build did `.write().expect("mesh cache poisoned")` while every other method on the type already degraded with `if let Ok(..)`. Reachable: the export path calls get_or_build on a spawned thread, so one panicking worker poisoned the lock and the next draw took the UI thread down with it. The cache is pure derived data -- every entry rebuilds from its node -- so a poisoned lock now costs memoisation, not correctness. The mesh is built before the lock is taken, and the double-check still prefers a racing thread's entry so Arc::ptr_eq comparisons stay consistent. Left alone, with reasons: 4 x cad_scene "default material/layer always exists" (SceneBuilder::new inserts both; verified) and 1 x arch_gltf serde_json::to_vec over a Value built in that file. New gate: "No new unwrap/expect in CAD production code", allowlist of 5. A bare count drifts upward quietly and a blanket ban just gets #[allow]-ed, so the count is pinned and each exemption is named in the comment. The gate skips #[cfg(test)] by BRACE DEPTH rather than stopping at the first one. That matters: arch_gltf.rs has production code after two test modules, so the existing panicking-macro gate's "stop at first #[cfg(test)]" awk cannot see line 850 at all. My first attempt used the same awk idiom and reported 4 of 5 -- I only noticed because the number disagreed with clippy. Verified the older macro gate is not currently hiding anything, but it is hiding it by luck. Both negative tests pass: an unwrap added to viewport.rs is caught, and one added to arch_gltf.rs *after* its test modules -- the exact blind spot -- is also caught, named with file and line. 13 gates now, all green. 761 lib + 154 integration tests pass.	2026-08-02 09:20:12 +00:00
nigig-ci	015462b386	ci(sms): install the Android SDK inline instead of a nonexistent action ... A runner was registered against this repo for the first time, so the workflows in .forgejo/ finally executed instead of only ever being run by hand. The android job failed immediately: Unable to clone https://data.forgejo.org/android-actions/setup-android refs/heads/v3: repository not found: Not found. android-actions/setup-android does not exist on data.forgejo.org, and Forgejo does not fall back to github.com for action resolution. The failure happens in "Set up job", before any step runs, which cancels all seven remaining steps. The job reported failure without compiling a single line -- so the Android gate, the only job in this file that sees the ~600 lines of JNI under #[cfg(target_os = "android")], has never checked anything. Replaced with an inline cmdline-tools install, which is the same sequence used to verify these crates by hand and depends only on actions/checkout and actions/setup-java -- both of which do resolve. Verified on the same runner in this run: gates, robius-sms (48 tests), nigig-sms (46 tests, floor gate, clippy ratchet) and supply-chain all pass. nigig-map.yml has the identical defect with actions/setup-rust@v1 and is left alone here.	2026-08-02 09:09:57 +00:00
andodeki	5800beb552	fix(pay): close the R1 gaps against the completion standard ... 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.	2026-08-02 08:52:32 +00:00
nigig-ci	25f32f7870	test(sms): build a real test suite (Phase G) ... 50 tests -> 102, and the two that were there at the start of this work are deleted. Where this started: robius-sms had ZERO tests, and nigig-sms had two -- bulk_sub_tab_default_is_contacts and bulk_sub_tab_variants_distinct. Both asserted a derived Default and a derived PartialEq. Neither mentioned SMS. Neither could fail short of the compiler breaking. That is the defect that produced every other defect in this plan: nothing could prove a change was safe, so nothing was ever deleted and every bug survived contact with review. Property tests (proptest, new dev-dependency) Seven over truncate_preview, format_timestamp, badge_text, and five more over segment_count, the rate limiter and ScheduleRequest. These are the ones that matter, because the hand-written cases in this repo all encode a bug someone had ALREADY found. proptest searches the space instead. I verified that by reinstating the original byte-slicing truncate_preview and confirming prop_truncate_preview_survives_mixed_scripts and prop_truncate_preview_respects_the_char_limit both fail against it -- they would have caught A3 before it shipped. prop_rate_limiter_respects_capacity models the window independently and asserts the invariant across random clock sequences, rather than re-implementing the limiter's own arithmetic in the assertion. Integration tests (2 new files, public API only) robius-sms/tests/sms_pipeline.rs and nigig-core/tests/sms_store.rs go through the public surface the application actually uses. The unit tests inside src/ can see private helpers; these cannot, which is the point -- they catch a refactor that keeps every unit test green while breaking the caller-visible contract. Two of them are privacy canaries. e1_message_bodies_are_never_persisted and e1_no_body_text_reaches_the_serialised_store fail if anyone removes #[serde(skip)] from OfflineSmsMessage.body. Verified by removing it: both fail, the other six pass. Nothing else in the tree would have noticed the inbox silently going back to plaintext on disk. Named regression tests One per defect, named for it -- c1_*, d1_*, d3_*, e1_*, e7_*, a4_*, c3_*, c7_* -- so a future reader goes from a failing test straight to the bug it guards rather than to a git archaeology session. New coverage for logic that had none - build_timeline_items / build_filtered_timeline_items: date-divider placement and the message indices the draw loop uses to index conv_data.messages. An off-by-one there renders the wrong body in the wrong bubble; it had no test at all. - kind_to_offline / kind_from_offline round-trip: the only thing stopping a cached Sent message reappearing as Inbox after a restart, which would flip the bubble to the wrong side of the screen. - normalize_number: what C1 groups on, across five formatting variants plus short codes and alphanumeric senders. MessageKind::from_android_type / to_android_type were hoisted out of sys/android/inbox.rs onto the type, the same way ScheduleRequest::validate was in A4, so the provider mapping is testable off-device. An unrecognised TYPE value is preserved verbatim in Unknown rather than defaulted, and there is a property test asserting the round trip is total over every i32. CI: a test-count FLOOR at 100. A floor rather than a ratchet -- unlike the clippy count, there is no reason to ever want this number to fall. Deliberately NOT faked: the JNI cursor loop, the keystore round-trip and broadcast delivery still need an emulator. A mock returning what I expect would test my expectations, not Android. Those remain called out in the Phase A and E commit messages. Verified: 11/11 checks. 48 robius-sms + 46 nigig-sms + 8 sms_store = 102. clippy -D warnings clean on host and aarch64-linux-android; nigig-sms ratchet holds at 32 (my first draft added an orphaned `use super::*`, caught by the ratchet and removed rather than baselined).	2026-08-02 08:43:16 +00:00
andodeki	a264f53eb7	feat(pay): complete phase R1 of the remaining-work plan ... 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	2026-08-02 08:33:16 +00:00
nigig-ci	1670ddf49c	refactor(sms): delete the dead code and the duplication (Phase F) ... Net -596 lines. No behaviour change except F10, which replaces a label that was lying. F2 -- four copies of one stub backend. apple.rs, linux.rs and windows.rs were BYTE-IDENTICAL 66-line files, and unsupported.rs was the same again. That duplication is what let them drift: Phase C3 had to fix `Error::Unknown` in exactly one of the four, because only one had it wrong. Collapsed into sys/stub.rs, which each platform module invokes. 268 lines become 35 plus one shared definition. The module is cfg'd out on Android, which has a real implementation and would otherwise report the macro as unused under -D warnings. F3 -- TWO dead compose implementations. SmsComposePage (189 lines) was registered in the VM and instantiated nowhere. Separately, the FAB and its compose overlay were left in the DSL as `visible: false` with a comment saying "FAB removed: SMS compose/inbox navigation now lives in SmsActionBar" -- but 102 lines of DSL and 53 lines of handler stayed behind, wired to a button no user can reach. Deleted both, and send_reply() with them: it existed only to serve the unreachable overlay. Compose navigation is SmsActionBar's, as the comment already said. F4 -- a whole second contact subsystem, unreachable. sms_screen.rs carried its own CONTACTS_CACHE, contacts_loaded(), load_contacts_into_cache(), display_name_for_number(), normalize_number(), try_load_contacts() and a contacts_load_attempted field. Nothing called any of it -- the live implementation is in conversations_list.rs. Worth noting the dead copy was also the WRONG one: its display_name_for_number did an O(n) linear scan of the whole phone book per lookup, where the live version is O(1) because cache_contact_number inserts under both the raw and normalised key. F5 -- the page tree was written out twice. sms_bulk_page, sms_schedule_page and sms_more_page were each declared under Desktop AND under Mobile, byte-identical apart from indentation. Any change to a page header had to be made in both places or the layouts silently diverged. Now three named widgets plus a shared SmsPageHeader, referenced from both variants. F8 -- serde, serde_json and robius-location were declared by nigig-sms and referenced nowhere in its sources. F9 -- was_scrolling was read twice per frame from the same portal list; the copy in handle_event was bound and never used. F10 -- the character counter was a hardcoded lie. The old compose page rendered "0 / 160 characters" and never updated it. It died with F3, but the bulk composer -- where the money actually goes -- had no cost indication at all. It now shows live segment count as you type, using segment_count() from Phase A5, because segments are the billing unit and "160" is only right for GSM-7: one emoji forces UCS-2 and drops the limit to 70. This is the only user-visible change in the commit. F1 and F7 were already done, in Phase A (shared cursor.rs) and Phase D1 (I/O out of draw_walk). The deletions orphaned eight imports, which are also removed. Together that takes the nigig-sms clippy ratchet from 49 to 32 -- these were not suppressed, the code they reported on is gone. Verified: 10/10 checks. clippy -D warnings clean on host AND aarch64-linux-android, 28 robius-sms tests, 22 nigig-sms tests, nigig-build still builds, metadata --locked clean.	2026-08-02 08:21:07 +00:00
andodeki	23fce675de	feat(pay): USSD automation on by default; containment moves to packaging ... 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).	2026-08-02 08:06:43 +00:00
nigig-ci	737a3e5d5d	security(sms): encrypt scheduled payloads, report real send status (E3, E7) ... Closes the two Phase E items I had left open and documented as open. E3 -- scheduled message bodies were plaintext on disk. Pending schedules must outlive the process so SmsAlarmReceiver can send them when the alarm fires and so they survive a reboot, so recipient and body go to SharedPreferences. MODE_PRIVATE is the right primitive -- the file is UID-scoped -- but the contents were in the clear, readable by anything running as the same UID and swept into cloud backup by default. Same asset class as the inbox (THREAT_MODEL.md T-I4). Adds SmsScheduleCrypto: AES-256-GCM, fresh IV per value, key generated inside the platform AndroidKeyStore and non-exportable. An attacker with the prefs file but not the keystore gets ciphertext. Deliberately NOT androidx.security.EncryptedSharedPreferences: that is a Gradle dependency, and this crate compiles its Java with bare javac against android.jar (see build.rs), so using it would mean a Gradle build or a vendored jar. AndroidKeyStore and javax.crypto are both in android.jar and give the property that matters. The key is deliberately NOT user-authentication-bound: an alarm fires while the device may be locked and the receiver must decrypt with no user present. This protects against another app and against an extracted backup, which is the threat in scope -- not against someone holding an unlocked handset. Fails CLOSED. If the keystore is unavailable, encrypt returns null and schedule_sms errors rather than writing plaintext. A row that cannot be decrypted -- wrong key after a reinstall, tampering, or written by an older build -- is treated exactly like a missing row and skipped; sending a garbled body would be worse than not sending. E7 -- "sent" was a guess. Both the sentIntent and deliveryIntent arguments were null, so nothing could report back. send_sms returning Ok meant "the JNI call returned", not that the radio accepted the message and certainly not that it arrived -- and the UI rendered that as a tick. A send rejected for no service, no SIM or a throttled radio was indistinguishable from a delivered one. Adds send_sms_tracked, which attaches real PendingIntents and returns a correlating token, plus SmsSentReceiver to collect the platform result and SendOutcome/SendReport to express it: Sent (radio accepted) is now a different value from Delivered (handset acknowledged), and failures carry the RESULT_ERROR_* code. Three details worth recording: - multipart takes ArrayList<PendingIntent>, one entry per part, so the intent is repeated part_count times. Passing null here, as before, meant no status for exactly the messages most likely to fail: the long ones. - the request code is derived from (token, kind), or the two intents collide and the delivery report overwrites the send report. - the broadcast is package-scoped and the receiver registered NOT_EXPORTED, so another app cannot forge a delivery report. If the receiver class is unavailable the send still goes out with null intents: losing the status report is much better than losing the message. Also fixes A5 on the scheduled path. SmsAlarmReceiver still called sendTextMessage directly, so a scheduled message over 160 GSM-7 characters -- or 70 with any emoji -- was silently truncated by the carrier. It now divides and sends multipart, as the Rust send path has since Phase A. CI: two gates, both negative-tested by reverting the fix and confirming they fail. One asserts schedule_sms never writes request.recipient or request.body directly; the other asserts the send path still passes sent/delivery intents in both the single-part and multipart calls. THREAT_MODEL.md T-I4 and the delivery-confirmation row move from open to fixed, with the residual risk stated: callers may still use the untracked send_sms, which remains honest about meaning only "handed to the platform". Tests: robius-sms 25 -> 28. Verified: 13/13 CI checks, clippy -D warnings clean on host and aarch64-linux-android, both new Java classes javac-compile and dex. NOT verified on a device. The keystore round-trip, the broadcast delivery and the token correlation all need an emulator or handset; there is still no CI runner on this repo.	2026-08-02 07:58:44 +00:00
andodeki	9f0e133c4b	fix(pay): the APK crate could not reach the demo flag at all ... Reported: building the pageflipnav APK, the Pay sheet shows no PIN field and reports that dispatch is unavailable, with no way to enable the *334# automation. That is a real defect and it is worse than the earlier feature-forwarding gap. pageflipnav is the crate that produces the APK. It depends on nigig-pay and nigig-mpesa but declared no `demo` feature, so: grep -c demo crates/pageflipnav/Cargo.toml -> 0 The flag was unreachable from the only build that matters. Adding `--features demo` to nigig-pay does not change what the APK contains, so every instruction I gave for enabling the automation was useless to anyone building the real app. pageflipnav now forwards it: demo = ["nigig-pay/demo", "nigig-mpesa/demo"] Verified with a compile probe rather than cargo tree, which truncated its output and initially suggested the wiring had failed: a `#[cfg(feature = "demo")] compile_error!` in nigig-pay-ui fires twice under `cargo check -p pageflipnav --features demo` and zero times without it. A CI guard asserts pageflipnav keeps forwarding the flag, and the README now leads with the pageflipnav command and states plainly that building nigig-pay alone does not affect the APK. Unrelated: nigig-map fails to compile on clean HEAD (12 errors, `no field center_lat on ViewportState`), so a full pageflipnav build is currently blocked by that regardless of this change.	2026-08-02 07:51:25 +00:00
andodeki	775eec4424	fix(pay): remove PIN capture from default builds rather than hiding it (0.3) ... 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
Arena Agent	cb5def965b	fix(cad): exports reported success on a failed write ... Every file export buffered its output through a BufWriter and never flushed it. BufWriter flushes on drop and DISCARDS any error it hits doing so, so a write that fails only when the buffer is pushed to disk -- full disk, revoked permission, a network mount going away mid-export -- returned Ok(()). The status label then said "STL: 11 parts -> model.stl" while the file on disk was truncated. Demonstrated before fixing, with a writer that accepts buffered writes and fails on flush: without explicit flush -> Ok(()) with explicit flush -> Err("flush failed: disk full") Three affected paths: PDF (exporters.rs), STL and GLB (workspace.rs). All now go through exporters::export_to_file, which owns the BufWriter, flushes it, and maps a failed flush to "<what>: could not write file: <cause>". Putting the flush in one place is the point -- it was forgotten three times independently. The fourth BufWriter, in arch_pdf, writes into a Vec<u8> where flush cannot fail. Left alone and excluded from the gate with that reason recorded, rather than churned for uniformity. Two tests pin the mechanism rather than one exporter: a small write stays in the buffer, so write_all succeeds and only the flush can report the failure. One asserts the drop path hides it, the other that an explicit flush surfaces it. If BufWriter's drop behaviour ever changed, the first test would fail and tell us the helper is no longer needed. CI gate added and negative-tested: no BufWriter::new in the CAD module outside the flushing helper. Also checked, and found clean: arch_svg still escapes both XML sinks after the Phase-4 projection rewrite, and all three cargo-deny exemptions are still live -- removing them makes exactly those three advisories fire, so none is a stale entry silently widening the policy. 723 lib + 154 integration, 0 failed. All twelve gates pass.	2026-08-02 07:12:23 +00:00
nigig-ci	a38c41c00a	security(sms): stop persisting message bodies, throttle sends (Phase E) ... E1 -- the inbox was written to disk in plaintext. offline_store wrote every SMS body to app_data_dir/offline_store/sms_messages.json as pretty-printed JSON. SMS is the transport for OTPs, banking codes and M-Pesa confirmations, so that file was the user's complete authentication history sitting in app-private storage -- readable by anything running as the same UID, and included in backups. This repository already knew the answer. THREAT_MODEL.md T-I2 records "Raw SMS persisted to PSV file" as fixed in Phase 0, with raw_message omitted from save_to_disk() so it "lives in memory only". The SMS app then re-introduced the same defect at larger scale: the entire inbox rather than just M-Pesa messages, and with no retention limit until D6. OfflineSmsMessage.body is now #[serde(skip)]. Dropping the field rather than encrypting it is the deliberate choice: every consumer already reads the device provider FIRST and writes the cache second (nigig-sms fetch_from_device, and the mpesa and pay transaction pages), so the provider is the system of record and no body needs to survive a restart. Encryption would keep the plaintext reachable to anything holding the key. Not writing it removes the asset. Two consequences handled: sms_key() no longer hashes the body, since a reloaded row has an empty one and dedupe would otherwise never match its own cached entry and grow a duplicate per refresh; and the cached first paint shows a neutral placeholder rather than a blank preview for the instant before the provider read lands. E9 -- the Linux backend's dependencies were pure cost. robius-sms declared polkit =0.17.0 and gio =0.17.0 for target_os = "linux". sys/linux.rs references neither: all twelve functions return Err(PermanentlyUnavailable). Those two crates dragged in glib and proc-macro-error and were the origin of RUSTSEC-2024-0370 and RUSTSEC-2024-0429 for every consumer of this crate. Deleting the block removes 340 lines from Cargo.lock. polkit, gio, glib and proc-macro-error no longer appear in the workspace at all, which also closes the LGPL-2.1 linkage question outright rather than routing around it as Phase B did for nigig-build alone. E4 -- ROBIUS_SMS_BOOT_LIB was a code-injection vector. build.rs interpolated that environment variable straight into a Java string literal, which is then compiled, dexed and loaded at runtime with the app's full permissions. A value containing a quote closes the literal and injects arbitrary Java that runs on the device at boot. Build-time environment is not trusted input. Now validated against [A-Za-z0-9_]+ and the build fails loudly otherwise. Tested both ways: an exec payload is rejected, a legitimate name builds. E5 -- undefined behaviour in the dex loader. new_direct_byte_buffer was handed RECEIVER_BYTECODE.as_ptr() as *mut u8 -- a &'static [u8] in .rodata cast to a mutable pointer, when the API is documented as taking writable memory and InMemoryDexClassLoader may write through it. Now copies into an owned allocation and leaks it, which is correct rather than lazy: the buffer backs a ClassLoader cached in a OnceLock for the process lifetime. E6 -- two bindings for one native method. rustRestoreSchedules was both exported #[no_mangle] and registered dynamically via register_native_methods. Which one won was unspecified. Kept the dynamic one, because the class is loaded from an in-memory dex and is not on the JVM's search path, so symbol binding is not guaranteed to find it. E8 -- no send rate limiting. Nothing capped send rate, and the bulk UI exists to blast a scraped directory. Android's practical throttle is ~30 messages per 30 minutes per app, past which sends are silently dropped -- so an unthrottled batch both overspends and fails opaquely. Adds SendRateLimiter, a pure token bucket taking an explicit clock so it is unit-testable without sleeping, wired into the bulk sender. A 200-recipient blast now stops at 30 and says why. E10 -- robius-sms carried no license field, so cargo-deny needed a [[licenses.clarify]] override asserting one. Stated in the manifest; override removed. E2 was already satisfied by D6 (retention capped at 5,000). E7/E11 are documented rather than fixed, which is the honest status: delivery confirmation needs real PendingIntents plumbed through (A8 documents that Ok != delivered), and sender validation cannot be solved client-side. Both are now rows in THREAT_MODEL.md instead of findings in a markdown report -- along with T-I2b for E1 and T-I4 for E3, which is NOT done: scheduled message bodies are still plaintext in SharedPreferences. CI: adds a gate asserting OfflineSmsMessage.body keeps #[serde(skip)]. Removing that attribute silently resumes writing plaintext and nothing else would fail. Negative-tested. deny-nigig-build.toml drops to 2 exemptions (from 5 before Phase B). Tests: robius-sms 21 -> 25. Verified: 12/12 CI jobs, clippy -D warnings clean on host and aarch64-linux-android, cargo deny "advisories ok, bans ok, licenses ok, sources ok", clippy ratchet holds at 49.	2026-08-01 04:58:46 +00:00
Arena Agent	03dea4d14f	fix(cad): remove panics from CAD production paths ... Continued auditing the code the KeyCode fix (667f565) made reachable, and followed the panic sites out from there. Command::merge's default was `panic!("merge() called on a Command that didn't override it")`. That is reachable, not defensive: any command that overrides can_merge but forgets merge hits it. UndoRedoStack::execute runs it on every frame of a drag, and the code five lines away already declines to panic on a genuinely unreachable branch, with the comment "never panic on the undo path". A CAD editor holds unsaved work -- losing the session is far worse than the mis-merged undo entry being reported. Now a debug_assert!: loud in development where it is actionable, and survivable in a shipped build. Proven by test in both configurations -- the release behaviour is pinned by a test marked `#[cfg_attr(debug_assertions, ignore)]`, since the dev build is *supposed* to fire the assertion. Negative-tested by restoring the panic. Also replaced three `unreachable!()`s: - add_part_command and split_selected_part_at both did `position(..)` then `get(idx).unwrap_or_else(|| unreachable!())`. The index round-trip is what created the Option they then had to discharge with a panic; `find(..).cloned()` and `enumerate().find(..)` express the same thing without arming one. - The DDE digit match's `_ => unreachable!()` arm is genuinely dead (the outer arm narrows to Key0..Key9), but a keyboard handler is not worth a crash to say so. It swallows the keystroke instead. CAD production code now contains no panicking macros at all, enforced by a new CI gate that scans each file up to its first #[cfg(test)] -- test code may panic freely. Negative-tested. One test-authoring note worth recording: my first version of the merge test built five MoveNode commands all with `from: 0`, which tripped MoveNode::execute's debug_assert that the scene's previous position matches `from`. The assertion was right and my test was wrong; a real drag chains each frame's `from` to the previous `to`. The corrected test now also verifies the thing that assertion protects -- five drag frames collapsing into one undo entry. 710 lib + 154 integration, 0 failed. All ten gates pass.	2026-08-01 04:56:02 +00:00
nigig-ci	69dd169ca6	perf(sms): get blocking I/O off the render thread (Phase D) ... D1 -- the SMS read blocked the render thread. fetch_from_device() called robius_sms::list_messages() directly from draw_walk. That is a blocking cross-process ContentProvider query over Binder, plus ~10 JNI calls per message row, plus a full rewrite of the offline JSON store. On a populated inbox it is a multi-hundred- millisecond stall, taken on the render thread. robius_android_env::with_activity() calls attach_current_thread_permanently() and ContentResolver.query is thread-safe, so the read is safe off the UI thread. It now runs on a worker and hands back through a results queue drained on Event::Signal -- the same shape the contact lookup in this file already used. D2 -- the app never idled. draw_walk ran a 5-second wall-clock refresh, and that refresh called redraw(), which scheduled the next frame, which re-entered draw_walk. A self-sustaining loop, running whether or not the SMS tab was even visible, each cycle paying D1's cost plus D3's clone. Refresh is now event-driven: first load, Event::Resume, pull-to-refresh, and the permission-granted callback. NOT yet a ContentObserver on content://sms -- that is the remaining half of D2 and is called out in the code. Until it lands, a message arriving while the app is open appears on the next Resume or pull rather than within 5s. That is a deliberate trade: a bounded staleness window in exchange for an app that can reach idle. D3 -- a deep clone per refresh, purely to diff. `let previous = self.conversations.clone()` copied every message in every conversation on each cycle so the result could be compared for equality. Replaced with a u64 digest over (count, address, message_count, newest date_ms). Bodies are immutable once stored, so that is sufficient to notice an insert, a delete or a new message -- and the test asserts exactly those three cases. D4 -- ~900 pointless worker kicks per second. start_contact_lookup_worker() was called once per visible row per frame (~10-15 per frame, ~900/s at 60fps). Each call took 3-4 mutex locks before early-returning, contending with the worker thread trying to write results back. Now called once, after the draw pass. The per-row code only enqueues. D6 -- the offline store grew without bound. upsert_sms_messages re-read, re-hashed, re-sorted and rewrote the whole file on every call, with no cap -- while append_location() a few lines below has always truncated to 512. Capped at MAX_CACHED_SMS (5000), newest-first so truncate drops the oldest. This is a display cache; the device provider stays the system of record. D7 -- O(rows x selected) per frame in the bulk list. get_selected_companies() returns a Vec and the draw path called `selected.contains(..)` once per visible row, so "Select All" over the Nairobi directory made every frame quadratic. Now a HashSet. Same fix in sync_selected_to_recipients, whose phone de-duplication was also O(n^2) via `phones.contains()`. Refactor note: group_into_conversations() and conversations_digest_of() were lifted out as free functions. ConversationsList holds a Makepad View and is not Default, so nothing in it could be unit tested; the logic D1/D3 depend on now can be. CI: adds a gate rejecting blocking robius_sms provider calls inside any draw_walk. Negative-tested by reinserting the call and confirming it fails. Tests: nigig-sms 19 -> 22, nigig-core +1. Verified: clippy ratchet holds at 49, clippy -D warnings clean on host and aarch64-linux-android, nigig-build still builds. NOT measured. The plan asked for before/after frame timings on a 5,000 message fixture and this sandbox has no device or emulator, so the figures above are reasoned from the code, not profiled. D5 (bulk send still blocks the UI thread) is untouched and needs the same worker treatment as D1. Pre-existing and unrelated: nigig-core's pending_tx_lifecycle test fails identically on pristine origin/main (wall-clock assumption in an M-Pesa expiry test).	2026-08-01 04:34:22 +00:00
nigig-ci	0614a70888	fix(sms): correctness pass on grouping, errors, dates and iOS (Phase C) ... C1 -- one contact appeared as several conversations. populate_display_messages() keyed the group HashMap on the RAW provider address. A Kenyan inbox routinely carries three forms of the same person -- +254712345678, 0712345678, 254712345678 -- depending on whether they were on-net, roaming, or saved in contacts. Each became a separate thread holding part of the history, and a reply went to whichever one happened to be open, so the user's own messages scattered across the duplicates. normalize_number() (last 9 digits) already existed and was already used for contact-NAME lookup; it just was not used for grouping. get_conversation() and insert_sent_message() now match the same way, so a reply joins the existing thread. Alphanumeric senders ("Safaricom", "MPESA") normalise to empty and fall back to the raw address, so they are not all merged into one bucket. C3 -- every JNI failure said "Unknown error". From<jni::errors::Error> mapped everything to Error::Unknown, discarding the payload. A failed send read identically whether the cause was a missing method, a mismatched descriptor, a Java exception or an unreachable JVM. Field diagnosis was impossible -- and this is how the setRepeating signature bug (fixed in A4) stayed invisible. Adds Error::Jni { kind: JniFailure, detail } and classifies. Also fixes sys/unsupported.rs, which returned Unknown for all twelve functions where every other stub backend returns PermanentlyUnavailable, so unsupported targets reported "Unknown error" instead of "not supported here". C4 -- dead branch in show_conversation(). Two blocks; the second ran unconditionally and re-applied the no-search behaviour, making the search branch above it dead. Opening a conversation with an active filter scrolled to the bottom of the UNFILTERED timeline instead of the top of the matches. A merge artifact, invisible unless you read the control flow rather than the surrounding "Mirror Robrix" comments. C5 -- timestamps were wrong outside East Africa. ~160 lines of hand-rolled civil-date arithmetic with a hardcoded UTC+3. The desktop branch attempted to read $TZ but stripped the alphabetic characters and parsed the remainder, so "America/New_York" became "/New_York", failed, and fell through to +3 as well: it could never have worked for any named zone. Deleted in favour of chrono, which was ALREADY a dependency of this crate and already used correctly in conversation_screen.rs. Verified green under TZ=UTC, Africa/Nairobi, America/New_York and Asia/Tokyo. C6 -- iOS thread ids were from the wrong namespace. read_modern_db selected m.handle_id -- a PARTICIPANT id -- and wrote it into SmsMessage.thread_id. But list_thread_messages() filters on chat_message_join.chat_id and read_modern_threads() reports chat.ROWID. Three namespaces, so an id handed out on read never matched the id expected on query: thread navigation on iOS was broken by construction. Now joins chat_message_join and selects the chat id. apple_date_to_ms() also assumed nanoseconds unconditionally; older chat.db revisions store whole seconds in some columns, which divided by 10^9 and rendered as 1970. Now disambiguates by magnitude. C7 -- the bulk path skipped its validation. The app never called send_bulk_sms; it wrote its own loop over send_sms so it could report per-recipient success. That also skipped validate_bulk_send_request, the only check that rejects a whitespace-only recipient inside a batch -- a stray blank line in the recipients box. Rather than give up the per-recipient reporting, the rule moves onto BulkSendRequest::validate() and the caller runs it explicitly. C2 was already fixed in Phase A (A10, persisted id counter). Tests: robius-sms 15 -> 21, nigig-sms 15 -> 19. CI: nigig-sms clippy ratchet 50 -> 49 (C5 removed the dead helpers). Verified: clippy -D warnings clean on host and aarch64-linux-android, nigig-build still builds, cargo deny still passes.	2026-07-31 22:56:08 +00:00
nigig-ci	817ba02625	build: drop the unused robius-sms dependency and its two advisories (Phase B) ... nigig-build declared robius-sms and robius-location and called neither: zero references to robius_sms or robius_location anywhere under nigig-build/src. That dead dependency pulled in robius-sms -> polkit -> gio -> glib -> glib-macros -> proc-macro-error which is the ONLY reason deny-nigig-build.toml carried RUSTSEC-2024-0370 (proc-macro-error, unmaintained) RUSTSEC-2024-0429 (glib VariantStrIter unsoundness) plus an unanswered LGPL-2.1 distribution question about linking polkit into a shipped mobile binary -- all of it for code that never ran. Removing the direct dependency alone was NOT enough, which is the part worth recording. `cargo tree -p nigig-build -i polkit` showed three paths, not one: the direct declaration, and two more through nigig-core and nigig-uikit. Both of those also declare robius-sms and also never use it. All three declarations had to go before polkit left the graph. nigig-core keeps robius-location: unlike the others it genuinely uses it, in src/location.rs. Verified, not assumed: - before: cargo tree -p nigig-build -i polkit resolved, three paths - after: polkit, gio and proc-macro-error no longer resolve at all - cargo deny check -> "advisories ok, bans ok, licenses ok, sources ok" with two fewer ignores (5 -> 3) - nigig-build, nigig-core, nigig-uikit and nigig-sms all still compile - Cargo.lock loses 5 lines Also in this commit: - A CI gate so the declarations cannot come back. Deliberately scoped to robius-sms/robius-location on these three manifests rather than a blanket `cargo machete`: five other unused dependencies exist here (chrono, futures, postcard, rand, serde_json) and a gate that is red on its first run gets switched off. Negative-tested by re-adding the dependency and confirming the gate fails. - Three pages carried the same placeholder string telling the user they were looking at a "RobrixStackNavigationView destination ... just like SMS conversation screens". That is user-visible UI copy, not a comment. Replaced with text describing the page. The identical "This follows the SMS/Home pattern" comment in the same three files now says what the code does instead of naming another module. Note the scope limit: 26 of the 29 crates in this workspace declare robius-sms and never use it. This commit fixes the three that put advisories on nigig-build. The rest are the same latent problem and should be swept separately.	2026-07-31 22:45:58 +00:00
nigig-ci	5cfeec26ff	fix(sms): recover from mutex poisoning instead of panicking (A6) ... Every access to the SMS contact-cache statics used `.lock().unwrap()` -- 25 call sites across four files, 21 of them in conversations_list.rs. `Mutex::lock()` returns Err only when a previous holder panicked while the guard was alive. `.unwrap()` on that converts one transient panic into a permanent one: from then on EVERY later access to the same mutex panics. The cache is written from a spawned worker thread (start_contact_lookup_worker) and read from draw_walk, so a single panic in the worker left contact resolution broken for the rest of the process, surfacing as a panic in the renderer with no connection to the original fault. For this data, aborting is the wrong trade. The protected values are a name cache, a lookup queue and an in-flight flag. None has an invariant that a mid-update panic could break in a way that makes the data unsafe to read; the worst case is a half-populated cache, which self-corrects on the next lookup. Adds sms_frame/lock_ext.rs with LockRecover::lock_recover(), which recovers the guard via PoisonError::into_inner(). All 25 sites now use it. Note this crate already knew the answer and applied it inconsistently: companies_list.rs used `if let Ok(mut s) = ..lock()` in two places, which is poison-safe but silently DROPS the write. lock_recover() keeps the update. Tests: three, including stays_usable_across_repeated_access_once_poisoned, which poisons a mutex from a panicking thread and then performs 100 further accesses -- the exact failure mode described above, and one the old code could not survive past the first. CI: the .lock().unwrap() step goes from a ratchet at 19 to a HARD gate, since production code is now at zero. lock_ext.rs is excluded: its tests must poison a mutex to observe the Err, which needs a real .lock().unwrap(). Verified: 15 tests pass (was 12), clippy 50/50, hard gate passes.	2026-07-31 21:33:34 +00:00
nigig-ci	00290ad682	fix(sms): stop truncate_preview panicking on multi-byte text (A3) ... truncate_preview() compared `trimmed.len()` -- BYTES -- against PREVIEW_MAX_CHARS, then sliced the string at that byte offset: if trimmed.len() <= PREVIEW_MAX_CHARS { .. } else { let mut end = PREVIEW_MAX_CHARS; // 120, bytes if let Some(pos) = trimmed[..end].rfind(..) { .. } // panics format!("{}…", &trimmed[..end]) // panics } Slicing a &str at a byte offset that is not a character boundary panics. This function runs inside draw_walk, once per visible row per frame, over message bodies that arrive from anyone who knows the device's number -- so a single such message took down the conversation list on every frame until it was deleted. Why it survived review, and why my own first repro was wrong: it does NOT fire on uniform multi-byte text. 120 is divisible by 2, 3 and 4, so a body of pure emoji or pure Arabic happens to land exactly on a boundary. I initially "confirmed" the bug with 40 emoji and got a clean pass. It needs MIXED text -- any misaligning run of ASCII before the multi-byte part -- which is the normal shape of real traffic: "a" + 40 emoji panicked "Hi " + 40 emoji panicked "Confirmed. Ksh1,000 sent to JOHN DOE ..." + 🎂 panicked "x" + 130 é panicked The fix counts characters via char_indices().nth(), and every offset it slices at now comes from char_indices() or rfind() on a &str, both of which return character-start offsets by construction. Two behaviour changes fall out of counting correctly, both fixes: - Bodies under 120 CHARACTERS are no longer truncated. Three of the four cases above are 41-78 chars; they were being cut only because 120 bytes of emoji is 30 characters. A real M-Pesa confirmation with a trailing emoji now displays in full. - A long first word no longer collapses to a bare "…": the whitespace break is only honoured when it leaves something to show. Tests: 10 new unit tests, including never_panics_for_any_prefix_alignment, which sweeps a 0-7 char ASCII prefix across 2-, 3- and 4-byte fillers so the cut offset lands at every possible position inside a character. That sweep fails against the old implementation. Lints: sms_utils.rs moves from #![warn] to #![deny] for clippy::indexing_slicing and clippy::string_slice. The two remaining slices carry a scoped #[allow] with the boundary proof written out -- clippy cannot distinguish a proven-safe offset from an arbitrary one, so the exemption is explicit and argued rather than a blanket mute. CI: the byte-offset gate stays pinned at 2 (those two proven-safe sites) with a comment recording that the real guarantee is now the deny plus the tests, not the grep. The clippy ratchet returns 52 -> 50, since the two string_slice reports Phase 0.7 surfaced are gone. Verified: 12 tests pass (was 2), clippy 50/50, gates 19/19 and 2/2, metadata --locked clean.	2026-07-31 21:29:07 +00:00
nigig-ci	486fa5f168	ci(sms): finish Phase 0.7 and close a false negative in the slice gate ... Two defects in my own Phase 0 work, both found by re-checking the plan's task list against what actually shipped. 1. Task 0.7 was never done. The previous commit's subject line claimed "Phase 0.2-0.7" but no lint attribute was ever added. sms_utils.rs now carries module-level #![warn(clippy::indexing_slicing)] #![warn(clippy::string_slice)] This module formats untrusted text -- SMS bodies straight off the wire -- and every function in it runs inside draw_walk, once per visible row per frame. warn rather than deny: the two known sites are still present and deny would make the crate fail to build. The point is that a THIRD site cannot appear silently. Raise to deny when A3 lands. 2. The byte-offset slice gate had a false negative. It required a leading `&`: &[A-Za-z_][A-Za-z0-9_]*\[\.\.[A-Za-z0-9_]+\] truncate_preview() contains two slices one line apart: 193: if let Some(pos) = trimmed[..end].rfind(..) <- MISSED 196: format!("{}…", &trimmed[..end]) <- caught Line 193 is autoref'd, panics identically, and being first is the one that actually fires. The gate reported "1" and looked audited while missing the instance that runs. Pattern no longer requires the `&` and also covers `[n..]` and `[..=n]`; baseline 1 -> 2. The clippy ratchet moves 50 -> 52: the two new diagnostics are exactly the clippy::string_slice reports for those two lines, which is the intended effect of 0.7 -- A3 is now visible in CI instead of only in a markdown file. Verified with the pinned toolchain (1.97.1): gates: lock().unwrap() 19/19 PASS, byte-slice 2/2 PASS robius-sms: clippy -D warnings PASS, test PASS nigig-sms: check PASS, test 2 passed, clippy ratchet 52/52 PASS supply-chain: metadata --locked PASS, lock unchanged, whitespace PASS repo-hygiene: markers PASS, workflow YAML PASS, 40-char SHA PASS	2026-07-31 20:50:58 +00:00
nigig-ci	76e4d0c391	ci(sms): add the SMS workflow (Phase 0.2-0.7) ... The SMS stack had no CI of any kind. crates/robius-sms (2,019 LOC) and crates/apps/nigig-sms (5,706 LOC) shipped with two tests, both of which assert derived trait impls (BulkSubTab::default() and a PartialEq) and neither of which touches SMS. That is 7,725 lines with no automated signal, for a feature whose job is to send real, billable messages. Five jobs: gates Two source scans, both for defect classes already present here and both invisible in review. robius-sms check + clippy -D warnings + test, host. android The job that matters. Everything in sys/android/ -- all ~600 lines of JNI, every function this crate actually performs on a device -- is behind #[cfg(target_os = "android")] and is not compiled by any host job. The three clippy errors fixed in the preceding commit were invisible until this job existed. nigig-sms check + test + a scoped clippy ratchet. supply-chain --locked lockfile, whitespace. Only enforceable because the preceding commit tracked Cargo.lock. The android job pins JDK 17 via setup-java. build.rs compiles two .java files and dexes them with d8, and d8 (build-tools 34) rejects class file major version > 61. A runner defaulting to JDK 21 fails inside d8 with "Unsupported class file major version 65", which reads like a toolchain bug rather than a JDK mismatch. Reproduced locally on 21, green on 17. Three steps are RATCHETS against a recorded baseline rather than hard gates, because each has pre-existing violations whose fixes belong to later phases: .lock().unwrap() 19 lines. Poisoning one mutex bricks contact resolution for the process lifetime. byte-offset slicing 1. truncate_preview() panics on any inbound SMS containing emoji or non-Latin text, per visible row per frame. nigig-sms clippy 50. All mechanical, but `clippy --fix` cannot apply them through Makepad's script_mod! proc macro, so they need hand edits. 34 of the 50 are dead-code reports for the duplicated contact subsystem in inbox/sms_screen.rs. A ratchet fails if the count RISES, and also if it FALLS without the baseline being lowered, so progress cannot be silently undone. The alternative -- a hard gate red on its first run -- gets switched off, which is the reasoning already recorded in doc-engine.yml for not gating cargo fmt. Clippy for nigig-sms is filtered by package id: a plain -D warnings would also fail on ~89 pre-existing warnings in nigig-core, nigig-uikit and matrix_client, which are out of scope here. --no-deps does not help, since those are workspace members rather than registry deps. Every job was executed locally against this tree before commit: toolchain 1.97.1 per rust-toolchain.toml, JDK 17, SDK platform 34 / build-tools 34.0.0. gates PASS, robius-sms PASS (0 tests), android PASS, nigig-sms PASS (2 tests), supply-chain PASS.	2026-07-31 20:06:15 +00:00
Arena Agent	5e714577fb	ci: require full 40-character SHAs for git dependency revs ... 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.	2026-07-31 19:53:49 +00:00
Arena Agent	a0dbd192c1	ci: add an unfiltered repo-hygiene workflow ... Every workflow in .forgejo/workflows is scoped with `paths:`. That is right for build and test jobs -- no reason to compile the CAD module because a payment file moved -- but it leaves a structural hole: a commit touching only unfiltered paths runs no checks at all. Commit 8c9ccb9 fell straight through it, pushing 30 unresolved conflict-marker lines across five files. The worst of them was .forgejo/workflows/nigig-build.yml itself: with markers in it the file is not valid YAML, so all seven CAD gates silently stopped running. A workflow that does not parse does not fail -- it does not run, which is strictly worse than a red build because nothing announces it. `git diff --check` would have caught this and was already a step. It was in the file the same commit broke, and only ran for that workflow's filtered paths. So this workflow has no path filter, no toolchain dependency and no network dependency -- it cannot be knocked out by an unrelated build break -- and it validates the CI configuration itself: 1. No conflict markers anywhere in the tree. 2. Every workflow file parses as YAML and has a top-level `jobs:`. 3. Whitespace errors. Verified against the real failure, not a synthetic one: checked out 8c9ccb9 and confirmed check 1 reports all 30 marker lines and check 2 names both unparseable workflows. Also probed for false positives -- `>>`/`<<` shift operators, `// =======` comments and a "=======" string literal do not trip it, because the patterns are anchored to column 0 and the closing marker requires the trailing space git writes. Also fixes the fifth file from that commit, which I missed while repairing the others because I only grepped .rs and .yml: ARCHITECTURE.md still had two live conflicts on main. Both were stale-vs-current versions of my own lines; kept the current text. While there, refreshed every LOC figure in the file table from the actual files -- 14 of them were stale, workspace.rs by 64 lines.	2026-07-31 19:41:10 +00:00
Arena Agent	80425bbfb8	fix: repair the makepad fork and unblock the build ... 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.	2026-07-31 19:32:08 +00:00
andodeki	8c9ccb92cc	Update makepad fork to latest dev branch (d6d1f99c) ... - Sync with upstream commit 5d4483f (latest map improvements + platform updates) - Include location API, audio echo cancellation, bridge-dz overlay - Add new libraries: geodata, map_nav, i_float, i_shape, i_tree, converse, llama vision - Preserve all fork-specific re-exports (gltf, csg, test) - All 102+ map improvements now available: 2D/3D toggle, shadows, labels, overlays, pattern fills	2026-07-31 18:47:37 +00:00
Arena Agent	36c9c0b0d1	fix(cad): both Save buttons reported success when the write failed ... Went looking for remaining defects before starting the viewport split and found silent data loss in the two Save buttons. cad_store::save_cad_script(&project.id, &source).ok(); self.view.label(..).set_text(cx, "Saved"); save_cad_state(&source, &solid).ok(); self.view.label(..).set_text(cx, "Saved with mesh"); Both discard a Result carrying a real io::Error and then report success unconditionally. The user is told their work is safe when nothing was written. This is reachable, not theoretical. `cad_store::cad_projects_dir()` only *logs* a `create_dir_all` failure and returns the path anyway, so the following `fs::write` fails with a genuine error -- which was thrown away. An unwritable data directory, a full disk or a permissions problem all render as "Saved". Save-As had a third silent failure on the same line of reasoning: if `bake_parts_solid()` returned None the save never even ran, and it still said "Saved with mesh". Fixed via save_status_message(what, result), so the label is derived from the outcome instead of assumed. Three distinct messages now: the caller's success wording, "Save failed: <cause>" with the underlying error text preserved, and a specific message for the no-mesh and no-active-project cases, which were previously indistinguishable from success. Also replaced the hand-rolled widget borrow in Save-As with with_viewport, the helper added in 5.2. Tests assert the two properties that matter: a failed save must not read as success and must carry its cause through to the user, and a successful save must use the caller's wording. Negative-tested. CI gate added and verified in both directions: greps for a discarded Result on a save/write/persist/store/export call. A dropped Result is merely untidy in most places; on a save path it is data loss. These were the only two instances -- the CAD module now has zero discarded write Results. 685 lib + 154 integration, 0 failed. All seven gates pass.	2026-07-31 18:42:27 +00:00
andodeki	00f1dfbc12	fix(pdf): fuzz every target, and close two ADR 0004 gaps ... Three defects found by auditing the ADR merge criteria against the code rather than against memory. 1. The scheduled fuzz job ran five hardcoded targets. Four had been added since and were never fuzzed: parse_revision_chain, decrypt, eval_function and parse_colorspace. eval_function is the sharpest of those - it executes PostScript taken verbatim from an untrusted file. The list now comes from `cargo fuzz list` and the job fails rather than passing vacuously if it comes back empty. `cargo fuzz list` reads the manifest, so a target file added without its [[bin]] entry would still be skipped silently. The engine job, which runs on every push, now checks the two agree. Both directions of that guard were exercised before committing. 2. ADR 0004 rule 3 promises an annotation whose appearance cannot be generated "keeps its original /AP and is reported as skipped". The keeping worked - to_dict clones the source dictionary - but nothing reported it: SaveReport only tracked skipped appearances for form fields. A caller who moved a stamp was never told its artwork still showed the old position. Adds SaveReport::annotation_appearances_skipped and appearance_is_generated, which enumerates the out-of-scope types explicitly so a new AnnotationType fails to compile until classified. 3. SetContents and SetFlags had no round-trip test. Both were implemented and unit-tested against the in-memory model, but neither was ever reparsed from written bytes - the assertion ADR 0004 calls central. New fixture annotations/stamp.pdf carries real /AP artwork for a Stamp (undrawable: must be preserved and reported) beside a Square (drawable: must not be reported), so the reporting cannot pass by reporting everything. The stamp test was mutation-checked: it fails when the reporting line is removed. ADR 0003 and 0004 merge criteria are now ticked. 0003's were genuine paperwork - every box traced to an existing named test. 0004's were not, and its ADR now records what was missing rather than implying it always worked. TEST_TARGET=pdf 447 -> 451 passing. rustfmt and clippy -D warnings clean.	2026-07-31 18:34:25 +00:00
Arena Agent	e7a201246c	ci(cad): scope the source-scanning gates to .rs files ... The "No build-time paths used at runtime" gate has been failing since the commit that introduced it, and I did not notice because I verified the gate's intent rather than running its exact shell pipeline. It greps the CAD directory for env!("CARGO_MANIFEST_DIR") and filters out lines starting with a // comment marker. ARCHITECTURE.md documents this very rule, so it necessarily names the macro -- and a Markdown list item is not a // comment, so it survived the filter. The gate failed on its own documentation. Both files were added in the same commit (b5471e3), so this was broken from the start rather than regressed later. Fixed with --include='*.rs' on that gate, and pre-emptively on the other two source-scanning gates (RFC1918 endpoints, by-value Vec3f getter writes). Neither matches a non-.rs file today, but both would break the same way the moment their rule gets documented in the same directory -- which is exactly what a reviewer is likely to do next. Verified by extracting every grep-based step from the workflow and running them in a shell with set -euo pipefail, rather than by re-reasoning about the regexes. All six pass. Negative-tested the fixed gate: a real env!("CARGO_MANIFEST_DIR") call in persistence.rs still fires it. Lesson worth recording: a gate must be executed as CI executes it. This one asserted something true about the code and still failed, which is the failure mode that erodes trust in CI fastest.	2026-07-31 18:28:33 +00:00
andodeki	e12a3bb5c4	fix(pay): reword the bulk gate message too ... 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.	2026-07-29 08:56:53 +00:00
andodeki	3e77bf52b0	fix(pay): make the demo gate reachable and its message honest ... 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.	2026-07-29 08:54:22 +00:00
andodeki	cbafa92269	fix(pay): close the fail-open biometric the coordinator migration would open ... 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
andodeki	b4ad9348d3	fix(pay): make batch progress bounded and derived, not counted ... 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
andodeki	a6e8f4c04e	fix(pay): remove the last false-success labels, and fix B6 where it is real ... 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
andodeki	750d856668	feat(pay): Phase 6 truthful payment states, and run UI tests in CI ... 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
andodeki	d0f5e74435	feat(pay): Phase 5 platform gateway boundary, and verify Phase 4 ... 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
arena-agent	7143b3e798	feat(doc): render CRDT-native tables in CrdtDocEditor ... - projection_layout builds ProjectedTableLayout geometry (fixed 160x28 cells matching the legacy bridge width, merge spans with covered-cell flags) and per-block origins so blocks after a table clear it instead of using the fixed line step - ProjectionRenderer draws the table grid through a new draw_table_border live field and consumes layout block origins for text placement - table_hit_test maps points to merge-anchor cells for future cell editing - doc-engine: split the cramped style-patch line clippy flagged as possible_missing_else; the crate is clippy-clean again - README: tick verified CRDT bridge boxes (dependency, wiring, selected replacement/formatting/table/toolbar migration, font size/color migration) and document the table milestone - CI: add doc-engine workflow (engine tests + clippy -D warnings + nigig-build consumer check/test) and trigger nigig-build CI on doc changes	2026-07-28 21:01:31 +00:00
andodeki	08d3e9a7fb	fix(map): increase MAX_ELEMENTS_PER_TILE to 250k and add CI workflow ... - Increased MAX_ELEMENTS_PER_TILE from 100,000 to 250,000 to handle Kenya MBTiles that contain 101k-140k elements per tile - Updated security limit test to use valid JSON with 260k elements - Added .forgejo/workflows/nigig-map.yml CI workflow to catch map-related regressions in tile parsing, style compilation, and tessellation Fixes runtime errors: - 'failed to triangulate local mbtile: too many elements (N > 100000)' - Tiles with 101k-140k elements now process successfully	2026-07-28 20:41:02 +00:00
Arena Agent	667f565ddd	fix(cad): match arms bound new variables instead of matching KeyCode ... Went looking for a clippy ratchet and found that clippy had never run on this crate at all: two dependencies fail deny-by-default lints, so `cargo clippy -p nigig-build` aborted before linting nigig-build. Fixing those two unblocked the crate and immediately exposed a real defect class. THE BUG. A bare identifier in a match pattern that is not a known variant is parsed by Rust as a NEW BINDING that matches everything. Nine such names were used as KeyCode patterns: KeyEnter, KeyBackspace, BracketLeft, BracketRight (cad/viewport.rs) Digit0..Digit9, Equal, LeftBracket, RightBracket, Apostrophe (doc, invoice, pm) Real names are ReturnKey, Backspace, LBracket, RBracket, Key0..Key9, Equals, Quote. Consequences, in severity order: - cad/viewport.rs: `BracketLeft => { .. }` is UNGUARDED and sits above the numeric arms, so it swallowed every remaining key. All direct-distance-entry input -- digits, '.', '-', ',' -- was unreachable. The entire DDE feature was dead. - The guarded ones fired for any key satisfying the guard: pressing Q while drawing with a non-empty buffer committed the coordinate. - doc/invoice/project_management: `Digit0 => '0'` swallowed the rest of the keymap, so every digit and symbol typed produced '0'. This compiles cleanly and no test catches it. rustc's only signal is `unreachable_pattern` plus `unused variable: \`Capitalised\`` -- both buried in the 200+ warnings nobody could see, because clippy never ran. 85 unreachable-pattern warnings before, 1 after (a benign catch-all). UNBLOCKING CLIPPY. Two deny-level errors in dependencies: - nigig-uikit user_project_pill.rs: a `for` loop returning on its first iteration (never_loop). Rewritten as `.next()`. - spreadsheet-engine formula2.rs: CellRef had an inherent to_string shadowing Display (inherent_to_string_shadow_display). The naive fix is a trap: Display::fmt was `f.write_str(&self.to_string())`, which resolved to the inherent method -- delete it and the same call resolves to ToString::to_string, which calls Display::fmt, recursing until the stack overflows. Verified with a standalone repro before fixing. The body moved into Display; Range got the Display impl it never had. Tests: keycode_variant_tests names the four correct variants, so it stops compiling if any is renamed -- the point being that the old code compiled precisely because the names were wrong. Two formula2 tests pin that `x.to_string()` and `format!("{x}")` agree, which is what the shadowing lint exists to protect. CI gate added and negative-tested: greps for a capitalised `unused variable`, which is the signature of this bug. Restoring BracketLeft makes it fire. 625 lib + 154 integration + 225 spreadsheet-engine + 44 doc-engine, 0 failed.	2026-07-28 20:24:03 +00:00
Arena Agent	80c3f578dd	ci(cad): gate nigig-build with cargo-deny (Phase 0.4) ... Blocked since Phase 0 because cargo-deny resolves from the lockfile, and unblocked once Cargo.lock was committed in 0.2. I predicted this would surface a large backlog. It did: 5 advisories, 6 licence rejections, 126 git-source errors and 20 unlicensed crates. The triage is the work here, not the config. ONE ACTUAL VULNERABILITY. RUSTSEC-2026-0187: unbounded recursion parsing nested PDF objects in lopdf 0.31, reached via printpdf 0.7. A ~21 KB crafted file aborts the process with SIGABRT, and because it is a stack overflow rather than a panic, catch_unwind cannot contain it. Not exploitable here, and I checked rather than assumed. The advisory is specifically about Document::load*, the parsing entry points. This crate never parses a PDF -- arch_pdf.rs only writes them through printpdf's drawing API, there is no lopdf import anywhere in the workspace, and there is no PDF read path of any kind. There is also no fix in range: printpdf 0.7 pins lopdf 0.31 and `cargo update -p lopdf` moves nothing. Clearing it means printpdf 0.8+, a breaking change across arch_pdf.rs. So it is ignored with the reasoning written down AND with the condition that invalidates it: the moment anything in this crate reads a PDF -- an import feature, a thumbnailer, a preview pane -- this becomes a live DoS and the ignore must go. An ignore without its expiry condition is how real vulnerabilities get inherited. The other four advisories are unmaintained/unsound transitive crates (proc-macro-error, ttf-parser, atomic-polyfill, glib VariantStrIter), none with a safe upgrade, all arriving through the GUI/platform stack. Listed individually rather than disabling the unmaintained class, so a new one still fails. Six licence rejections were permissive licences simply absent from the allow list. MPL-2.0 (option-ext) is weak copyleft, so it is granted to that one crate rather than added globally -- MPL reciprocity is per-file and only bites if the crate is vendored and edited. The 126 git-source errors and 20 unlicensed crates are artifacts, not findings. Every git dependency is pinned to a full rev, which is stronger than a version range and already enforced by its own CI step; the unlicensed crates are first-party and vendored forks with no license field. Both handled WITHOUT allow-ing the class: sources are governed by the rev-pinning step, and the 20 crates are clarified by name, so a genuinely new unlicensed dependency still fails. Blanket-disabling would have hidden exactly the case worth catching. Separate file from deny.toml on purpose. Merging them would mean loosening the payment rules to fit a Makepad + Robius graph, and the payment policy is the one worth keeping tight. Verified both pay crates still pass unchanged. Negative-tested in both directions: removing the lopdf ignore fails the gate, and removing MIT from the allow list produces 79 rejections. A gate that cannot fail is worse than no gate. 622 lib + 154 integration, 0 failed.	2026-07-28 19:51:37 +00:00
Arena Agent	f20ba795c0	fix(cad): time-box script evaluation so a runaway cannot wedge the worker ... Phase 2.4, which I deferred during Phase 2 on a wrong assumption. An unterminated CAD script hung the evaluator permanently. vm.eval is a single blocking call, so `while true { s = s.translate(..) }` never returns: the rebuild worker blocks forever, every later edit queues behind it, and the UI sits on "Computing 3D model..." until the app is killed. The source is user- and AI-authored, and an LLM will emit a loop like that without much provocation. Reproduced before fixing. A probe test was still running at 60 seconds, and again at 90 with the budget removed. I deferred this originally saying it needed "a cooperative check inside the Makepad script VM or a watchdog that can kill and respawn the worker thread", and called it a VM design decision. That was wrong, and I should have read the VM before concluding it. makepad_script already provides ScriptRunBudget::from_durations(soft, hard, sample_interval), checked in run_core against a sampled Instant. The CAD module simply never set it. The fix is six lines in eval_cad_script_in_vm. PHASE2_STATUS.md now records the bad call rather than quietly marking the item done. Only the hard deadline is used. A soft hit sets TimeBudgetYield, which expects a host that drives the VM back to completion; there is no such driver here, so a soft deadline would present as a script that silently produced nothing at all. Soft is set equal to hard so the budget can only ever produce a real, reported error. The previous budget is saved and restored, so one evaluation cannot starve the next. 5 seconds, sampled every 4096 instructions. Generous deliberately: a real document is a few hundred CSG ops and finishes in milliseconds, so anything still running is a runaway rather than a slow model. Result: the same script now returns "script time budget exceeded" in 5.2s. Three tests. The runaway case is #[ignore]d because it burns the whole budget by design; the other two guard the ways a timeout typically breaks things -- that a normal script is unaffected across repeated evaluations (catching a budget that is not reset), and that a syntax error still reports itself rather than being misattributed to the timeout. CI gate added and negative-tested: losing the budget line reintroduces a hang that no test failure announces, only a frozen app. 617 lib + 154 integration, 0 failed.	2026-07-28 19:26:26 +00:00
Arena Agent	0405067bd5	fix(cad): properties panel and Extend tool never wrote anything (Phase 4.3) ... Collapsing the nine duplicated properties-panel blocks exposed that all nine were silent no-ops, and so was the Extend tool. CadNode::pos/rot/size return Vec3f BY VALUE. `p.pos().x = val` therefore assigns to a temporary and discards it. It compiles, warns about nothing, and produces a control that does nothing: type a position into the panel and the part does not move. Confirmed with a standalone repro before touching anything. Eleven call sites, all wrong the same way: - the nine position/size/rotation inputs in workspace.rs - viewport.rs Extend, which carefully computes new_w/new_h and throws both away, so the tool has never extended a part Fixed by reading into a local, mutating it and calling set_pos/set_rot/ set_size, which have existed in cad_scene.rs the whole time. The duplication is what hid this. Nine copies of a 25-line block, each differing in one token, is not a thing anyone reads closely. Phase 4.3 was scheduled as tidying; it turned out to be the only reason the bug was found. The blocks now call one helper, apply_field_to_selected_part(cx, value, set), which also fixes the other hazard of the copy-paste: each block had to remember four separate invalidation steps (part_geoms.remove, invalidate_node, mark_scene_dirty, script_dirty), and missing the first one fails silently -- the part keeps rendering its old shape until something else evicts it. part_geoms.remove call sites in workspace.rs: 13 -> 5. Three defences, each negative-tested by reintroducing the defect: - cad_scene::size_tests::setters_write_through_but_getters_are_copies asserts both halves: that writing through the getter does NOT reach the node, and that the set_* methods do. It documents the trap rather than just guarding against it. - workspace::properties_panel_setter_tests applies all nine closures exactly as the call sites do and asserts each reaches its own axis and leaves the other two groups alone -- so a wrong-axis copy-paste fails too. Verified: restoring one broken closure fails with "size.y: setter did not reach the node (got 1)". - A CI grep for `.pos()/.rot()/.size().[xyz] =` outside comments. Verified it fires on the restored Extend defect. 610 lib + 154 integration, 0 failed.	2026-07-28 18:29:26 +00:00
Arena Agent	0a8d5b5abb	test(cad): move the integration suite out of src/ (Phase 4.7) ... src/.../cad/tests.rs -> tests/cad_integration.rs. It was 2,840 lines of integration tests living inside the library, compiled into every `--lib` build and able to reach anything in the crate. The move is worth more than the line count suggests, because a test target compiles as a separate crate and so sees only the public API. That turned an invisible question into a compile error: six items would have needed `pub` for the file to build in its new home — cad_mesh_data_from_solid, part_mesh_buffers, CommandBorrows, CadRenderMode, DrawingState, SnapSettings, plus the private fields of the last two. Visibility was not widened. Promoting editor internals to a public contract so a file can sit in a different directory is the wrong trade, and `pub` is far harder to take back than to grant. Instead the 16 tests that reach those items moved to where the items live: - viewport.rs gains `mod viewport_helper_tests` (10 tests: the mesh producers and the plain-data helpers). - mod.rs gains `mod editor_state_tests` (6 tests: SnapSettings polar defaults, DrawingState beam sections, CadEditorActivePane). Two of the relocated tests are worthless and are now visible as such: cad_render_mode_variants and command_borrows_fields_accessible assert that a type exists and derives Debug, which the compiler already guarantees. Left in place rather than deleted in the same commit as a move; they are for the Phase 6 sweep. The rule and its rationale are now written down in ARCHITECTURE.md under "Where a test goes", alongside corrected LOC figures for the six files that changed size since the table was written. CI gained a step. The existing job ran only `--lib`, which does not build a test target, so all 154 relocated tests would have run in no pipeline. The step names cad_integration explicitly instead of testing the whole crate, because tests/cost_estimator.rs and tests/cost_estimator_ui.rs do not compile — pre-existing upstream breakage, verified against a clean checkout, documented in a comment with instructions to fold them in once fixed. Test names diffed before and after: 0 lost, 12 gained (the Phase 4.4 characterization tests). 608 lib + 154 integration = 762.	2026-07-28 18:29:26 +00:00