80 commits

Author	SHA1	Message	Date
andodeki	a2b05c56c9	feat(makepad-table): opt-in capabilities feature, and raise the matrix_client defect ... The two caveats from the dependency investigation. ## The capabilities feature Camera and location attachments are now available behind `features = ["capabilities"]`, which pulls `nigig-uikit` and supplies `UikitAttachmentProvider`. Measured: 89 crates by default, 275 with the feature on. That cost is real and it is inherent, not packaging waste. `camera_widget` imports `send_geocode_request` and `request_map_tile` from `nigig-core`, both of which call `spawn_async` — the shared Tokio runtime — and the first makes an HTTPS call to Nominatim. A camera that geocodes needs an async runtime and an HTTP client; there is no lighter honest version. It is affordable because it is opt-in, and because any app enabling it already depends on `nigig-core`, so that app's own tree grows by nothing. Everything touching `nigig-uikit` is in one module, so the boundary is a file rather than `#[cfg]` scattered through the widget. The provider holds no widgets of its own: the host owns the `CameraWidget` already in its tree and this asks it to open, because a provider that instantiated a second camera would fight the first for the device. A second request while one is outstanding is refused rather than overwriting. The table turns that refusal into `AttachmentUnavailable`, so the user is told the camera is busy instead of watching their first request vanish. File picking is deliberately declined here — `robius-file-picker` already ships unconditionally and costs nothing, and two paths for one job is one too many. Two CI gates, both verified to fail when they should: the opt-in build must keep compiling, and the default build must pull none of `tokio`, `reqwest`, `hyper`, `clap`, `csv`, `image`, `nigig-uikit` or `nigig-core`. The second checks the resolved `cargo tree` rather than the manifest, because feature unification can switch an optional dependency on from a sibling crate. Tests 99 default, 105 with the feature. Both clippy-clean. ## The matrix_client defect Raised in REVIEWS/MATRIX_CLIENT_FEATURE_GATE.md rather than fixed. It is not my crate, nothing depends on the broken combination, and a blind fix could change behaviour someone relies on. `matrix_client` declares `native = ["dep:tokio", "dep:reqwest", "dep:rusqlite"]` but its source gates on `#[cfg(not(target_arch = "wasm32"))]`. Two switches for the same modules, so on a native target with the feature off the modules compile and their dependencies do not — 19 errors, 26 ungated uses across 7 files. There is no CI job for the crate, which is why it rotted unnoticed. The note corrects an overstatement I made while arguing for the trait hook. I said fixing this would unblock wasm. It would not: `matrix_client` already builds clean for wasm32 with `--no-default-features`, and `nigig-core` has 8 wasm errors of its own (`crate::platform::spawn` missing) that have nothing to do with it. The only broken combination is native-target-with-feature-off, which nothing builds. I also said earlier that `matrix_client` was heavy — it is a 7-dependency local crate, not matrix-sdk. That was wrong and it inflated the case for the trait hook; the note records the measured numbers instead.	2026-08-18 18:03:26 +00:00
andodeki	9989043a37	ci: gate the coverage that was already measured and unenforced (Phase 0) ... Phase 0 of REVIEWS/REPO_COVERAGE_100_PLAN.md, and the reason it is Phase 0: no new tests, no new measurement, just ratchets on numbers that were already good and already decaying-capable. **spreadsheet** — `tools/test-spreadsheet-coverage.sh` has had a 96 floor for the engine and another for the UI controllers, and no CI job has ever run it. New `.forgejo/workflows/spreadsheet.yml`, two jobs: engine-coverage 98.83% of lines (floor 96) ui-controller-coverage 98.85% of lines (floor 96) Split in two because the halves cost very differently. The engine is pure Rust and finishes in about three minutes; the UI half has to build Makepad's Linux backend to link a test binary. One job would hide an engine regression behind a ten-minute build. **CAD widget layer** — `cad-widget-coverage` in nigig-build.yml, deliberately REPORT-ONLY. It sits at 13.25% of 10,637 lines with six files at exactly zero, and a floor there would read as a blessing rather than a debt. What the job buys is that the number is printed on every push instead of being rediscovered in six months. The first real input test should set a floor behind it. Also corrects the plan. It claimed the doc workspace module was ungated; it is not — nigig-build.yml has run doc-workspace-coverage since before the plan was written. I had surveyed by grepping workflow files for the word "coverage" and attributed nigig-build's coverage jobs to CAD alone. I nearly committed a duplicate workflow on the strength of it. The census table was right; the prose under it was not, and the correction is in the file. One thing checked and deliberately NOT changed: the spreadsheet script appears to skip its UI half when the native packages are absent. It does not. `makepad-native-libs.sh --check` returns 1, the script runs under `set -e`, and it aborts. What misled me was reading `$?` after piping the script into `tail` — which reports tail's status, not the script's. The same class of mistake this repository's CI comments warn about; no fix was needed and none was made. Verified by running each job's exact command line: COVERAGE_TARGET=engine ./tools/test-spreadsheet-coverage.sh rc=0 COVERAGE_TARGET=ui ./tools/test-spreadsheet-coverage.sh floors met ./tools/test-cad-widget-coverage.sh 13.25%, rc=0	2026-08-18 10:20:22 +00:00
andodeki	3928063392	ci(email): raise the domain floor; record the finance-email path ... email.yml: FLOOR 225 -> 230. The review doc records the email-to-finance sharing and notes the chat/Matrix path remains unbuilt (matrix_client has login+sync only).	2026-08-18 10:07:54 +00:00
nigig-ci	6e784fffee	fix(ci): the sample_thread gate was inverted under pipefail ... The 'Development sample data must not reach the UI' gate ran count=$(grep ... | wc -l) under set -euo pipefail. When sample_thread is correctly ABSENT from the UI, grep returns exit 1 (no matches), pipefail propagates it, and set -e kills the script -- so the gate reported FAIL in the GOOD state and would have passed in the BAD state. Add || true so a zero-match result is counted as 0 and the gate passes, as intended. Verified: all 11 gates now pass, and sample_thread is confirmed gone from the UI crate (only in email_store.rs, definition + tests).	2026-08-18 09:36:35 +00:00
nigig-ci	632479c964	fix(ci): email.yml has been invalid YAML for six commits ... `python3 -c "yaml.safe_load(open('.forgejo/workflows/email.yml'))"` fails: mapping values are not allowed here in ".forgejo/workflows/email.yml", line 455, column 35 A workflow that does not parse does not fail -- it does not RUN. So every gate in this file has been silently absent: the S2 password checks, the multi-recipient regression check, the TLS check, the coverage floors. All of them. The file has looked like protection while providing none. Cause: the "Coverage floors" step was rewritten to call tools/test-email-coverage.sh, and ten lines of the previous inline implementation were left behind underneath the new `run:` scalar. YAML reads the first `echo "$out" | grep -E '^test result:'` as a new mapping key and gives up. Broken by 3dab4a1 and still broken at 6bf138d -- six commits, every one of which believed it was adding or tightening a gate. Verified the removal is safe: tools/test-email-coverage.sh exists, is executable, and enforces the floors itself (TOTAL_FLOOR=88 plus per-file floors), so the orphaned lines were duplicating work the script already does. Nothing was lost. The repo's own repo-hygiene.yml WOULD have caught this -- it has an "Every workflow file must be valid YAML" step, added precisely because commit 8c9ccb9 once broke nigig-build.yml the same way and silently disabled the CAD gates. I ran that step by hand against this tree and it fails, correctly. It did not catch it because no runner is registered, so repo-hygiene has never executed on these commits. That is the actual lesson here and it is not about YAML: a gate that has never run is indistinguishable from a gate that does not exist. This is the third time in this crate's history that a check existed, looked right, and was doing nothing. After the fix: YAML valid, 4 jobs / 32 steps, all 12 source gates pass, cargo check --all-targets clean on both crates, 213 email domain tests pass, fmt clean.	2026-08-18 05:33:18 +00:00
andodeki	6bf138d027	ci(email): cover the trip-report modules ... The domain test filter and floor (225) now include finance_report and email_receipts, and test-email-coverage.sh instruments both new files. Domain tests 216 -> 234; coverage 90.6% over 15 files. The review doc records the new feature.	2026-08-17 12:08:19 +00:00
arena-agent	b478945c34	ci(doc): gate the doc-workspace coverage floor on every push ... Adds the doc-workspace-coverage job to the nigig-build workflow, mirroring the CAD gate: checkout, then ./tools/test-doc-workspace-coverage.sh, which installs its own instrumented toolchain into a shell-trap-cleaned temp dir and fails if the total floor (92%) or any per-file floor is not met. The script joins the workflow's push/PR path filters next to tools/test-cad-coverage.sh so edits to the harness itself re-run the gate. The doc README gains the milestone section recording the 28.55% -> 96.76% line measurement, the honest exclusions (widget layer, persistence write-path wrappers, defensive traversal guards) and the behavior pins and defect fixes the drive surfaced.	2026-08-17 10:25:27 +00:00
andodeki	b83e7122c4	feat(makepad-table): file picker, search, recents, New/Delete (Invoicer UI Phase 3) ... The last open phase. The sidebar gains a search box, a filtered document list and a recents list; the toolbar gains New Invoice/Quote/Receipt, Open, Save As and Delete. **The file picker is robius, not makepad's.** Makepad has an `open_system_openfile_dialog`, and it is implemented on macOS only — the Linux and Android backends never handle `CxOsOp::SelectFileDialog`, so the op is queued and dropped. It compiles, it runs, the dialog never appears. That is the worst kind of broken, so this uses `robius-file-picker`, the same crate `nigig-build`, `nigig-pay-ui` and `nigig-sms` already depend on at the same pinned revision, which goes through `rfd` on desktop and the platform picker on Android. CI gates against the macOS-only call returning. The picker's callback runs off the UI thread with no `Cx`, so it parks its outcome in a mutex and signals; `drain_file_picker` applies it on the next `Event::Signal`. Same shape as the SMS bulk CSV import. Model additions, in `makepad-doc-model` so they are testable without a window: `DocKind` with `blank()` constructors, `DocumentLibrary::create`, `remove`, and `selection_after_remove`. Decisions worth naming, because each has a wrong answer that looks fine: - **A new document is empty**, not seeded from the samples. A blank invoice arriving with "Acme Studio LLC" on it invites someone to export it without noticing whose name is there. `issue_date` is blank too — there is no clock in that crate and a guessed date is worse than none. - **Generated numbers cannot collide**, including with documents loaded from disk, and they reuse gaps left by deletions. The number becomes the filename: two documents called INV-1 save over each other and one is lost silently. - **Delete removes the row, not the file.** Removing an entry from a list is not consent to delete a document off disk, and there is no undo here. The status line says the file is untouched. - **Save reports "Choose where to save…", not "Saved."** The dialog being open is not the file being written. - **Search filters on every keystroke**, unlike the header fields, which commit on Return. Every prefix of a query is a valid narrower search; there is no such thing as a half-typed one. - **Searching does not move the selection.** Filtering is a view change, and switching the open document because a letter was typed loses the user's place. - **`selection_after_remove` is separate and exhaustively tested.** Deleting before the selection shifts it, deleting the selection keeps the index unless it was last, deleting after it changes nothing, and emptying the library selects nothing. Every wrong answer silently shows a different document; one of them indexes out of range. The document list is a fixed pool of 12 button slots rather than a `PortalList`, because this app opens documents one at a time. The pool is honest about its limit: anything past it renders as "+n more — narrow the search to reach them" rather than being dropped. Tests 79 -> 90. Six of them are the invoicer's first: `App` derives `Script` and cannot be built outside a live `Cx`, so the sidebar's presentation logic was extracted into four pure functions and tested there. Verified by reintroducing six defects across the two crates — silent overflow, a selection marker that shifts the indent, whitespace counting as a search, colliding numbers, a selection that ignores the shift, and a `blank()` that pre-fills. Also fixed, all pre-existing and all now blocking the `-D warnings` gate that has been running on these crates since the workflow was added: `std::io::Error::new(ErrorKind::Other, _)` in two crates, a manual `RangeInclusive::contains`, a manual `is_multiple_of`, a single-arm `match`, and a duplicated `#[test]` attribute that was annotating one function twice — which is why the count reads 36 rather than 37 here; no test was lost. The sample data keeps its `12_000_00` money literals, where the last group is the minor units and the number reads as "12,000.00" at a glance. `inconsistent_digit_grouping` is allowed at the crate root with that reasoning, rather than regrouping every amount into thousands and making each one need arithmetic to check against its comment.	2026-08-17 10:01:43 +00:00
andodeki	189377a3a3	ci(email): build the wasm path; document the closed §8 gaps ... email.yml: install the wasm32-unknown-unknown target and check the email domain's credential-bearing wasm half (call_email_api, WasmFetchTransport, set_email_api_url) so a browser-only breakage cannot reach main unseen. The domain test floor ratchets 205 -> 210. The review doc's §8 is rewritten: the TLS handshake, the wasm build, B1 and the test/clippy baselines are now executed/measured; the only entries left are the ones that genuinely cannot run in CI (a live relay's cert, a browser's fetch), stated with their exact reasons.	2026-08-17 09:39:04 +00:00
andodeki	89ca5186c6	docs(pdf): Phase 4 is not 100% — audit it, and verify the half that is ... Asked whether Phase 4 was complete, I checked the tree instead of my own commit message, and the commit message was wrong. Three items named in the Phase 4 spec are **not** implemented, and the status line said "complete" over them: - **Field-value reconciliation** (`form_reconcile_test.dart`). Setting a value writes /V, marks the field dirty and regenerates /AP — that all works. What is missing is the reconciliation case: a file opened with /V and /AP *already disagreeing*, where the right answer depends on /NeedAppearances. Nothing decides that today. - **Type1/CFF embedding.** The spec hedges with "if feasible", so this is a legitimate deferral rather than an oversight — but "complete" did not say so. `sfnt.rs` detects CFF outlines and `font.rs` reads an existing /FontFile3; nothing writes one. Creation is TrueType-only. - **`repair-cmap`.** No equivalent exists. `text_box_appearance_test.dart` *is* covered, by appearance.rs:235 — it just does not carry that filename, which is why a grep for the dart test names is a starting point and not an answer. The other half of the exit criterion — "generated PDFs open cleanly in external viewers" — had never been checked at all. The sample generator's own doc comment admits no test in this repository can assert it. So I ran it through implementations we share no code with, and **it passes**: qpdf --check no syntax or stream encoding errors pdfinfo title, author, subject, keywords, 2 pages, Form: AcroForm pdftotext all text, including the embedded DejaVu subset and its em-dash qpdf --list-attachments readme.txt, extracted by name with description catalogue /Outlines /Names /EmbeddedFiles /PageLabels /Dests /PageMode /ViewerPreferences /AcroForm `tools/check-pdf-external-readers.sh` makes that repeatable, and pdf.yml runs it. It treats a qpdf *warning* as failure, not just an error: qpdf warns where it had to reconstruct, and reconstructing is exactly what a stricter viewer will refuse to do. Negative-tested twice — removing the attachment fails 3 checks, and corrupting the startxref offset makes qpdf report "file is damaged". Two defects that audit found: - **The sample never exercised XMP**, so the Phase 4 feature most likely to be silently missing was also the one nothing looked at. Probed separately: `set_xmp_metadata` works, pdfinfo reports `Metadata Stream: yes`. - **A `Banner` naming an unregistered font produces a structurally valid PDF that renders no text.** qpdf --check passes; poppler says `Unknown font tag 'F1'` and draws nothing. `stamp.rs` cannot register the font itself — fonts belong to the document, and a banner does not know which document it will be drawn into — so this is now documented on `Banner` with a worked example, and pinned by `a_banner_font_must_be_registered_or_the_page_lacks_the_resource`, which asserts on the page's /Font resources because that is the thing actually missing and the thing a caller can check. The plan now records that it was wrong once, rather than quietly correcting itself. A status line that has been overstated should show its working. Engine suite 952 -> 953. Phase 4's engine half is verified end to end against third-party readers; the ui.rs interaction half is written and still blocked on the Makepad headless backend.	2026-08-17 09:11:03 +00:00
andodeki	fc0b1f287f	ci(email): run the conversation-kit tests; mark Phase E complete ... email.yml: the domain test floor ratchets 190 -> 205, and the nigig-email job runs cargo test -p nigig-uikit --lib -- conversation so an email-driven regression in the shared kit cannot silently surface in SMS. The review doc marks E1-E6 done and records the honest correction E5 surfaced: lettre's timeout bounds only the TCP connect, not the greeting/command reads — the send path now bounds the whole operation.	2026-08-17 05:09:15 +00:00
arena-agent	228bc2c81f	ci(doc-engine): gate the engine coverage, and note it in the doc README ... New coverage job runs tools/test-doc-engine-coverage.sh on changes to crates/apps/doc/**, the script itself, or the workflow. A coverage number nobody gates goes down; the floors (total plus per-file) are the enforcement. The doc workspace README records the milestone and the two CRDT-tolerance behaviors the new tests pin.	2026-08-17 04:33:08 +00:00
andodeki	2a74c6cac4	ci(email): gate the keystore feature, cover email_bulk ... email.yml: the feature-compile check now covers imap,keystore together. test-email-coverage.sh instruments email_bulk.rs (91.9% line) alongside the rest of the domain; total 89.84%, floors enforced. The review doc records C6/C7/C1f as fully closed, with the honest caveats unchanged (network sockets and the OS vault are compile-checked, not runtime-verified).	2026-08-17 04:29:30 +00:00
andodeki	ab17c72c55	feat(makepad-table): drag-reorder columns, and the first tests this crate has ... Phase 4 of the README's table, plus the test infrastructure Phases 1-3 never had. The crate had zero tests before this; it now has 21. Drag-reorder: - A press on a column header no longer commits to an action. It arms a drag and resolves on release: travel more than 8px and it reorders, release without travelling and it opens the column menu as before. Without that ambiguity resolved, every menu open would jitter into a one-pixel drag. The threshold matches `TouchTracker::MOVE_THRESHOLD` so a mouse and a finger agree on what a drag is. - While dragging, the carried column is tinted full-height and a 2px bar marks the boundary it would land on. The bar is suppressed when the drop is a no-op, so no bar means nothing will happen rather than a bar sitting misleadingly at the source edge. - `TableAction::ColumnMoved { from, to }` fires only when the index actually changed, so a host persisting column order is not asked to write on every wobble. An open cell editor is cancelled, because it addresses a cell by index and the indices just moved underneath it. `draw_drag: DrawVector` — declared, never used anywhere — is replaced by two `DrawColor` layers. `DrawVector` is a full tessellator with path, vertex, index and paint state; a translucent rectangle and a vertical bar do not need any of it. Testability, which needed a structural change rather than a test file: `Table` derives `Script` and `Widget`, so it has no `Default` and cannot be constructed without a live `Cx`. Nothing about it was unit-testable. The logic worth testing does not need a widget, so it moved off it — `ColumnGeometry` owns boundary and drop-position arithmetic, and a free `reorder_columns` owns the move. `Table` forwards to both, and `compute_layout` now goes through `ColumnGeometry` too, so there is one implementation rather than two that can drift. The 21 tests cover column geometry at even and uneven widths and at a non-zero origin, drop-position resolution including the exact-midpoint case and clamping outside the table, the index shift in both directions, no-op drops, out-of-range refusal, cells travelling with their header, ragged rows, a permutation property over repeated drags, and the Phase 3 menu's geometry and hit-testing. Verified by reintroducing three defects separately: removing the shift for the removed source column fails 7 tests, dropping the no-op guard fails 1, and moving headers without their cells fails 3. Phase 3 was marked "scaffolds only" in the README and was in fact substantially complete — menu state, open, hit-test, apply, and drawing all present, with 15 row and column actions wired. Corrected to done, with its geometry now under test. Also adds `.forgejo/workflows/makepad-table.yml`, the first CI this tree has had. Every step passes `--manifest-path` explicitly: the crate is excluded from the root workspace, so `-p` from the repo root cannot reach it and `--workspace` skips it — omitting the flag does not fail loudly, it silently tests nothing. The workflow gates tests, clippy at `-D warnings` and fmt, and asserts three invariants that would otherwise regress quietly: that the exclusion still holds from both sides, that no manifest tracks a git branch instead of pinning a revision, and that monetary fields stay integer. Each gate was checked by breaking what it protects. The exclusion check caught a defect in itself while being tested: a bare grep for the path also matched the explanatory comment above the exclude list, so deleting the entry and keeping the comment passed. It now anchors on the quoted entry. Two pre-existing clippy warnings fixed so the new `-D warnings` gate starts from zero.	2026-08-17 04:22:22 +00:00
andodeki	1c91d6b398	ci(cad): gate the engine coverage, with per-file floors ... The harness measured; nothing enforced. A coverage number nobody gates goes down. tools/test-cad-coverage.sh now exports llvm-cov JSON and fails when the total drops below 85% or any of the fourteen engine files drops below its own floor. The per-file floors are the point: deleting every test in persistence.rs moves the total by under two points, so a single number would wave that through. Each floor sits a couple of points under today's measurement, so refactoring does not trip it and a real loss does. The low floors are the honest ones. arch_pdf (72) and arch_gltf (72) have gaps in byte-layout paths that only a real PDF or GLB consumer reaches; arch_svg (79) and cad_scene (78) have gaps in widget-facing helpers and defensive arms on invariants SceneBuilder already enforces; exporters (88) cannot reach the save-dialog branch without a windowing system. Raising those needs work, not a bigger number here. Also in this commit, from running the script the way CI will rather than with a warm local checkout: - the Makepad fetch is sparse + blobless + depth 1 over the actual path-dependency closure (math, csg and its six siblings, micro_serde, its derive, micro_proc_macro, live_id, id_macros). 29 MB and two seconds instead of a 319 MB checkout of a repository that is mostly shaders, fonts and demos. Two of those crates were found by the run failing at manifest-read time, which is why the script now verifies all thirteen manifests exist before building instead of trusting the sparse pattern. The new cad-engine-coverage job needs no native packages and no GPU -- makepad-math and makepad-csg are dependency-free Rust, which is the whole reason the engine can be measured at all. It installs its own toolchain into a temp dir and deletes everything through a shell trap: nothing cached between runs, nothing left in the workspace. Verified end to end with a cold run: fresh toolchain, fresh sparse fetch, 466 tests green, total 88.75%, all floors met, environment cleaned.	2026-08-16 22:26:42 +00:00
andodeki	b87d8b0762	test(email): coverage over the full domain; IMAP feature gate in CI ... tools/test-email-coverage.sh now instruments all twelve email files (the new pacing, credential-store, cache, session and imap modules) and enforces per-file floors; measured 90.7% line coverage over the domain. email.yml: the domain test filter gains imap_client::/credential_store::, the test floor ratchets 150 -> 190, the sample-data gate is now a hard zero (sample_thread is test-only), and a new step checks the feature-gated IMAP transport still compiles. The review doc marks Phase C and Phase D complete with the honest caveats (sockets/keystore/pool-reuse are not host-verified).	2026-08-16 22:22:55 +00:00
andodeki	3dab4a1fd5	test(email): coverage floors for the email domain ... tools/test-email-coverage.sh instruments the nigig-core email domain and enforces a whole-domain floor (90%) plus per-file floors on the files that harboured the bugs. It runs in an isolated temp dir and reports over only the seven email source files, excluding Makepad's generated code. Wired into email.yml, which also now runs mail_proxy tests and ratchets the domain test floor to 150. Measured 93.4% line coverage across the domain.	2026-08-16 21:49:12 +00:00
nigig-ci	c0b27d0586	feat(email): MailBackend trait and BackendKind — both backends (C1a/C1b) ... You chose to support IMAP-on-device AND a server-side proxy, user selectable. This is the seam that makes that contained rather than two parallel apps. Why it is cheaper than it sounds: wasm cannot open a raw TCP socket, so a proxy always had to exist for the browser target. The second backend was never optional -- it was implied scope nobody had named. C1a, mail_backend.rs: BackendKind { ImapSmtp, ProxyApi } with three predicates that exist so the UI cannot get them wrong: is_available_on_wasm() IMAP is raw TCP; a browser cannot open one, so the chooser must not offer a dead option stores_reusable_password() IMAP keeps a REUSABLE mailbox password on the device. For most people that is the password-reset channel for every other account they own. A revocable proxy token is strictly safer, and the chooser must say so rather than presenting a free choice summary() the honest one-liner, asserted by test to actually mention "password" / "revoke" BackendSettings is the PERSISTABLE half and carries no secret, exactly as EmailAccount does for the password (S2). BackendDraft::validate returns (settings, Secret) and reports every problem in one pass. The trait is deliberately synchronous and tiny -- kind(), is_configured(), describe(). Anything computable above the line (grouping, previews, threading) is NOT a backend concern, which is why email_store did not change at all. I/O stays in the free functions that already own the async context, so this file is host-testable with no runtime. ImapSmtpBackend exists with validation but no protocol client yet; that is C1e and nothing here claims a connection works. C1b: EmailAccount gained `backend: BackendSettings`, #[serde(default)] so existing persisted accounts still load. A test asserts the serialised account -- including the backend section -- contains neither the token nor a field named password/token. Provider defaults now fill IMAP too, so a Gmail user still fills one field. Outlook is special-cased: its IMAP host is outlook.office365.com, not imap.outlook.com, so the naive smtp->imap rewrite would produce a name that does not resolve. New gate, negative-tested both ways: stores_reusable_password() and is_available_on_wasm() must exist, and the persisted settings structs must not declare password/token/secret fields. Domain tests 99 -> 126. Test floor 95 -> 120.	2026-08-16 20:30:33 +00:00
nigig-ci	901cddc716	fix(email): abandon_send shipped as dead code; wire it and gate it (B6) ... Auditing Phase B against the tree rather than against my own notes found that abandon_send() existed in nigig-core and NOTHING called it. The user had no way to stop waiting on a hung send. I had marked B6 "partial" for the right reason -- lettre cannot cancel mid-transaction -- and missed that the part I did implement was unreachable. A control the user cannot reach is not a control. It is dead code wearing a safety label, which is worse than an acknowledged gap because it reads as done. Now wired: while a send is in flight the Send button becomes "Stop waiting". The label is deliberately not "Cancel" -- this does not stop delivery, because once DATA is accepted the message is sent whether we wait for the reply or not. It frees the UI and suppresses a result the user has stopped caring about. The 20s timeout from A6 bounds the window. New gate: abandon_send() must exist in nigig-core AND be called from the UI. The wiring is the thing checked, not the function. That gate was ALSO broken when first written -- it grepped for `abandon_send()` across src/, and the comment block explaining why the control exists mentions it by name, so unwiring the call left the gate green. Same flaw as the B5 gate in the previous commit, found the same way: delete the fix, watch the gate. Now excludes comment lines. Twice in two commits I have written a gate that its own explanatory text satisfied. Worth stating rather than quietly fixing: a gate is only evidence if you have watched it fail. Phase B verified closed: B1-B6 all done, 11 gates pass, 99 domain tests, check --all-targets clean on both crates, fmt clean.	2026-08-16 20:04:41 +00:00
nigig-ci	d889cbecd4	ci(email): gate multi-recipient send, and a gate that did not work ... Two new gates, and one of them was broken when I first wrote it. B1 gate: the send path must call email_send::parse_recipients, must NOT contain a single-Mailbox parse of the whole To field, and must add every accepted recipient. Three checks rather than one, because each failure mode is separately reachable. B5 gate: spawn_send_email must keep the SEND_IN_FLIGHT swap. THE B5 GATE DID NOT WORK AS FIRST WRITTEN. It grepped the whole file for `SEND_IN_FLIGHT.swap(true`, and the unit TESTS for the guard contain that same string -- so deleting the guard from production code left the gate green. I found it by negative-testing, which is the only reason I know. Now scoped to the text before `#[cfg(test)]`. That is worth recording rather than quietly fixing: a gate whose own test fixtures satisfy it is indistinguishable from a gate that works, and the only way to tell them apart is to break the thing on purpose. Negative tests, all confirmed firing: remove the list parse -> fires reintroduce `let to_mbox: Mailbox = ..` -> fires delete the in-flight guard -> fires (after the fix) and all 10 gates pass on the clean tree. Test floor 60 -> 95 (actual 99). Bulk page: builds through EmailSendRequest, so a partly-invalid list reports what was dropped instead of refusing everything, and requires a second tap before sending. The prompt quotes the recipient count and any duplicates or rejections, so the user knows what they are confirming. Editing the message after arming re-prompts rather than sending the old confirmation.	2026-08-16 19:51:21 +00:00
nigig-ci	b3d562f4e2	ci(email): gate the Phase A security properties, and surface the warning ... Three new gates in email.yml, each negative-tested by reverting the fix and confirming the gate fires: 1. SmtpConfig.password must be a Secret, AND SmtpConfig must not derive Serialize/Deserialize. Two separate checks, because either alone re-opens S2: a Secret that gets serialised is still exposed, and a String that never gets serialised still Debug-prints. 2. set_email_api_url must call email_api_url_is_safe. Checks both that the validator exists and that the setter uses it -- a validator nobody calls is decoration. 3. tls_mode_for_port must exist, and Tls::None / Tls::Opportunistic must not appear. Opportunistic is the dangerous one: it silently accepts a downgrade, which is exactly the attack starttls_relay's Tls::Required prevents. Negative tests, all confirmed firing: password: Secret -> String gate fires re-add #[derive(.., Serialize)] gate fires remove the validator call gate fires introduce Tls::None gate fires and all 8 gates pass on the clean tree. Domain test floor raised 38 -> 60 (actual: 68) and the filter widened to include the secret:: module, so the new tests are actually covered by the floor rather than sitting outside it. Also surfaces config_warning() in the setup flow, so a from/username mismatch is shown while the user can still fix it, rather than becoming a silent provider rejection later. One YAML trap worth recording: the test filter ends in `secret::`, and a bare trailing colon makes YAML parse the line as a mapping. The run string has to be quoted. Caught by validating the workflow before committing, which is the only reason this is not a broken pipeline.	2026-08-16 19:22:40 +00:00
andodeki	cf73ef4c1d	test(pdf): assert what a file declares is delivered, and floor the coverage ... Every serious bug in this stack has had one shape: a valid, well-typed, empty-or-default value where the file plainly declared content. xobjects empty for every document; acroform() dropping every field behind an indirect reference; DCTDecode returning its own compressed bytes; a JPEG decoder that was a stub returning black. None errored, none panicked, and the tests asserted Ok, which they got. Coverage would not have caught any of them. Measured when each shipped: page.rs 92.4%, form.rs 93.6%, content.rs 89.2%, xref.rs 95.2%. The buggy lines ran; nobody checked what they produced. So: a property test that walks the raw object graph of every corpus fixture, counts what the file declares, and requires the API to deliver it - fonts, xobjects, graphics states, colour spaces, form fields, filters, MediaBox. It reimplements the resolution rule independently of page.rs on purpose; a test that asks the code under test what to expect agrees with the bug. It failed the day it was written, on a shape the corpus had never contained. Every fixture wrote /Resources inline, and all six extractors read it with dict.get_dict("Resources") - which returns None for an indirect reference and never consulted /Parent. A page with "/Resources 5 0 R", the commonest shape in real PDFs, reported no fonts, no xobjects, no graphics states and no colour spaces. Same for a page inheriting resources from its /Pages node. Empty, not wrong, so nothing failed. Fixed by resolving /Resources once in PdfPage::from_obj through a helper implementing the full inheritance rule (32000-1 Table 30), and passing the resolved dictionary down. Indirect /MediaBox entries resolve too. Six resources/ fixtures cover the shapes that were missing. Mutation-checked: reverting inheritance kills 5 tests, the sub-dict reference 3, indirect MediaBox 2, and removing the depth bound hangs. One mutation survived - a visited-set guarding a /Parent cycle, which the depth bound already handles - so it was deleted rather than left as untested defence with a reassuring comment. tools/test-pdf-coverage.sh enforces a floor instead of printing a number, with per-file floors as well as a total: image.rs could fall from 33% to 5% and move the total by under a point. All three failure modes verified to fail. It caught a bug in itself first - its ignore regex matched its own work directory and reported a confident TOTAL 0.00%. .gitattributes marks *.pdf binary. An xref entry must be exactly 20 bytes (7.5.4), so with a one-digit generation field it ends in a space, and git diff --check was reporting unfixable "trailing whitespace" on every fixture in the corpus. TEST_TARGET=pdf: 695 passed, 0 failed (was 680). Coverage 83.42%. ADR 0017 records the four mutations so they can be repeated by hand.	2026-08-16 19:04:57 +00:00
nigig-ci	7751e96c54	ci(email): give nigig-email a CI workflow, and fix two bugs it caught ... (Phase 0.3, 0.6) nigig-email had no CI of any kind. That is how a binary with unbalanced braces reached main and stayed there -- `cargo check -p nigig-email` failed while `--lib` passed, so the library was fine and the BINARY had never compiled once. It is also how four unused dependencies survived. Four jobs: gates 4 source scans, no toolchain, fail fast email-domain the 38 pure tests in nigig-core + a floor nigig-email check --all-targets, test, fmt, clippy ratchet supply-chain unused deps, lockfile, whitespace `--all-targets` is deliberate in the check step: `--lib` alone passed for the entire time main.rs was syntactically invalid, which is precisely the failure this job exists to prevent. Phase 0.6: fmt is a HARD gate here, not report-only. The crate already formats clean so there is no pre-existing drift to grandfather in -- unlike sms.yml and nigig-map.yml, which inherited hundreds of diffs and had to settle for reporting. WRITING THE GATES FOUND TWO REAL BUGS, both in bulk.rs: B3 -- `port_t.parse().unwrap_or(587)` was still live. A typo'd port like "465x" silently became 587, and because the port selects the transport (465 implicit TLS vs 587 STARTTLS) that silently changed the security posture with no message. Now routed through AccountDraft::validate, which is unit tested in nigig-core and returns AccountError::PortInvalid. B2 -- the handler read five TextInputs and built an SmtpConfig on EVERY action event: ten heap allocations per keystroke, per scroll, per timer tick from any widget in the app, for a struct only read on click. It also captured whatever the fields happened to hold when an unrelated action fired. Now read on click. I also got a baseline wrong and corrected it. I set the clippy ratchet to 2, having seen two `unexpected_cfgs` warnings for native_activity from the app_main! macro. Measuring with the same dedupe the script uses gives 0 -- those two attribute to the bin target and are filtered by the package_id check. A baseline above the real count is not a harmless margin: the script fails when n < BASELINE precisely so slack cannot hide a regression. Every gate negative-tested: password field on EmailAccount -> fails unwrap_or(587) in non-comment code -> fails a new clippy warning -> fails (0 -> 2) test floor raised above actual -> fails (38 < 99) and all pass on the clean tree. Two of my own regexes were too strict on the first run and are fixed here: the port gate matched the comments that document the old behaviour, and the sample-data gate counted the `use` import as a call site. A gate that trips on its own rationale is a gate nobody keeps. Verified: check --all-targets clean; 41 tests pass; fmt clean; clippy 0 at baseline 0.	2026-08-16 18:35:39 +00:00
nigig-ci	964fd5d4ef	build(email): drop three unused dependencies, and gate the platform one ... (Phase 0.5) nigig-email declared four dependencies its source never mentions: serde 0 references in src/ serde_json 0 robius-location 0 chrono 1 <- KEPT, see below robius-location is the same defect SMS Phase B removed from nigig-build, nigig-core and nigig-uikit: it drags polkit/gio/glib into the dependency graph, which is where RUSTSEC-2024-0370, RUSTSEC-2024-0429 and an LGPL-2.1 distribution question come from -- for code that is never called. A CI gate already exists to stop that regressing ("The removed platform deps must not come back"), but its manifest list covered only three crates and nigig-email was not one of them. Added it, so this cannot come back the way it did here. Correction to the assessment: it listed chrono as unused. That was true when written and is no longer -- inbox.rs::format_thread_time uses it for list-row timestamps. Kept, with a comment saying why, so the next person auditing this file does not delete it and break the build. Gate negative-tested: appending robius-location back to the manifest produces ERROR: crates/apps/nigig-email/Cargo.toml declares robius-location but never uses it and removing it passes again. Verified: cargo check -p nigig-email --all-targets -> 0 errors; 41 tests still pass (38 nigig-core email_*, 3 nigig-email).	2026-08-16 18:35:39 +00:00
nigig-ci	e89dea347a	ci: name the nigig-build formatting gate for what it actually checks ... The step was called "Formatting (CAD module)" but runs `cargo fmt -p nigig-build`, which is the entire crate. Of the 1,559 diffs it reported on its first real run, the three worst files were doc/widgets/doc_widget.rs, doc/tests.rs and project_management/mod.rs -- none of them CAD. Anyone debugging the red job was pointed at the wrong directory.	2026-08-04 06:47:16 +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	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	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