andodeki
3a3db4efa6
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
doc-engine / coverage (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
email / gates (push) Has been cancelled
email / email-domain (push) Has been cancelled
email / nigig-email (push) Has been cancelled
email / supply-chain (push) Has been cancelled
nigig-map / test (push) Has been cancelled
p2p-intel / engine (push) Has been cancelled
p2p-intel / notifications (push) Has been cancelled
p2p-intel / coverage (push) Has been cancelled
p2p-intel / makepad-app (push) Has been cancelled
p2p-intel / exchange-tab (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
spreadsheet / engine-coverage (push) Has been cancelled
spreadsheet / ui-controller-coverage (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-build (CAD) / cad-engine-coverage (push) Has been cancelled
nigig-build (CAD) / doc-workspace-coverage (push) Has been cancelled
nigig-build (CAD) / cad-widget-coverage (push) Has been cancelled
Two gaps remained on the Apple and Windows side after the last commit, and
they are different in kind.
The first was a missing backend. iOS returned PermanentlyUnavailable with a
note saying it was not implemented. It now uses UNUserNotificationCenter --
the only option there, since every iOS process is bundled, so the
Objective-C exception that rules that API out on macOS cannot occur. The
objc2-user-notifications bindings are real and fetchable, so this
type-checks for aarch64-apple-ios against the actual framework, and the
module was confirmed genuinely reachable by injecting a type error and
watching the target build fail.
Its is_available() returns a constant false, and that is a limit worth
naming rather than burying. The real answer comes from
getNotificationSettingsWithCompletionHandler:, which is asynchronous -- it
hands the settings to a block on an arbitrary queue. A synchronous
is_available() could only produce that by blocking on a completion handler,
which on the main thread is a deadlock rather than a delay. Returning false
errs toward telling the user delivery is unverified; the alternative is
claiming an availability the platform never confirmed, which is the whole
failure this crate was written to remove. A correct answer needs an async
entry point, which is a change to the public API rather than a bug fix, so
it is recorded in the ADR instead of being quietly wrong.
The second gap is subtler and, I think, the more valuable fix. The Apple and
Windows backends contain pure logic -- XML escaping, tag clamping -- that
was sitting inside #[cfg(target_os = "windows")], where cargo test on the
only available machine could never reach it. Those functions had zero tests
and no prospect of any.
They now live in src/payload.rs, which is cfg-free and runs on every target.
Eleven tests cover them, and they cover exactly the rules a compiler cannot:
an unescaped `&` in a merchant name makes the toast XML malformed and
Windows discards the whole notification rather than showing a mangled
character, and the Binance P2P book is full of `&`. A byte-wise truncation
of the 64-character tag limit panics outright on the full-width names that
book also contains -- so the clamp cuts on character boundaries, pinned by a
test that would panic if anyone changed it back.
One test pins something deliberately counter-intuitive: escaping is not
idempotent, and escape_xml("&") is "&". That is correct, and the
test exists so nobody "fixes" double-escaping by teaching the function to
detect already-escaped input, which is precisely how escaping filters grow
holes.
The distinction this commit is really about: the last one made the Apple and
Windows backends *compile*, which catches wrong selectors -- it found
CreateToastNotifier(&HSTRING), which does not exist. It could not catch
wrong behaviour. Extracting the logic is what makes the behaviour testable
on a machine that will never run either platform.
62 tests, up from 51. Clippy clean with -D warnings on all five targets:
linux-gnu, linux-android, apple-darwin, apple-ios, windows-msvc. p2p-intel
unchanged at 225.
169 lines
8.4 KiB
Markdown
169 lines
8.4 KiB
Markdown
# ADR 0036 — robius-notification: a zero-dependency D-Bus client, and three honest gaps
|
|
|
|
Status: accepted
|
|
Date: 2026-09-01
|
|
|
|
## Context
|
|
|
|
ADR 0035 left one of p2p-intel's four limits open: there was no OS
|
|
notification backend, so alerts stopped when the app window closed. The seam
|
|
existed (`NotificationSink`) with one implementation that truthfully did
|
|
nothing.
|
|
|
|
This records building the crate that fills it, because three decisions in it
|
|
are not obvious and one of them reverses on evidence.
|
|
|
|
## Decision
|
|
|
|
### Linux is written by hand, with no dependencies
|
|
|
|
Posting a notification is one D-Bus method call. Three ways to make it were
|
|
measured in the sandbox rather than guessed at:
|
|
|
|
| Approach | Crates | Problem |
|
|
|---|---|---|
|
|
| `notify-rust` + `libdbus-sys` | 12 | a C library; needs `pkg-config`, breaks the Android and iOS cross-compile |
|
|
| `notify-rust` + `zbus` | 169 | an async executor, for one method call |
|
|
| hand-rolled | **0** | ~250 lines that must be exactly right |
|
|
|
|
`robius-sms` deleted `polkit` and `gio` for exactly this reason. Its E9 note
|
|
records that they were the sole source of two RUSTSEC advisories and an
|
|
LGPL-2.1 question for every consumer, and that nothing in the crate
|
|
referenced them. Adding a 169-crate tree back into the same family would
|
|
reverse that decision for a worse reason.
|
|
|
|
The cost is real: the D-Bus wire format has to be correct, and a mistake
|
|
makes the daemon disconnect with no diagnostic. That cost was paid in tests
|
|
(below) rather than in production.
|
|
|
|
### The tests run against a real daemon, not a mock
|
|
|
|
`dbus-daemon` is available, so the suite starts a private session bus per
|
|
test. This matters more than it sounds. The marshaller and the parser were
|
|
written from the same reading of the specification, so **them agreeing with
|
|
each other proves only that I was consistently wrong or consistently right**.
|
|
Only a third party — an actual bus — can tell which.
|
|
|
|
It found three bugs no unit test would have:
|
|
|
|
1. **Every error reply parsed as success.** The header walk assumed all
|
|
fields were strings; `REPLY_SERIAL` is a `u32`, and reading its four bytes
|
|
as a string length desynchronised the cursor so `ERROR_NAME` was never
|
|
reached. `post` returned `Ok` against a bus with no notification service.
|
|
That is the exact bug this crate was written to eliminate, reintroduced
|
|
inside its own parser — which is the strongest argument available for
|
|
testing against something you did not write.
|
|
2. **`is_available()` was true on a bare bus.** `NameHasOwner` *succeeds* and
|
|
reports `false` in its body. Checking only for an error meant reporting a
|
|
working notifier on a machine with no notification daemon.
|
|
3. **Replies were not correlated.** The bus sends `NameAcquired` unprompted
|
|
after `Hello`. "Read the next message" consumed that signal and treated it
|
|
as the reply. Now matched on `REPLY_SERIAL`.
|
|
|
|
The suite also serialises env-var mutation behind a mutex. `--test-threads=1`
|
|
would have made the failures go away too, and would have hidden a real
|
|
hazard from the next reader.
|
|
|
|
### All four are written; only Linux is executed
|
|
|
|
An earlier revision of this ADR refused to write the Apple and Windows
|
|
backends, on the grounds that code no compiler has seen is not an
|
|
implementation. That reasoning was right; the premise was wrong.
|
|
|
|
`cargo check` needs the **target's standard library**, not a linker or an
|
|
SDK. `rustup target add x86_64-pc-windows-msvc aarch64-apple-darwin` makes
|
|
the real `windows` and `objc2` crates — with the genuine WinRT metadata and
|
|
Objective-C class definitions — available to the type checker on a Linux
|
|
host. So all four backends are now written and every one of them compiles
|
|
against the actual frameworks:
|
|
|
|
| Backend | Compiles for | Executed |
|
|
|---|---|---|
|
|
| Linux | `x86_64-unknown-linux-gnu` | **yes**, against a live `dbus-daemon` |
|
|
| Android | `aarch64-linux-android` | no device |
|
|
| macOS | `aarch64-apple-darwin` | no machine |
|
|
| Windows | `x86_64-pc-windows-msvc` | no machine |
|
|
| iOS | `aarch64-apple-ios` | no device |
|
|
|
|
Compile-checking is not a formality here. It immediately caught
|
|
`ToastNotificationManager::CreateToastNotifier(&HSTRING)`, which does not
|
|
exist — the AUMID overload is `CreateToastNotifierWithId`. Nothing short of
|
|
a compiler holding the real metadata would have found that, and it would
|
|
have shipped as plausible-looking text.
|
|
|
|
Because a `cfg`-gated module can silently compile to nothing, each backend
|
|
was verified to be genuinely reachable by injecting a type error and
|
|
confirming the target build failed. Both macOS and Windows were checked this
|
|
way.
|
|
|
|
Two platform decisions are worth recording:
|
|
|
|
**macOS uses `NSUserNotification`, not `UNUserNotificationCenter`.** The
|
|
modern API raises an Objective-C exception when the process has no bundle
|
|
identifier, and that unwinds through Rust frames and aborts the host — so a
|
|
non-bundled `cargo run` binary would crash rather than report unavailable.
|
|
`NSUserNotification` is deprecated; aborting the host is worse than
|
|
deprecated.
|
|
|
|
**Windows requires the host to supply an AppUserModelID.** It comes from an
|
|
MSIX manifest or a Start Menu shortcut, which is packaging a library cannot
|
|
invent, and a fabricated one yields a notifier that constructs and then
|
|
fails at `Show`. `set_app_user_model_id` is therefore part of the public
|
|
API, a no-op off Windows so portable hosts call it unconditionally, and
|
|
`is_available()` is false with an explanatory reason until it is called.
|
|
|
|
**iOS uses `UNUserNotificationCenter`** — the only option there, since every
|
|
iOS process is bundled so the exception that rules it out on macOS cannot
|
|
occur. Its `is_available()` returns a constant `false`, which is a limit
|
|
worth naming rather than hiding: the real answer comes from
|
|
`getNotificationSettingsWithCompletionHandler:`, which is asynchronous, and
|
|
a synchronous `is_available()` could only produce it by blocking on a
|
|
completion handler — a deadlock on the main thread. `false` errs toward
|
|
telling the user delivery is unverified, and the alternative is claiming
|
|
availability the platform never confirmed. A correct answer needs an async
|
|
entry point, which is an API change rather than a bug fix.
|
|
|
|
**The pure logic is out of the `cfg` blocks.** XML escaping and tag clamping
|
|
now live in `src/payload.rs`, which compiles and is tested everywhere. While
|
|
they sat inside `#[cfg(target_os = "windows")]` they were unreachable by
|
|
`cargo test` on the only machine available, and they are exactly the kind of
|
|
rule a compiler cannot check: an unescaped `&` in a merchant name makes the
|
|
toast XML malformed and Windows discards the notification silently, and a
|
|
byte-wise truncation of a 64-character tag limit panics on the full-width
|
|
names the Binance book actually contains. Eleven tests cover them.
|
|
|
|
What remains unproven is behaviour: whether a toast actually appears,
|
|
whether the macOS centre accepts the notification. Only a device settles
|
|
that, and the support table keeps "compiles" and "executed" in separate
|
|
columns so nobody reads one as the other.
|
|
|
|
## Consequences
|
|
|
|
- p2p-intel's fourth limit is closed **on Linux**, narrowed on Android, and
|
|
unchanged on Apple and Windows. The README table says exactly that.
|
|
- `SystemNotifications` replaces `UnavailableNotifications` as the dashboard's
|
|
sink. `UnavailableNotifications` stays: on platforms with no backend it is
|
|
still the truthful answer, and a test needs something that reliably cannot
|
|
deliver.
|
|
- The dashboard already displays `unavailable_reason()`, so on a machine with
|
|
no notification daemon the user is told, rather than assuming they are
|
|
covered.
|
|
- Alerts are tagged per fiat, so a market's alert replaces its own previous
|
|
one instead of stacking. A 30-second poll would otherwise fill the shade,
|
|
and a full shade is what makes someone disable notifications for the app.
|
|
|
|
## Alternatives rejected
|
|
|
|
**Taking `zbus` for a quiet life.** 169 crates and an async runtime in every
|
|
consumer of a crate family that has been deliberately kept lean. The hand-
|
|
rolled client is more code *here* and far less code *everywhere else*.
|
|
|
|
**Leaving Apple and Windows as stubs.** That was the earlier decision and it
|
|
was over-cautious: it assumed compile verification was unavailable when a
|
|
`rustup target add` would have provided it. Type-checked code against real
|
|
framework metadata is a materially different artefact from untested prose,
|
|
and the distinction is now carried by the support table rather than by
|
|
refusing to write the code.
|
|
|
|
**Claiming the four backends are equally done.** They are not, and the table
|
|
says so in two columns. Only Linux has posted a notification.
|