678 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| 7fdf436510 |
test(pay): Phase 5 lifecycle matrix as domain tests (R2.3)
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
Payment domain, storage, platform and UI / isolated-payment-tests (push) Successful in 2m36s
Payment domain, storage, platform and UI / payment-ui-tests (push) Successful in 3m17s
PDF engine / engine (push) Successful in 46s
PDF engine / makepad-integration (push) Successful in 3m23s
PDF engine / fuzz (push) Has been skipped
The exit criterion names five scenarios: permission denial, cancellation, backgrounding, app restart, out-of-order callbacks. Four of the five are state questions, not hardware questions. A device adds confidence that Android really emits a given callback sequence; it cannot tell you how the domain reacts, because the state machine decides that. So the matrix runs against the real coordinator on every commit instead of when a phone is free, and a regression names the invariant it broke. 13 tests in crates/nigig-pay-domain/tests/lifecycle_matrix.rs, including the cases that only exist as races: a success arriving after a cancellation; backgrounding before a grant (must refuse) versus after one (must be preserved — the user did authorise); restart before dispatch versus after; a foreign grant; a replayed grant. Plus a clean-path test so the matrix cannot pass by refusing everything. ## A coverage hole the matrix found a_restart_after_dispatch_cannot_redispatch passed with the duplicate-dispatch budget removed. The state machine refuses Submitted -> Dispatching first, so the budget was never reached. That is good defence in depth and bad coverage — nothing proved the budget still worked. the_dispatch_budget_survives_a_state_machine_walk_back forces the intent back to Dispatching, exactly as a faulty recovery path would, leaving the budget as the only guard. It fails when the budget is removed. The forcing hook is behind a `test-hooks` feature, not #[cfg(test)]: an integration test is a separate crate and does not see cfg(test), so the method was simply missing. The isolated runner enables it explicitly, otherwise that test is silently filtered out and proves nothing. ## Verified by injection authorization gate removed -> 7 of 13 fail dispatch budget removed -> 1 fails (the new one) ## What still needs hardware That Android actually produces these sequences: permission dialogs, process-death timing, callback ordering under memory pressure. This file asserts the response is correct for each sequence; a device confirms the sequences are the real ones. Different claims, both needed. Tracked as R2.3b. ## Validation domain 148 unit + 13 matrix, fmt, clippy -D warnings, bench pass storage 46 / platform 64 / mpesa 29 / pay-ui 78 pass clippy -p nigig-pay-ui --no-deps -D warnings 0 errors |
|||
| 703bba3652 | perf(spreadsheet-ui): distinguish dirty cells and full invalidation | |||
| d567978119 |
feat(pay): key rotation for the encrypted ledger (R2.2)
Examined R2.2 the way R2.1 turned out to need, rather than assuming the whole item was device-blocked. Most of 3.1 was already present: DatabaseKeyProvider, open_encrypted, wrong-key rejection distinct from corruption, keystore-unavailable failing closed, and a test asserting no PII appears in the raw file. ## The real gap was rotation, and it is pure logic A StaticTestKeyProvider key lives forever. An Android Keystore key does not: KeyPermanentlyInvalidatedException is thrown after fingerprint re-enrolment, adding or removing a screen lock, or a device restore. That is ordinary, not exceptional. With no rotation path the only responses were "lose the ledger" or "keep using a key that no longer exists" — and the second is not available, because the key is gone. Deleting the ledger is not an option either. It destroys the record of money that may have left the account, which is the same reasoning that makes retention.rs refuse to sweep unreconciled rows. rotate_key uses PRAGMA rekey, which re-encrypts every page inside SQLCipher's own transaction, then proves the new key reads the data before returning — a rekey that reported success but left the file unreadable would otherwise only surface on the next launch, by which time the old key may be gone. 5 tests: records survive rotation, the superseded key stops working, an empty key is refused without damaging the file, rotation is repeatable, and the schema version is untouched. Verified by neutering rotate_key: 3 fail. Storage tests 41 -> 46. ## Ordering, documented at the trait The caller persists the new key only after rotate_key returns Ok. The reverse order leaves a stored key that does not open the file. This order leaves, at worst, a re-keyed file whose new key was not saved — recoverable by rotating again from the old key still in the keystore. ## What remains The JNI call itself: KeyGenParameterSpec with user authentication required, the AndroidKeyStore provider, and catching KeyPermanentlyInvalidatedException. The full contract is written on DatabaseKeyProvider so it is not rediscovered from scratch. Tracked as R2.2b. Everything except the platform call is already exercised by the sqlcipher suite. ## Validation storage 46 (was 41) with --features sqlcipher pass domain 148 / platform 64 / mpesa 29 / pay-ui 78 pass clippy -p nigig-pay-ui --no-deps -D warnings 0 errors builds: pay, mpesa, core pass rotation injection: 3 tests fail when rotate_key is neutered pass |
|||
|
|
a66d7d2198 |
feat(doc): carry table grids in document-level clipboard payloads
A block-span selection — Shift+Arrow across a table, select-all, any multi-block drag — used to copy blank lines where tables sat, so copying a document lost every table's content. Table blocks now contribute their whole grid at their block position as tab/newline lines, built by the same table_grid_tsv helper the cell-range payload uses (extracted from cell_range_clipboard_text): one builder, one convention, no drift between "copy a range" and "copy across a table". Stored cell text exports verbatim — a merge's covered cells keep their hidden values — and empty tables still contribute nothing. Cutting such a span was already structurally correct through replace_block_range / CancelBlockRange, so the milestone is payload-only: the payload now matches what actually disappears — verified by a cut over [paragraph, 2x2 table, paragraph] draining the document with "lead\na\tbc\nd\te\ntail" in the payload and one undo restoring blocks AND every cell value. Also covered: select-all through the TextCopy hit, and a partial mid-paragraph span splicing the grid between its text fragments in order. The stale "tables are skipped" select-all bullet is retired. |
||
| 85f21a6f35 | perf(spreadsheet-ui): consume dirty cells during intent dispatch | |||
|
|
c6e7635c5a |
test(cad): fuzz the coordinate parser; audit all 84 indexing sites
I dismissed indexing_slicing as "mechanical churn" without checking.
That was an unchecked claim about 105 reported panic sites, which is
exactly what I have been objecting to elsewhere in this codebase. Audited
all of them.
105 clippy hits are 84 unique lines. Every one is bounded:
43 fixed-size arrays indexed by a literal or a 0..N loop whose bound
matches the array -- mat4_mul, mat4_inverse, ray_aabb_intersect,
the 8-corner projected box. Unindexable by construction.
~30 behind an explicit length check in the same function:
verts.len() >= 12 (I-beam web), >= 8 (HSS inner wall),
pts.len() == 4, chamfer_rect's `< 4` early return, polygon_area's
`n < 3`, pick_part's per-triangle bounds test.
~11 `% len()` on a non-empty slice, or selection[i] over
0..selection.len().min(3).
Converting these to .get() adds ~84 `else { continue }` arms guarding
conditions the compiler or an adjacent check already rules out, each one
a place to get the fallback subtly wrong. Not gated; the audit is
recorded in ARCHITECTURE.md 2c so the next reader gets the evidence
rather than the dismissal.
The one genuinely input-facing site is fuzzed instead.
parse_coord_input takes raw text from the coordinate box on every
keystroke and indexes parts[0..2] after a split. Two new tests: an
adversarial corpus (multi-byte leading characters, lone separators, RTL
and combining marks, 500-char inputs, inf/NaN/1e400) and every
char-boundary prefix of a valid input, because the box parses as you
type.
Building the corpus is where the actual work was. My first version --
garbage strings -- passed even with the length guards deleted, because a
first component that fails to parse makes `?` return before the second
index is evaluated. It looked like a strong test and tested nothing. The
cases that reach the guards are the ones whose FIRST component is valid:
"@5", "5<", "@5<45".
Negative test, per guard:
spherical parts.len() == 3 removed -> FAILS, index out of bounds
polar parts.len() == 2 removed -> FAILS, index out of bounds
relative parts.len() >= 2 removed -> still passes, and that is
correct: the branch is gated on contains(','), and a string
containing a comma always splits into at least two parts,
so the check is redundant. Verified rather than assumed;
left in place because it states intent locally.
763 lib + 154 integration tests pass. All 13 CI gates pass.
|
||
| 874a8481fa | perf(spreadsheet-ui): track dirty grid cells | |||
| 1d3e6ab72a |
fix(pay): correlate USSD callbacks to the payment that asked for them
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
doc-engine / consumer (push) Successful in 3m56s
Payment domain, storage, platform and UI / payment-ui-tests (push) Failing after 4s
doc-engine / engine (push) Successful in 15s
nigig-map / test (push) Failing after 2s
Payment domain, storage, platform and UI / isolated-payment-tests (push) Failing after 1m58s
sms / gates (push) Successful in 3s
sms / robius-sms (push) Successful in 22s
sms / android (push) Successful in 21s
sms / nigig-sms (push) Successful in 4m6s
sms / supply-chain (push) Successful in 4s
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
|
|||
|
|
072dcde979 |
docs(cad): record Phase 5/6 outcomes, including the two rejections
Updates the file table in ARCHITECTURE.md for the three new files and
corrects the viewport.rs/workspace.rs descriptions, which still claimed
to hold the rendering and input code that moved out. A doc that lies is
worse than no doc.
Marks the plan items honestly:
5.5 DONE viewport.rs 8,023 -> 4,796. The original six-file target was
not met and is not being pursued: the remaining 4,796 lines
have no seam comparable to the two that were taken.
5.6 DONE workspace.rs 3,991 -> 3,273.
5.7 REJECTED after counting. "Branched on in 40 methods" was 14, and
21 of the 37 branches were in one function.
5.8 REJECTED. The premise is false -- kind is not derivable from the
solid, because six PartKinds share CadSolid::Box.
Phase 6 marked partially done, with what is left stated plainly: the
indexing_slicing ratchet is 105 sites in CAD, mechanical churn rather
than defect-finding, and the panic family it was really aimed at is now
at zero and gated.
Recording the rejections in the plan matters as much as the completions:
both items would have destroyed something real, and the next reader
should find the counter-evidence rather than the instruction.
|
||
|
|
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.
|
||
|
|
4df2193859 |
docs(doc): retire stale in-cell paste and migration-status claims
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
doc-engine / engine (push) Successful in 15s
doc-engine / consumer (push) Has been cancelled
Tabular paste superseded the clipboard section's "in-cell pastes always stay a single whole-cell write, newlines included", and the doc-engine README still framed the controller migration as "next" even though the workspace navigation already runs on CrdtDocEditor (the classic editor stays registered as a fallback). Found during the phase audit. |
||
|
|
b26f4c8b91 |
refactor(cad): split handle_event into pointer and touch dispatch
Phase 5.7. handle_event was 952 lines holding two independent dispatch loops -- one over raw Event (mouse buttons, motion, wheel), one over hit-tested Hit (finger down/move/up/scroll). They share no locals, so the seam was already there. The plan item asked to "consider making 2D/3D two widgets" because the view mode was "branched on in 40 methods". I counted before acting: 14 methods, 37 branches, and 21 of those branches are inside handle_event alone. The remainder are mostly one-liners picking a pan axis or a zoom helper. Two widgets would have duplicated the entire input layer to delete a handful of matches! guards, and doubled the surface the shared CadDocument must keep coherent -- which is precisely what Phase 5.2 was spent removing. The size problem was the event handler, not the enum. Split the handler; keep the enum. Reasoning recorded in the module doc so the plan item is not re-attempted from its original framing. Verified as a move rather than a rewrite: the sequence of Event::/Hit:: match arms across viewport.rs + viewport_input.rs is identical to HEAD's -- 41 arms, same order -- and the 3D-only desktop camera fall-through appears exactly once, as before. Order between the two loops is preserved and load-bearing: pointer arms set drag state the hit arms read. viewport.rs: 5,702 -> 4,796 lines (8,023 at the start of Phase 5.5). 761 lib + 154 integration tests pass. All 12 CI gates pass. |
||
|
|
f02de5645f |
refactor(cad): split handle_actions into three per-panel handlers
Phase 5.6. `CadWorkspace::handle_actions` was a single 1,133-line
method: one flat sequence of `if button.clicked(actions)` arms covering
every control in the editor. Adding a control meant reading all of it to
find where the related ones lived.
Split along the seams the comments already marked -- the panel groupings
existed, they just were not expressed in the code:
handle_view_and_plane_actions work/inclined planes, per-axis grids,
reference and clip planes,
construction toggles, coordinate input
handle_export_and_file_actions CLI/OBJ/PDF/3D/STL/SVG export, the AI
attach-image and model controls,
open/save/save-as
handle_properties_panel_actions colour swatches, layer/material
dropdowns, per-axis position/rotation/
size inputs, DOF toggles
Order is the risk in splitting a sequential handler: several arms depend
on running after an earlier one has updated state. Verified mechanically
rather than by reading -- the full sequence of `ids!(..)` widget
references through handle_actions plus the three extracted bodies is
byte-identical to HEAD's, 147 references in the same order.
19 helpers became pub(super). That is not a widening: they were already
reachable from anywhere in the cad module, and pub(super) keeps them
exactly there. Nothing was made `pub`.
workspace.rs: 3,991 -> 3,273 lines.
761 lib + 154 integration tests pass. All 12 CI gates pass.
|
||
| 648c662a1e | perf(spreadsheet-ui): cache visible text measurements | |||
|
|
1bc5d792a8 |
refactor(cad): extract the 25 draw_* methods into viewport_render.rs
Phase 5.5, first cut. viewport.rs was 8,023 lines with a single 5,930-line `impl CadViewport`. The drawing methods are the largest coherent seam in it: 25 methods, 2,321 lines, that only read editor state and emit draw calls. Rust merges inherent impl blocks for the same type across files, so this is a pure move -- no signature changed and no caller was touched. Verified mechanically rather than by eye: the set of method names across viewport.rs + viewport_render.rs is byte-identical to the set in viewport.rs at HEAD, 187 before and 187 after, none lost, none gained. Two items needed pub(super) -- `part_model_matrix_cadnode` and `DrawCadMesh::draw`. That is not a widening: both were already reachable from anywhere in the cad module, and pub(super) keeps them there. No item was made `pub`. The module doc states the rule for what belongs in the file: a method lives here if it takes &mut Cx2d/&mut Cx3d and emits drawing commands. Anything that DECIDES what to draw -- picking, snapping, hit-testing, tool state -- stays put. Deliberately mechanical, because the previous attempts to split this file were judgement calls and did not hold. viewport.rs: 8,023 -> 5,702 lines. --- Phase 5.8 (remove kind_hint): REJECTED, with the counter-example recorded as a test rather than a comment. The plan item says to "derive kind from the solid variant". That is not possible. Six PartKinds -- Cube, Wall, Slab, Door, Window, Beam -- all build CadSolid::Box, and Cylinder/Column both build CadSolid::Cylinder. kind_hint is not duplicated state; it is the only record of which one the user asked for. Removing it would silently downgrade every Wall, Slab, Door, Window and Beam to a Cube, taking the properties panel's thickness controls and the Extrude tool's kind filter with it. several_part_kinds_share_one_solid_variant_so_kind_is_not_derivable asserts the collision directly, so the next person to read that plan item finds the evidence instead of executing it. I also built a script_tag/from_script_tag round-trip so the kind could survive a save, then deleted it before committing: nothing reads parts.cad back, so it would have been a write-only annotation -- the exact "add an abstraction, declare the migration done" pattern this phase exists to stop. The real gap is that the parts list has no serialisation at all, which is a feature, not a cleanup. 756 lib + 154 integration tests pass. All 12 CI gates pass. |
||
|
|
c5c1e0a3e9 |
feat(doc): paste tabular payloads across table cells
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
doc-engine / engine (push) Successful in 15s
doc-engine / consumer (push) Successful in 3m55s
A TextInput carrying tabs or newlines while the caret lives in a table now pastes the spreadsheet way — one cell per tab stop, one row per line — starting at the caret cell, or at the armed range's normalized top-left (consuming the range like any paste-over- selection). The distribution rides the grouped set_table_cells from the previous milestone so the rectangle un-pastes in one undo step, and the caret parks at the last cell the payload touched. Rows or columns past the table edge clip, CRLF strips per line like text-block pastes, empty fields clear their targets, and unchanged cells are skipped so a paste lands neither redundant LWW ops nor dead group members. Plain payloads keep the whole-cell char splice, and cells still never spawn blocks. The pre-existing in-cell paste test asserting the old "newline stays embedded in the cell" behavior is rewritten to the distribution semantics; new runtime tests cover the 2x2 distribution with caret parking and the one-undo/redo round trip, edge clipping without wrap-around, backward-spanned ranges pasting from their top-left, CRLF with empty-field clears, and the mobile menu's Paste action distributing from the pressed cell. Engine integration coverage replays a grouped multi-cell write into a peer and asserts the projections converge. |
||
|
|
5ffc515f91 |
perf(cad): build the command scene snapshot lazily -- 254us -> 0.8us
CadCommandCtx::new eagerly called scene_cache.scene_for(parts), which is O(nodes): every node cloned, every material re-registered. No production command reads that scene -- MoveNode, ResizeNode, RotateNode, YawNode, ModifyNode, CreateNode and DeleteNode all address a node by id. Only tests and the benchmark call ctx.scene(). Every command mutation bumps the PartsStore generation, so the next context construction was a guaranteed cache miss. move_selected issues one command per selected part per frame, making a drag O(commands x nodes) for work that is O(1) per command. This arrived with Phase 5.1, when the generation counter turned a previously accidental cache hit into a guaranteed miss; the 0.4us baseline predated it. The snapshot is now built on the first scene() call and memoised. CommandContext::scene() returns Arc<CadScene> rather than &CadScene so an implementation can build on demand instead of keeping one alive for the context's lifetime. This was also a latent CORRECTNESS bug, not only waste. The eager snapshot was captured BEFORE the command ran, so a command that mutated and then read scene() saw its own edit missing. Every mutating method now drops the memo; those invalidate_snapshot() calls are placed directly beneath the existing scene_cache.mark_dirty() calls so a new mutator cannot silently miss one. Two tests, both verified to fail against the old code: - a_command_that_never_reads_the_scene_does_not_build_one asserts on SceneCache::rebuild_count (a new #[cfg(test)] counter) rather than wall-clock, so it is deterministic rather than machine-dependent. Fails 20-vs-0 when construction is made eager again. - a_lazily_built_scene_reflects_edits_made_earlier_in_the_same_context reads the scene BEFORE mutating, then again after. Reading only after the mutation passes even with invalidate_snapshot() gutted -- the lazy build simply happens later -- so the first read is what gives the test teeth. I checked: the obvious version of this test was vacuous. The benchmark was also measuring the wrong thing. It called ctx.scene() each iteration to read the start position, which no production path does: move_selected and finish_part_drag both read p.pos() off the parts list. Fixing the lazy build alone moved undo 228us -> 0.5us but left execute at 231us, because the benchmark was timing its own scene read. It now mirrors the production callers. Measured: execute 254us -> 0.8us, undo 246us -> 0.5us (1000 commands over a 1000-node scene), ~300x. No other benchmark regressed. 755 lib + 154 integration tests pass. Test-name list diffed: +2, nothing dropped. All 12 CI gates pass. |
||
| f33e999991 | perf(spreadsheet-ui): improve unicode text width estimate | |||
| 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. |
|||
| 3fda46b1eb | fix(spreadsheet-ui): flush toolbar mutation intents per event | |||
|
|
11ef0fbf67 |
fix(cad): GPU buffers self-invalidate; stop re-meshing on every drag frame
Phase 5.4. Two defects with one root cause: part_geoms was keyed on the
raw node id, so staleness was invisible to the type and correctness rested
on seventeen scattered `part_geoms.remove(&id)` calls at the edit sites.
All seventeen are deleted; the map is now keyed on (id, ParamHash), the
same key MeshCache uses, and an entry whose hash no longer matches its
node is simply never read.
1. subdivide_selected drew the wrong geometry. It replaced each selected
part's solid with a fresh Csg and invalidated the MESH cache, but never
removed the part's part_geoms entry -- so the stale uploaded buffer
stayed a hit and the viewport kept drawing the un-subdivided shape.
An audit of every mutation path found this was the only edit site
missing its manual eviction, which is exactly the failure mode a manual
protocol produces.
2. ParamHash covered the transform and the material, so a pure move
invalidated the mesh. It should not: build_mesh reads neither. The
cached TriMesh is in the node's LOCAL space, and all four consumers --
the draw loop, pick_part, the STL and glTF exporters -- apply the model
matrix themselves. Dragging a part therefore re-triangulated it on
every frame to produce byte-identical triangles, at up to 886 ns per
extruded part per frame, per selected part. The hash now covers the
solid parameters and nothing else.
Two existing tests asserted the old behaviour and were INVERTED, not
deleted -- they described what the code did rather than what it needed to
do:
cache_detects_transform_edits_via_param_hash
-> a_transform_edit_reuses_the_cached_local_space_mesh
mesh_cache_self_invalidates_on_parameter_and_transform_edits
-> mesh_cache_self_invalidates_on_solid_parameter_edits
New: build_mesh_output_does_not_depend_on_the_transform guards the
assumption the key now rests on -- if anyone makes build_mesh bake the
transform in, it fails and the key must grow it back. Plus
mesh_cache_hits_on_a_pure_transform_edit, mesh_cache_hits_on_a_material_edit
and an_uploaded_buffer_is_not_reused_after_the_solid_changes. Negative
test: restoring the transform to the hash makes the first two fail;
restored and green.
Deletion is still explicit (`retain` on the live id set) because it is the
one case a content hash cannot express -- there is no node left to hash.
ParamHash is pub(crate), not pub: it is how the caches agree on staleness,
not a consumer contract.
Also recorded in BENCH_BASELINE.md, not fixed here: bench_command_execute
_overhead is 227 us/cmd against a stale recorded 0.4 us. Verified
pre-existing -- 254 us on the commit before this branch, so this work
slightly improves it. CadCommandCtx::new eagerly builds a scene snapshot
that most commands never read, and every command bumps the generation, so
it is O(commands x nodes). The fix is to make that snapshot lazy; it is a
separate change and gets its own commit.
753 lib + 154 integration tests pass. Test-name list diffed, not just the
count. All 12 CI gates pass.
|
||
| 9a14c0ccad | refactor(spreadsheet-ui): dispatch formula edits through intents | |||
|
|
25f32f7870 |
test(sms): build a real test suite (Phase G)
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
doc-engine / consumer (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
nigig-map / test (push) Failing after 1s
sms / gates (push) Successful in 3s
sms / robius-sms (push) Successful in 23s
sms / android (push) Failing after 54s
sms / nigig-sms (push) Successful in 4m12s
sms / supply-chain (push) Successful in 6s
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).
|
||
| 79950bbba6 | refactor(spreadsheet-ui): dispatch grid intents through workspace | |||
| 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 |
|||
|
|
139f6de25a |
refactor(cad): one CadDocument shared by all three viewports
The three CadViewport instances each owned a private PartsStore, kept in step by copying the whole list every frame (parts_snapshot / replace_parts_snapshot). Both are now deleted. They share one Rc<RefCell<CadDocument>>: an edit in any viewport IS the edit in all of them, so there is nothing to reconcile. Copying was not an implementation detail, it was the source of a class of bugs that were individually fixable and collectively unfixable: - Only one viewport wins a sync frame, so the loser's id allocations were absent from the winner's snapshot and the assignment moved the counter BACKWARDS. Two parts drawn in different views between syncs got the same NodeId -- which keys MeshCache, part_geoms and every command. (Bounded earlier by sharing the allocator; now moot.) - Breaking on the first dirty viewport left later ones dirty, so the loser overwrote the winner's edit on the next frame. - Each sync deep-copied a Vec<CadNode> per destination and forced a full scene rebuild in each: ~98.5 us/frame at 100 parts, sustained for the whole of a drag. sync_parts_from_any_dirty_viewport survives but no longer moves data. It clears every script_dirty flag in one pass, returns the reporting viewport's script, and asks the others to repaint. Reads go through vp.parts() -> Ref<PartsStore>, writes through vp.parts_mut() -> RefMut. Both borrow self, so "never hold a read guard across a write" is checked by the compiler rather than left to reviewer discipline. Where a read loop's body needs &mut self (a draw call), the handle is cloned first via document() + read_parts() so the guard is tied to the local Rc instead of to self -- restoring the disjoint-field borrows the plain struct member used to give for free. Deleted in the same commit as their cause: - parts_snapshot / replace_parts_snapshot (40 lines) - split_for_command + CommandBorrows, which existed only to prove to the borrow checker that parts, scene_cache and command_stack were distinct fields. The parts list is no longer a field at all; with_command_ctx replaces them. - PartsStore::as_mut_vec, the last migration escape hatch. Its two remaining callers only wanted to edit fields, so they use a new iter_mut(); structural changes go through the specific methods, which is what makes "the generation moved" mean something. share_document runs from request_rebuild as well as initialize_split_viewports: the split viewports are created lazily, and one that missed the join would quietly edit a document nobody can see. Selection, part_geoms, scene_cache and camera state stay per-viewport by design -- what is highlighted is a property of a view, not the model. Test swap, verified by diffing the full test-name list rather than the count (both are 761): command_borrows_fields_accessible is gone -- it constructed a struct literal and read its fields back, asserting something the compiler already guarantees, and would have passed against every sync bug this removes. In its place, an_edit_through_one_document_handle_is_visible_through_the_others asserts the actual property. Negative test: replacing the shared handles with three independent documents makes it fail (left: 0, right: 1); restored and green. 746 lib + 154 integration tests pass. All 12 CI gates pass. |
||
| 59dd29d1ce | feat(spreadsheet-ui): queue grid mutation intents | |||
|
|
08c3eed18d |
feat(doc): copy and clear cell ranges through the clipboard
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
An armed table cell range now copies as tab/newline text (rows top to bottom, cells left to right, the spreadsheet convention) through the same copyable_selection_text the TextCopy/TextCut hits answer with, so the long-press cell menu floats the full action set instead of a paste-only one. Cut and Backspace/Delete clear every non-empty spanned cell via the engine's new set_table_cells, which pops each sub-write's compensation into one Compensation::Group — the span un-clears in a single undo step where Backspace previously dropped the range and edited only the caret cell. Single writes keep the leaf compensation and empty write lists record nothing. Grouping cell writes surfaced a real undo asymmetry: an undo after a redo re-applied the redone cell text, because redo pushed the write's own text as its undo compensation (inverse() only swaps the verb, keeping the stale text) while undo's leaf special-case captured live cell state only in one direction. undo()/redo() now resolve cell-text inverses per compensation member against the projected text at both transition boundaries (redo_compensation/ undo_compensation), so leaves and groups round-trip through arbitrary cycles and the "cells stay out of groups" restriction is gone. |
||
|
|
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. |
||
| f3c2aa7a83 | feat(spreadsheet-ui): define grid mutation intent type | |||
| 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). |
|||
|
|
c83f8e083f |
refactor(cad): commands mutate through PartsStore, not a bare Vec
First migration step of Phase 5.2, and it closes a real hazard rather than only moving types around. `CadCommandCtx` held `&mut Vec<CadNode>`, taken out of the store by `split_for_command` via `as_mut_vec()`. Every command therefore edited the parts list *underneath* `PartsStore`, so none of their mutations bumped its generation. `SceneCache::scene_for` compares generations, so the only thing preventing a permanently stale scene was each command remembering to call `mark_dirty()` by hand -- the exact discipline-based invalidation the generation counter was introduced to replace. Every command happens to call it today. One omission in a new command would have produced a scene that never rebuilds, with no failing test. `CadCommandCtx` and `CommandBorrows` now carry `&mut PartsStore`. move_node and update_node go through `get_mut_by_raw_id`; create, delete and insert already used store methods once the type changed. The generation now moves on its own and the `mark_dirty()` calls become belt-and-braces instead of load-bearing. `CadCommandCtx::new` also builds its scene with `scene_for`, so it shares the cache rather than rebuilding unconditionally. Four tests. Three exercise the real `CadCommandCtx` against a real `PartsStore` -- the mock keeps its own `Vec` and structurally cannot observe the generation, which is why the gap survived this long. The fourth pins the hazard itself: a mutation that skips the generation still needs `mark_dirty()`, demonstrated through a `#[cfg(test)]` `as_mut_vec_without_bump` that models the old path. Negative-tested by restoring the bypass: "a command edit must move the generation, not rely on a manual mark_dirty()". Scope kept deliberately narrow: commands.rs only, ~93 sites left in viewport.rs and workspace.rs. ARCHITECTURE.md updated with what is done and what remains. 746 lib + 154 integration, 0 failed. All twelve gates pass, benchmarks still build and run. |
||
|
|
737a3e5d5d |
security(sms): encrypt scheduled payloads, report real send status (E3, E7)
Some checks failed
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
repo hygiene / hygiene (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.
|
||
| 7566374237 | refactor(spreadsheet-ui): make shared model handle mandatory | |||
| 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. |
|||
|
|
00b3eb6e98 |
feat(doc): select the whole editing context with Ctrl/Cmd+A
Keyboard select-all lands in the CRDT editor, mirroring makepad's text_input. A parked cell cursor selects its whole cell text (the in-cell character span, collapsing any armed merge range); otherwise the selection spans the first layout glyph to the last with the caret following the focus, so Copy, Cut, style toggles and Backspace-over-span all treat it like a maximal Shift+Arrow selection. Table blocks ride the range like any middle block: the clipboard payload keeps skipping their cells while a cut drains them through replace_block_range, so one undo restores the document. Touch sessions in Edit mode float the platform clipboard menu on the fresh selection (the keyboard-select-all pattern from text_input), mirrored through clipboard_menu like the long-press request; the cell-rect lookup it shares is factored into cursor_cell_rect. A desktop Ctrl+A never makes a menu request, and a document without glyphs leaves the caret put. copyable_selection_text goes crate-visible so hosts rendering their own copy affordances read the same payload the hits answer with. |
||
|
|
0d05a5e62f |
feat(cad): add the shared CadDocument (Phase 5.2 foundation)
Introduces `CadDocument` and `SharedCadDocument` (`Rc<RefCell<CadDocument>>`) -- one owner for the parts list, its generation counter and the id allocator -- with the semantics the viewports will move onto. Deliberately NOT wiring `CadViewport` onto it in this commit. That is 103 call sites (73 viewport.rs, 20 workspace.rs, 8 commands.rs), each needing its borrow scope checked individually, and bundling it with the type definition would produce a diff nobody can review and a bisect target nobody can isolate. This lands the destination, proven by test; the migration follows one file per commit. Four tests, each negative-tested: - an edit through one handle is visible through the other -- the whole point of the phase; - the generation counter is shared, so SceneCache::scene_for cannot serve a stale scene to a viewport that has not synced yet; - two separately constructed documents do NOT share, so the first two cannot pass for the wrong reason; - sequential borrows are fine and overlapping ones are rejected. That one states the borrow discipline as an executable rule rather than a comment, using try_borrow_mut so it documents the failure without aborting. Verified before designing, rather than assumed: - Every `self.parts` read in viewport.rs is a short-lived expression or a loop, and only two loops mutate `self` at all (~6231, ~6265) -- both touching `selection`, a different field, so neither becomes a BorrowMutError. - `split_for_command` hands out `&mut Vec<CadNode>` tied to `&mut self`, which is the one construct genuinely awkward under RefCell. Every caller already scopes it in a block and drops it before touching `self` again, so it maps onto a RefMut guard -- but that holds because of how the callers are written, not because the signature enforces it, and ARCHITECTURE.md now says to re-check it rather than assume it. The migration rule -- never hold a `parts()` guard across a call that can reach `parts_mut()` -- is recorded with the two loops to re-check first if a BorrowMutError ever appears. 738 lib + 154 integration, 0 failed. All twelve gates pass. |
||
| f021bdc0f5 |
fix(pay): the gate message hid the way to turn automation back on
Reported: the *334# automation used to work and now cannot be reached, and the status message reads as though the capability was removed. The capability was not removed. Verified with git: - `demo` and the USSD dispatch gate are both present in |
|||
| ea3eee6d41 | refactor(spreadsheet-ui): name model sheet selection explicitly | |||
|
|
5e2d578583 |
fix(project): port the mobile grid to the shared workspace model
origin/main (
|
||
|
|
5a4ddc993f |
refactor(cad): share one part-id allocator across viewports (Phase 5.2)
First piece of state to become genuinely shared rather than copied. Last commit fixed the id allocator moving backwards during sync by reconciling the three counters on every push. That bounded the damage but left the cause: three viewports each owning a `next_part_id`, kept in step by copying whole snapshots, where only one viewport can win a frame. A counter duplicated three ways cannot be made correct that way, only patched -- which is exactly the argument for Phase 5.2, so this takes the step instead of adding another patch. `PartIdAllocator` is an `Rc<Cell<u64>>` handed to all three viewports by `CadWorkspace::share_part_id_allocator`. An id handed out in any view is never handed out again, with no sync step involved. `Rc<Cell>` rather than `Arc<Mutex>`: the viewports are UI-thread only, and a `u64` is `Copy` so no borrow can outlive the call and there is no `RefCell` panic to reason about. Removed as a result: - `next_part_id` from `CadViewport` and all five open-coded `let id = self.next_part_id; self.next_part_id += 1;` sites. - The `next_part_id` element of `parts_snapshot` / `replace_parts_snapshot`. The snapshot pair is one field smaller, which is the direction Phase 5.2 is going. - `reconcile_next_part_id` and its four tests, now dead. Deleting the workaround in the same commit that removes its cause -- leaving both is how this codebase accumulated two of everything. Kept: ids arriving from outside the allocator (script rebuild, file load, undo restore) were never issued by it, so `reserve_past_nodes` still runs on every `replace_parts_snapshot`. Five tests. The load-bearing one asserts two clones interleave without collision AND that they alias the same counter; a companion asserts two *separately constructed* allocators do not share, so the first cannot pass for the wrong reason. Negative-tested by making `clone` deep-copy: "ids must be globally unique" fails. Also covered: reserving past adopted nodes, reserving never lowering the counter, and saturating at u64::MAX rather than wrapping to 0 and reissuing live ids. 734 lib + 154 integration, 0 failed. All twelve gates pass. |
||
| 6f4fb8058e | refactor(spreadsheet-ui): keep shared model attachment single-source | |||
| 92b1daf9ba | refactor(spreadsheet-ui): remove grid-owned spreadsheet data | |||
| 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.
|
|||
|
|
cbe47a5f6c |
perf(sms): close the outstanding Phase B and D items
Some checks failed
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (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
repo hygiene / hygiene (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
An audit of Phase 0 through D against the tree found three tasks marked
done in prose but absent from the code. This closes them.
D5 -- bulk send still blocked the UI thread.
The send loop ran synchronously in handle_send_bulk.
SmsManager.sendTextMessage queues to the radio and rate-limits, so a
200-recipient batch was a multi-minute ANR with no progress and no
way to tell whether anything was happening. Phase E8's throttle
bounded the worst case at 30 sends, but bounded blocking is still
blocking -- and I said as much when deferring it.
Now uses the worker pattern D1 established: spawn, publish progress
through a mutex-guarded slot, SignalToUI, drain on the UI thread. The
status line counts up ("Sending 12/200…") instead of freezing.
BULK_SEND_IN_FLIGHT prevents two overlapping batches.
D2 -- the ContentObserver, the half I left open.
Phase D removed the 5-second poll that re-armed itself via redraw()
and stopped the app ever idling. That fixed the busy loop but left a
gap I documented rather than closed: a message arriving while the app
was open did not surface until the next Resume or manual pull.
Adds SmsInboxObserver.java -- a ContentObserver on content://sms,
registered with a main-Looper Handler, idempotent so onResume can call
it freely -- compiled and dexed by the existing build.rs pipeline and
loaded through the same in-memory dex loader as the receivers.
onChange calls into Rust, which does two cheap things: set an atomic,
and invoke a registered waker. The waker matters. robius-sms has no UI
dependency and cannot call SignalToUI itself, so without it the flag
would only be observed on the next event-loop turn that happened for
some other reason -- which, with the poll gone, might be never while
the app sits idle. The app registers SignalToUI::set_ui_signal, so
this is a genuine push.
Native binding is dynamic, not #[no_mangle], for the same reason as
E6: the class comes from an in-memory dex and is not on the JVM's
search path.
B0 (wider) -- 23 crates declared robius-sms and never called it.
Phase B removed the three declarations that put RUSTSEC advisories on
nigig-build and explicitly flagged the rest as "the same latent
problem, sweep separately". This is that sweep: every crate with zero
references to robius_sms in its sources loses the dependency.
nigig-mpesa, nigig-pay and nigig-sms keep it -- they are the only real
users. nigig-system-prefs only mentions robius-sms in its package
description, so its manifest is untouched.
Cargo.lock loses another 21 lines.
Verified: 13/13 CI checks. clippy -D warnings clean on host and
aarch64-linux-android; the Android build compiles, javac-builds and
dexes the new observer class. cargo deny still "advisories ok, bans ok,
licenses ok, sources ok". clippy ratchet holds at 49. Sampled four of
the 23 stripped crates plus all five I edited; all build.
Pre-existing and unrelated: nigig-map fails to compile on pristine
origin/main (12 errors in view.rs, a Script/Widget derive problem), so
pageflipnav and anything else reaching it cannot be checked here. I
touched no files under crates/apps/map.
NOT verified on a device. The observer's registration, the onChange
callback and the waker all need an emulator or handset with a live SMS
provider; this sandbox has neither, and there is still no CI runner.
|
||
|
|
44c87d4f7b |
fix(cad): viewport sync reissued live part ids and undid edits
Two defects in sync_parts_from_any_dirty_viewport, both from the same root cause: three viewports each own state that only one of them can win with per frame. 1. THE ID ALLOCATOR MOVED BACKWARDS. Each CadViewport owns a `next_part_id` counter, and replace_parts_snapshot assigned the source's value directly. Only one viewport wins a sync frame, so the loser's allocations are absent from the winner's snapshot -- and its counter is then reset below what it has already handed out. Draw a part in the 2D view and one in the 3D view between two syncs: both allocate the same id independently, the loser's part is discarded, and the next part in that view reuses an id that is still live. NodeId is the key for MeshCache, part_geoms and every command, so a duplicate silently aliases two parts. Now reconciled via scene_holder::reconcile_next_part_id, which takes the max of the destination's counter, the source's, and one past the highest id actually in the incoming list. It cannot retreat, cannot collide with a live id, and saturates rather than wrapping at u64::MAX. Four tests, negative-tested. 2. THE LOSING VIEWPORT UNDID THE WINNER'S EDIT. The source loop broke on the first dirty viewport. take_script_dirty clears the flag as it reads it, so a viewport never visited kept its flag set, won the *next* frame, and pushed its own now-stale snapshot back over the edit that had just been applied. The loop now visits all three and clears every flag in one pass. The first dirty one still supplies the snapshot; the others' edits are already in it, because a sync pushes the winner's full parts list. Modelled both interleavings against the real control flow before changing anything. These are worth recording as evidence for Phase 5.2 rather than just fixed: a single counter duplicated three ways cannot be made correct by copying, only bounded. ARCHITECTURE.md now says so at the point where someone would otherwise reach for another patch. 730 lib + 154 integration, 0 failed. All twelve gates pass. |
||
|
|
ac1ddfe2c2 |
feat(doc): float the platform clipboard menu on long-press selections
The touch clipboard surface is now complete: a mobile long-press selection requests the platform clipboard menu (cx.show_clipboard_actions, iOS/Android backends) right after the selection lands, and its actions re-enter through the synthesized hits the editor already answers -- Copy/Cut as TextCopy/TextCut, Paste as TextInput (which also flows through multi-line block splitting). - Edit mode only: View keeps the long-press for merge/highlight (its hits stay gated), and desktop sessions never reach the touch-driven frame clock, so their clipboard story is unchanged. - has_selection follows the copyable payload exactly like the native menu: word selections anchor on the union of their glyph rects and offer Copy/Cut, while a cell range carries no payload and gets a paste-only menu anchored on the pressed cell's rect. - The request is mirrored on the widget as pub clipboard_menu: makepad's platform op queue is crate-private, so hosts rendering their own menu (and tests asserting the request) read it there. Runtime tests (real TouchUpdate + 24-frame NextFrame clock) cover the Edit word-menu request with Copy flowing back out, the cell paste-only menu rect matching the pressed cell with a menu Paste splicing into the parked cell, and View mode making no request at all. |
||
|
|
5c8ee16cbb |
fix(cad): a dropped rebuild request left the spinner up forever
CadRebuildWorker::request discarded the channel send error:
pub(crate) fn request(&self, request: CadRebuildRequest) {
let _ = self.request_tx.send(request);
}
The caller then unconditionally sets rebuild_pending = true and shows
"Computing 3D model...", and that flag is cleared in exactly one place:
drain_rebuild_results, on a result whose seq matches. So if the worker
thread is gone the request vanishes, no result can ever arrive, and the
spinner stays up for the rest of the session with every later edit
appearing to hang. No error is logged and nothing recovers.
request now returns whether the send succeeded. request_rebuild acts on
it: drops the dead worker so the next edit constructs a fresh one,
clears rebuild_pending, and says "Rebuild worker stopped; retrying on
the next edit" instead of lying about work in progress.
Test builds the worker from raw channel halves so a closed receiver can
be staged without a thread or a Cx. Negative-tested by restoring the
`let _ =`.
Also examined and found sound, recorded because the absence of a bug is
worth knowing:
- The seq protocol. I suspected a stale result could strand the pending
flag and modelled the interleavings; it cannot. The worker only emits
seqs the UI requested, seqs are monotonic, and drain keeps the last
result in the channel -- so the final result always matches the
current seq. wrapping_add on a u64 needs 2^64 rebuilds.
- Worker panic safety. catch_unwind wraps the evaluation, and both
save_cad_state and cad_mesh_data_from_solid are inside it, so a panic
in either is converted to an Error payload rather than killing the
thread. The dead-worker path this commit handles is therefore
defensive rather than routine -- but it costs three lines and the
failure it prevents is unrecoverable without a restart.
Swept the module for other discarded sends: none remain.
726 lib + 154 integration, 0 failed. All twelve gates pass.
|
||
|
|
92e4a2515a |
fix(cad): a failing undo or redo destroyed the command
UndoRedoStack::undo popped the entry and then ran it:
let cmd = self.undo_stack.pop_back().ok_or(..)?;
cmd.undo(ctx)?; // <- early return drops `cmd`
self.redo_stack.push(cmd);
If `cmd.undo` fails, `?` returns while the command is still a local, so
it is dropped: gone from the undo stack and never reaching the redo
stack. The user silently loses a history entry, and that edit can never
be reversed again. `redo` had the identical shape.
This is reachable, not defensive. Every command's undo bottoms out in
move_node / update_node / delete_node, each of which returns
NodeNotFound when the node is missing -- which is exactly what happens
after a script rebuild replaces the parts list. Undo an edit to a part
the script no longer produces and the entry evaporates.
Fixed by running the command while the stack still owns it (`back()` /
`last()`) and only moving it across after it succeeds. The subsequent
pop cannot fail, but is still handled with `if let` rather than an
unwrap, matching the no-panic rule this module now enforces.
Two tests, negative-tested by restoring the pop-first order: both assert
`undo_depth + redo_depth == 1` after the failure, i.e. the command is
still *somewhere*. Asserting only that undo returns Err would have
passed against the broken code -- it did return Err, while destroying
the entry. The failure message prints both depths, so a regression says
which stack lost it.
Swept the rest of the CAD module for the same pop-then-fallible-call
shape: no other instances.
725 lib + 154 integration, 0 failed. All twelve gates pass.
|
||
| a176b212ea | refactor(spreadsheet-ui): bind shared model before first grid draw |