Commit graph

10 commits

Author SHA1 Message Date
nigig-ci
25f32f7870 test(sms): build a real test suite (Phase G)
Some checks failed
doc-engine / engine (push) Waiting to run
doc-engine / consumer (push) Waiting to run
nigig-map / test (push) Waiting to run
sms / gates (push) Waiting to run
sms / robius-sms (push) Waiting to run
sms / android (push) Waiting to run
sms / nigig-sms (push) Waiting to run
sms / supply-chain (push) Waiting to run
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
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
nigig-ci
1670ddf49c refactor(sms): delete the dead code and the duplication (Phase F)
Some checks failed
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
nigig-map / test (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
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
nigig-ci
737a3e5d5d security(sms): encrypt scheduled payloads, report real send status (E3, E7)
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
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
nigig-ci
a38c41c00a security(sms): stop persisting message bodies, throttle sends (Phase E)
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
nigig-map / test (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
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
nigig-ci
69dd169ca6 perf(sms): get blocking I/O off the render thread (Phase D)
Some checks failed
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
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)
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
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
5cfeec26ff fix(sms): recover from mutex poisoning instead of panicking (A6)
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
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)
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
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
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
Two defects in my own Phase 0 work, both found by re-checking the
plan's task list against what actually shipped.

1. Task 0.7 was never done. The previous commit's subject line claimed
   "Phase 0.2-0.7" but no lint attribute was ever added. sms_utils.rs
   now carries module-level

       #![warn(clippy::indexing_slicing)]
       #![warn(clippy::string_slice)]

   This module formats untrusted text -- SMS bodies straight off the
   wire -- and every function in it runs inside draw_walk, once per
   visible row per frame.

   warn rather than deny: the two known sites are still present and
   deny would make the crate fail to build. The point is that a THIRD
   site cannot appear silently. Raise to deny when A3 lands.

2. The byte-offset slice gate had a false negative. It required a
   leading `&`:

       &[A-Za-z_][A-Za-z0-9_]*\[\.\.[A-Za-z0-9_]+\]

   truncate_preview() contains two slices one line apart:

       193:  if let Some(pos) = trimmed[..end].rfind(..)   <- MISSED
       196:  format!("{}…", &trimmed[..end])               <- caught

   Line 193 is autoref'd, panics identically, and being first is the
   one that actually fires. The gate reported "1" and looked audited
   while missing the instance that runs. Pattern no longer requires the
   `&` and also covers `[n..]` and `[..=n]`; baseline 1 -> 2.

The clippy ratchet moves 50 -> 52: the two new diagnostics are exactly
the clippy::string_slice reports for those two lines, which is the
intended effect of 0.7 -- A3 is now visible in CI instead of only in a
markdown file.

Verified with the pinned toolchain (1.97.1):
  gates: lock().unwrap() 19/19 PASS, byte-slice 2/2 PASS
  robius-sms: clippy -D warnings PASS, test PASS
  nigig-sms: check PASS, test 2 passed, clippy ratchet 52/52 PASS
  supply-chain: metadata --locked PASS, lock unchanged, whitespace PASS
  repo-hygiene: markers PASS, workflow YAML PASS, 40-char SHA PASS
2026-07-31 20:50:58 +00:00
nigig-ci
76e4d0c391 ci(sms): add the SMS workflow (Phase 0.2-0.7)
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
nigig-map / test (push) Has been cancelled
The SMS stack had no CI of any kind. crates/robius-sms (2,019 LOC)
and crates/apps/nigig-sms (5,706 LOC) shipped with two tests, both of
which assert derived trait impls (BulkSubTab::default() and a
PartialEq) and neither of which touches SMS. That is 7,725 lines with
no automated signal, for a feature whose job is to send real, billable
messages.

Five jobs:

  gates         Two source scans, both for defect classes already
                present here and both invisible in review.
  robius-sms    check + clippy -D warnings + test, host.
  android       The job that matters. Everything in sys/android/ --
                all ~600 lines of JNI, every function this crate
                actually performs on a device -- is behind
                #[cfg(target_os = "android")] and is not compiled by
                any host job. The three clippy errors fixed in the
                preceding commit were invisible until this job
                existed.
  nigig-sms     check + test + a scoped clippy ratchet.
  supply-chain  --locked lockfile, whitespace. Only enforceable
                because the preceding commit tracked Cargo.lock.

The android job pins JDK 17 via setup-java. build.rs compiles two
.java files and dexes them with d8, and d8 (build-tools 34) rejects
class file major version > 61. A runner defaulting to JDK 21 fails
inside d8 with "Unsupported class file major version 65", which reads
like a toolchain bug rather than a JDK mismatch. Reproduced locally on
21, green on 17.

Three steps are RATCHETS against a recorded baseline rather than hard
gates, because each has pre-existing violations whose fixes belong to
later phases:

  .lock().unwrap()        19 lines. Poisoning one mutex bricks contact
                          resolution for the process lifetime.
  byte-offset slicing      1. truncate_preview() panics on any inbound
                          SMS containing emoji or non-Latin text, per
                          visible row per frame.
  nigig-sms clippy        50. All mechanical, but `clippy --fix`
                          cannot apply them through Makepad's
                          script_mod! proc macro, so they need hand
                          edits. 34 of the 50 are dead-code reports
                          for the duplicated contact subsystem in
                          inbox/sms_screen.rs.

A ratchet fails if the count RISES, and also if it FALLS without the
baseline being lowered, so progress cannot be silently undone. The
alternative -- a hard gate red on its first run -- gets switched off,
which is the reasoning already recorded in doc-engine.yml for not
gating cargo fmt.

Clippy for nigig-sms is filtered by package id: a plain -D warnings
would also fail on ~89 pre-existing warnings in nigig-core,
nigig-uikit and matrix_client, which are out of scope here.
--no-deps does not help, since those are workspace members rather
than registry deps.

Every job was executed locally against this tree before commit:
toolchain 1.97.1 per rust-toolchain.toml, JDK 17, SDK platform 34 /
build-tools 34.0.0. gates PASS, robius-sms PASS (0 tests),
android PASS, nigig-sms PASS (2 tests), supply-chain PASS.
2026-07-31 20:06:15 +00:00