fix(ui): close the review findings from #510's notification_status fix

[sanity]

Follow-up to #542 (issue #510), which had already merged as f262d79d by the
time independent review came back. These are that review's findings, plus the
one item it left to my judgement.

Problem

M1 — the "reset the flag" half of #510 had zero coverage. Both statements of
the re-arm at notifications.rs could be deleted with the entire suite green:

baseline                                       787 passed
delete ENABLE_PROMPT_REARMS.fetch_add(1, ...)  787 passed   <-- survived
delete ENABLE_PROMPT_SENT.store(false, ...)    787 passed   <-- survived

Each shipped a silent regression. Without the SENT reset an unanswered prompt
never re-arms, which is literally the "no way to retry" the issue is about.
Without the REARMS increment the counter stays at 0, so rearms_used < MAX is
always true and the re-arm becomes unbounded — the shell's affordance bar
returns on every message the user sends, the exact nag the cap exists to prevent.

My own review comment on #542 claimed the_automatic_rearm_is_bounded covered
the second mutation. It does not: that test feeds the predicate a
caller-supplied rearms_used, so it passes with the increment gone. The row was
true for changing the constant, not for breaking the counter. My disclosed
reasoning ("no native test drives the real atomics — process-global statics vs
parallel test threads") correctly ruled out a behavioural test but stopped
there, missing the cheap remedy this same file already uses four times.

m4 — a vacuous assertion in notification-shell-status.spec.ts. The
unrecognised-status check asserted text that was already on screen from the
preceding denied, and toContainText passed on its first poll before the junk
could have been processed. It would have passed just as well had the junk
clobbered the stored status.

m3 — debug! for an unrecognised status is compiled out in release
(release_max_level_info). That line fires exactly when the cross-repo status
contract has drifted, which by my own test's rationale "reverts #510 exactly".

m8 — the status was only visible inside the bell modal. A user whose
permission is Denied saw an ordinary bell reading "Notifications: All
messages" and learned nothing unless they went looking. For an issue titled
"fails silently", that was the last place the silence lived.

Approach

M1: a source pin, which has no parallelism problem. It asserts each
statement individually with its own failure message, and then the whole guarded
block as one string so the two cannot drift apart or escape the bounded
predicate.

m4: the junk check is a non-event, so it now waits out a window rather
than racing — the write path is a single setTimeout(0), so a clobber renders
well inside it. It then posts a recognised status and waits for that to land,
because a parse that threw and killed the listener would otherwise look
identical to correctly ignoring the junk.

m3: warn!.

m8: I implemented it rather than declaring it out of scope. The bell gets a
small dot plus an aria-label explaining why. Two deliberate exclusions: a
Muted room is never marked (the user asked for no notifications, so warning
them they'll get none is noise), and Unsupported is never marked (nothing
the user does could ever clear it — a permanent badge is nagging, not
informing; the modal still explains it). The button's title stays the per-room
mode alone, so the tooltip keeps naming exactly one thing and the existing
exact-title assertions are untouched; the blocked state rides on aria-label
and the dot.

Testing

cargo test -p river-ui --bins: 791 pass (4 new). Playwright: 50 pass across
chromium, firefox, webkit, mobile-chrome and mobile-safari, plus 214 in the
layout-sensitive specs (responsive-layout, message-layout,
room-header-links, room-info-key-selection) since this adds an element to
the conversation header.

Every new assertion was mutation-verified, including the two the reviewer proved
had survived:

Mutation	Caught by
Delete ENABLE_PROMPT_SENT.store(false, ...)	an_unanswered_prompt_actually_rearms_the_flag_it_bounds
Delete ENABLE_PROMPT_REARMS.fetch_add(1, ...)	same
Replace the guard with a bare matches!	same
delivery_is_blocked always false	the_bell_is_marked_only_when_a_wanted_notification_cannot_arrive, plus both browser badge tests
Unknown status parsed as Undecided (clobber)	an_unrecognised_status_is_ignored_rather_than_guessed, and now the browser junk check at notification-shell-status.spec.ts:128

That last row is the point of the m4 fix: against the clobbering build the
browser assertion fails with Received string: "The notification prompt closed without an answer...". The version it replaces passed.

Closes the review of #542.

[AI-assisted - Claude]

On the record: two residual silent paths, on the shell side (freenet-core, not this PR)

Independent review of #542 surfaced these. I verified both against
crates/core/src/server/path_handlers/assets/shell_bridge.js on freenet-core
main rather than relaying them, since they are claims about another repo.

1. A per-message delivery drop is never reported. showAppNotification
   returns silently when the Notifications API is absent, when
   Notification.permission !== 'granted', or when contractHasConsent() is
   false. None of those paths calls notifyStatusToIframe.
2. The affordance-already-showing path returns with no status:
   if (notifyAffordanceShown) return; in maybeOfferNotifications.

Net effect, and it is the interesting one: a granted recorded once can keep
River displaying "Desktop notifications are enabled" indefinitely while every
delivery is dropped — for instance after the user revokes the permission in
site settings, which River cannot observe from an opaque origin.

That is the same staleness class this PR's predecessor fixed on the top-level
path, where River reads the browser's live answer and ignores what it
remembered. Framed, River has no such reading available, so only the shell can
close it: it would need to report the drop rather than returning silently. The
fix therefore belongs in freenet-core, not here. Happy to file it if wanted.

[sanity]

Independent review

Reviewed at 712c7ec3 by a reviewer that did not write the code, reading the source before the PR description. Verdict: safe to merge — nothing blocking. Two minors are worth folding in first; both are the same silent-failure class this PR exists to close.

The M1 pin genuinely pins, needle by needle

Simulated statically against the checked-out source:

mutation	needle1	needle2	needle3 (whole block)	pin
unmutated HEAD	pass	pass	pass	PASS
delete ENABLE_PROMPT_SENT.store(false, …)	FAIL	pass	FAIL	FAIL
delete ENABLE_PROMPT_REARMS.fetch_add(1, …)	pass	FAIL	FAIL	FAIL

Each deletion is caught twice. production_source() splits at the first mod notify_gate_tests while the needle literals live in the test half, so the include_str! self-match trap does not bite; each needle occurs exactly once in the whitespace-stripped production half, so no broader-string satisfaction; whitespace-stripping survives cargo fmt. Both mutations that were confirmed by execution to leave all 787 tests green on #542 are now closed.

The second mutation's stated consequence also checks out: MAX_ENABLE_PROMPT_REARMS = 1 and line 452 is the only increment in the crate, so deleting it pins rearms_used at 0 and the bound is always true — unbounded re-arm.

m8 is aria-clean and layout-clean, and two design calls are vindicated

The repo's known trap — a badge's aria-label joining its ROW's accessible name, with getByRole matching by substring — does not bite: the badge is aria-hidden="true" and the bell is a sibling rather than nested. Every spec in ui/tests/ was swept; nothing locates the bell or a neighbour by getByRole({name}), getByLabel, or an aria snapshot.

Keeping the blocked state off title was load-bearing rather than stylistic: toHaveAttribute with a string is an exact match and Chromium denies notifications by default, so appending to title would have broken all three existing assertions at notification-bell.spec.ts:43,86,114.

Layout is safe — relative does not change flow, the badge is absolute and out of flow, and flex-shrink-0 is retained.

Both exclusions upheld. Unsupported is right because no user action can ever clear that marker (no Notifications API exists on that platform), in contrast to Undeliverable, which is marked and is clearable via OS settings — the line drawn is actionability. Undecided being unmarked was also probed and holds, because River is actively asking in that state, so it is not silent.

Dioxus traps checked clean: NotificationMode is Copy so the read-once refactor compiles; no new closure props, so no memoization defeat; the try_read() Err path cannot be observed because the single write is deferred in single-threaded WASM, and the framed CI test proves the subscription fires.

MINOR-1 — the warn! from m3 is itself unpinned

notifications.rs:593. Reverting that one token leaves the entire suite green. ui/Cargo.toml:86 does carry release_max_level_info, so the rationale is correct — but this is the same unpinned-silent-behaviour class as M1, inside the PR closing it. Three lines with the existing helper, or better: assert the console.warn in the framed Playwright test, which would also allow deleting the waitForTimeout(500) from m4 (a fixed sleep means a slow clobber is missed as a false pass rather than surfacing as a flake).

MINOR-2 — ring-panel is a dead class

conversation.rs:3854. Verified independently: --color-panel is declared at tailwind.css:31, outside the @theme block (lines 9-26), and Tailwind v4 only generates colour utilities for --color-* registered in @theme. There are zero @utility ring-* definitions in the file, while every other :root colour required a hand-written one (@utility bg-panel, text-text, border-border…). ring-panel occurs exactly once in the repo — this badge.

Effect: ring-1 falls back to currentColor, so the separator ring renders grey (blue on hover) instead of panel-coloured. Silent, because Tailwind does not error on unknown scanned classes and toBeVisible() does not check colour. Fix: ring-[var(--color-panel)].

MINOR-3 — belongs in an issue against the shell, not here

A transient undeliverable marks the bell permanently. shell_bridge.js posts nothing on the success path, and notifyRegistrationReady(1500) can time out on the first message before the service worker is up — so on mobile Chrome/Firefox the first message can report undeliverable, the dot appears, delivery then starts working, and nothing clears it for the session. Inherited from the shell protocol (there is no "recovered" status), but this PR is what promotes it from a modal sentence to a persistent glyph.

Test discrimination

m4's vacuity is genuinely closed — the added toHaveCount(0) on the Enable button is what makes it discriminating, and the granted follow-up separately catches a parse that threw and killed the listener. m8 is pinned in both directions by the framed test (absent before any status, present after denied, gone after granted). Only m3 is unpinned (MINOR-1).

Note on freshness

CI is green on this exact head, and build.yml:311 runs bare npx playwright test, so all five projects exercised the new framed test. However this branch is four commits behind main (#546, #547, #552 merged since), so CI has not run on the merged combination — #552 changed the composer textarea class while this adds an element to the conversation header, both with Playwright layout coverage. A rebase and fresh CI run should precede merge.

[AI-assisted - Claude]

[sanity]

Review minors addressed, and rebased

MINOR-1 — the warn! was itself unpinned. Fair hit, and it is the same gap class one door down from M1. Pinned two ways:

• The framed Playwright test now waits for the listener's own console line instead of sleeping. That also removes the waitForTimeout(500), which was the weaker instrument you identified: a fixed sleep makes a slow clobber a false pass rather than a flake, whereas waiting for the log line is a positive signal that the junk was actually handled.
• A source pin gives the same guarantee in the fast native job (one second, not sixteen minutes) and does not depend on Dioxus continuing to spell the level "WARN" in the message text.

Worth recording: Dioxus writes every level to console.log with the level in the message text, so ConsoleMessage.type() is "log", not "warning". The assertion matches text accordingly. Verified with a throwaway probe rather than assumed.

Both instruments verified against the mutation: reverting to debug! fails the native pin, and fails the browser assertion because release_max_level_info compiles the line out of the release build the tests run against. That failure is also independent empirical proof of the compile-out, rather than just reading ui/Cargo.toml.

MINOR-2 — ring-panel was dead, and your mechanism was right. I confirmed it independently before changing anything: the built stylesheet had no .ring-panel rule at all, and .ring-1 resolves var(--tw-ring-color,currentcolor).

One correction to the suggested fix: ring-[color:var(--color-panel)] also generates nothing — I tried it first and grepped the output. What works is the hand-written @utility, matching what every other :root colour in tailwind.css already needs (bg-panel, text-text, border-border…). Added @utility ring-panel { --tw-ring-color: var(--color-panel); } and confirmed .ring-panel{--tw-ring-color:var(--color-panel)} now generates.

MINOR-3 — filed as freenet/freenet-core#5043, not changed here. I verified all three paths in shell_bridge.js before writing it up: notifyRegistrationReady resolving null for a registered-but-not-yet-active worker, the three silent returns in showAppNotification, and the notifyAffordanceShown early return. The issue makes the point you asked for — that this PR is what promotes a 1.5s worker-activation race into a session-long false warning, which is why it is worth fixing in the shell rather than documenting. It also notes that adding statuses is wire-safe, since River ignores unrecognised ones rather than guessing.

Rebased onto current origin/main (96f0bf06, picking up #546, #547, #552 and the #545 version bump). Clean, no conflicts. New HEAD 0044ce51; remote ref and PR head both confirmed at that SHA.

Re-verified on the rebased tree, not on the pre-rebase one: 792 native tests, 50 Playwright across all five projects, and 207 in the layout-sensitive specs — responsive-layout, message-layout, room-header-links, conversation-autoscroll — which matters because #552 changed the composer measurement while this adds an element to the conversation header.

Not merging and not arming auto-merge. CI is running on 0044ce51.

[AI-assisted - Claude]