Skip to content

feat: add shared browser and standard desktop notifications - #38

Merged
wesbillman merged 19 commits into
mainfrom
pinky/notifications-simple
Sep 13, 2026
Merged

wesbillman merged 19 commits into
mainfrom
pinky/notifications-simple

Conversation

@wesbillman

@wesbillman wesbillman commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Pinky is updating this PR on Wes's behalf.

Summary

  • One host-owned notification policy, account-local Settings and trusted-plugin categories. Mentions, DMs and participating-thread replies reuse verified live kind-9/kind-40002 traffic and the existing unread/viewing/read-marker readiness owners—no additional socket or unread engine.
  • Bounded plain-text arrival previews use cached sender/conversation names. History/reconnect replay, own messages, membership activity and observer telemetry stay quiet.
  • Browser and running desktop clicks use existing scoped navigation to open/focus the exact message or thread through the normal timeline/thread panel. Clicks never mark content read; ordinary visible, focused dwell does.
  • Desktop has a main-window Tauri bridge to mac-notification-sys 0.6.15, the standard freedesktop notification interface through zbus 5.19.0, and tauri-winrt-notification 0.7.3. It restores/foregrounds Buzz before activation, retains Windows callbacks after banner timeout, and uses GTK present on Linux. Linux now arms a same-connection native receiver before Notify. No dependency upgrades or new FFI. The redundant permission-only official plugin and synthetic permission shim are removed; desktop Settings honestly describes OS-controlled permission and sound, while browser permission controls remain.
  • No second policy/destination owner, durable notification store, cold-start recovery or exactly-once delivery system. Only bounded display text and an opaque presentation UUID cross the native boundary—not accounts, credentials or navigation destinations.

See notification behavior and limits. Message details are sent to the OS; lock-screen/preview settings control their visibility.

Current delivery and gates — 2026-09-13

Pushed head: 622cc50dcdc635153c8f805c3c858894286d4018, local/remote verified. New CI is running; DCO Check passed. The previous red Chromium focus failure has a reproduced production cause and a committed fix, detailed below—not an unrelated/flaky classification. Wes has confirmed successful running macOS and Chrome banner → foreground → exact-reply clicks; remaining platform/edge-case limits are recorded below. GitHub shows this PR non-draft; that UI state is not validation or approval. The repository requires CI required, DCO Check, one approving review and code-owner review. No approval or merge performed.

Chromium focus correction — 622cc50

  • In the failed CI merge bf803950, closing a panel restored focus to the trigger, then timeline reflow briefly unmounted that message row. Focus fell to the document and did not return when the row remounted. The CI trace shows this eviction; a throttled Chromium run at that exact merge reproduced the same failure with focus/mutation events.
  • 14 production lines keep only the focused message mounted through Virtua's existing keepMounted, keyed by message ID across prepend and released when focus leaves. No focus-retry subsystem or notification redesign. Existing viewport checks still exclude offscreen rows from read evidence.
  • The original throttled repro passes with the fix. A deterministic new browser regression proves real neighboring-row eviction, retained focused control, and release after focus leaves; both Chromium and WebKit fail without the fix and pass with it, without CPU throttling or added sleeps. The original panel-close focus assertion now runs after layout settles so transient focus cannot pass it prematurely.
  • All 16 layout cases pass on exact CI merge + the same production/browser diff. At actual pushed 622cc50, normal pre-commit/pre-push passed formatting, TypeScript and all 1,228 Vitest tests / 121 files. Added unit coverage checks ID/index changes on prepend, within-history focus, exit, history focus and removal. All 19 PR commits have author-matching DCO; the commit signature verifies.
  • Brain independently reviewed the exact three-file content, including resolved Virtua 0.51.0 behavior, with no source blocker. This is source review, not human approval or OS acceptance.
  • Failure retained: a preceding full Vitest run timed out unchanged signed-boundary.test.ts:91 after 5 seconds (1,227 passed); the earlier run and final normal push passed. Its cause remains unestablished; no timeout, worker override, retry setting, test weakening or transport rewrite was added.
  • Stable build commit handed to Brain for Wes's authorized live web/macOS test setup. Wes subsequently confirmed one successful macOS click and one successful Chrome click (details below). This fix adds no native code; untested platform/edge cases remain explicitly open.

Current review comments

  • The original kind-40002/read-marker blockers were fixed at 0e34478 and explicitly cleared in review. They remain fixed.
  • Carl's latest finding, repeated by the next automated review, describes one valid Linux early action/close race at 7298d95, not two defects. Permission cleanup alone does not resolve it.
  • d78a7d9 replaces the affected notify-rust wrapper with the already-locked zbus API. One sender/path/interface-filtered signal stream is active before Notify on the same connection, including unicast-to-caller delivery. A single pinned Notify future and that stream are polled together so the 64-slot library queue cannot stall the reply. First terminal responses are retained by ID until the server returns its ID; the 128-distinct-ID resource limit reports failure rather than silently evicting an action. Overflow is an unknown display outcome, not proof of non-delivery. Pending owns once-only callback/capacity release; navigation and focus are unchanged.
  • Six Linux-only real D-Bus tests use isolated buses and a fake notification daemon: pre-reply click and close in unicast/broadcast modes, unrelated IDs, duplicate/first-terminal behavior, >64-signal burst, bounded overflow, unsupported actions and Notify error. The production operation owns subscription and correlation; test setup never pre-arms its listener. These tests executed on Linux at the previous head and caught a wire-type defect (details below). At corrected head 0548649 / CI merge bf8039507de29aff3beff303179a7b4df8703f49, all six Linux D-Bus cases passed within the 18/18 native-package tests. Carl’s exact-head source review subsequently cleared the original Linux P2. This is backend-boundary evidence, not real OS click/focus acceptance.
  • Brain's source review caught a test-ordering defect, corrected by keeping the overflow Notify reply pending until client completion. Both the initial implementation and source review missed the D-Bus wire type; Brain separately reviewed the one-line Vec correction. Source review is not execution or human approval.

Cleanup and validation

27289d9 removes the obsolete JS/Rust notification plugin, registration, permission capabilities and shim-dependent tests; desktop permission becomes directly unknown/system-managed and ineffective buttons disappear. Browser permission behavior, native delivery, policy and exact navigation are retained. Brain independently source-reviewed this cleanup.

Completed local batch stages on macOS arm64, attributed to 27289d9 plus the exact Linux patch subsequently committed as d78a7d9 (only the Linux-only overflow fixture changed after the broad runs):

  • 1,227 Vitest tests / 121 files, 38 Node integration tests, 16 plugin-manager tests.
  • 330 Chromium/WebKit functional journeys + 8 serial measurements, including browser permission and exact notification navigation; 14 design-browser tests. No retries or reduced assertions. The initial combined command hit its five-minute tool limit after measurements/20 journeys; the subsequent complete functional invocation passed. This was not one successful monolithic scan.
  • macOS workspace Rust formatting/Clippy and 12 native-package tests + 16 plugin-manager tests, with doctests.
  • Locked JS install, lint/app and design types/rules, frontend build passed on the permission-cleanup bytes; those JS inputs are unchanged by the Linux correction. Design build passed. Existing large-chunk advisories remain.
  • Exact Linux operation/test copies compile with pinned zbus/futures-lite and Clippy on Mac, including the final overflow fix. This is neither a Linux Tauri build nor a real D-Bus run. No dbus-daemon is installed on this Mac; the Linux CI lane explicitly provisions it and runs all package tests.
  • At d78a7d9 and again at exact corrected 0548649, normal pre-push passed TypeScript and all 1,227 Vitest tests / 121 files, no bypass or worker override. All 18 outgoing PR commits have verified author-matching DCO; the correction is cryptographically signed.

Previous run 34770449097 succeeded at 7298d95, per Carl's exact-head review: Linux/Rust/tool integration, Windows native, JS, browser measurements/journeys and aggregate. This supersedes the historical pending report below; it did not detect or fix the Linux race. Run 34774442281 at d78a7d9 passed Windows native, JavaScript and browser measurements, but failed Linux Rust tests as recorded below; remaining lanes were incomplete at the status check. Corrected 0548649 CI passed Linux Rust/tool integration, Windows native, JavaScript, measurements and three browser shards. Chromium shard 1/2 failed panel resizing preserves bottom follow and the visible reading anchor at layout.spec.mjs:340 in CI merge bf803950: the GitHub link remained inactive after closing its panel (86 passed, 1 failed). This was the focused-row eviction diagnosed and corrected at 622cc50 above. CI required therefore failed on that historical run; the new run remains pending. Integrated main remains a67102a; fetched main advanced to 541694c, not silently included here.

D-Bus wire correction — 0548649

The only change after d78a7d9 is src-tauri/src/notifications/linux.rs:56: vec!["default", "Open"] instead of a fixed array. Locked zvariant 5.15.0 serializes the Vec as the required as; the fixed array serialized as (ss). No test, timeout, policy or navigation change.

Failure retained: Linux job 103769840326, PR head d78a7d9 / CI merge b95f413c1fa33c6654c67c6ff1520fd8235b4ed7, passed Clippy but failed 5 of 18 native tests (13 passed). The three click/close/burst assertions explicitly report got (susss(ss)a{sv}i), expected (susssasa{sv}i); overflow/rejection also fail because Notify never reaches the fixture. Node integration was consequently skipped. This is the wire defect, not a flaky test or early-signal timeout.

Before the corrected push, an executable scratch probe on macOS serialized the exact production argument expression into a zbus Message and deserialized the standard Notify body: Vec passed, mutation back to the old fixed array failed with SignatureMismatch, and the restored Vec passed. Exact production/test copies also passed compile checks and Clippy. This probe proves serialization, not a real bus or OS click. Existing real-bus regressions caught the defect; they are unchanged, not weakened or duplicated. Native source SHA-256 b1ae5d40bc463690ce900b8dba345527ef8ad8c534d613a77a4e81d15aea7778; tests 9bec170c6b5f9cb37ac9479a922d50758b3ca557be6201b81e9d3f90ec36ce45.

Human web/macOS checks — 2026-09-13

Wes tested from this worktree during the 622cc50 handoff and explicitly reported both workflows successful:

  • Running macOS dev app: visible banner with sender/channel and message preview; clicking brought Buzz forward and opened the exact reply. Human confirmation. The screenshot shows Terminal attribution, the documented development limitation—not packaged Buzz branding.
  • Chrome / just web: WEB TEST 1 (4053ec1ed74e5e5b093a1829b377ba18822663653f9fad5dd09642b86fcf02ab) produced a working notification; Wes confirmed click → browser foreground → exact reply. Human confirmation.

Wes is satisfied with this manual pass; no further probes scheduled. These are human-confirmed happy-path checks, not full platform-matrix certification. Actual Windows/Linux GUI interactions, packaged macOS identity, separate minimized/Notification Center/dismissal/alerts-off and stale-access/capacity cases were not individually established by these two checks. The browser permission prompt itself was not separately reported.

Latest one-shot CI check at 622cc50: JavaScript, Rust/tool integration, Windows native, browser measurements, Semgrep, zizmor and DCO passed; all four browser journey shards still running. No approval or merge.

Native acceptance still required

The required contract is running click → foreground Buzz → exact message/thread on macOS, Windows and Linux. For each OS: two distinct targets, background/minimized Buzz, immediate click, fade then Notification Center click, dismissal without navigation, and old-account/revoked-access rejection. In particular Windows timeout→later activation, Linux compositor focus and actual packaged macOS identity need real interaction evidence. No native GUI was launched in this batch. Coordinate any visible launch with Wes; rebuild native code rather than hot-reloading into an old plugin-enabled app.

Native/frontend active presentations remain capped at 128. Account change/disposal invalidates navigation callbacks; an old alert may still foreground the current app. Permission/sound, actual display, OS history and installed identity remain OS-controlled. No artificial notification expiry, actual permission receipt, uniform withdrawal, cold/reload recovery, closed-app push or exactly-once delivery promise is added.

Previous native delivery report (historical; superseded by the current status above)

Current delivery and gates — 2026-09-13

Current pushed head: 7298d95ba35c72b1bf576c5dbc8a8783baff8d55. Local/remote heads match and the working tree is clean. Not merge-ready: corrected-head hosted CI is running, real native click/focus acceptance remains outstanding on all three OSes, and human/code-owner approval is required. DCO passed at this head. No approval or merge performed.

  • Native-click implementation: ddf468c; main conflict resolution is included via cd13d24, integrating main through a67102a. Main subsequently advanced to d8fa1a2; this branch does not claim to include that newer commit. GitHub reported mergeable at the last query.
  • Windows CI correction 7e585aa: exactly three cfg(unix) annotations align existing terminal helpers with their already-Unix-only callers. No Windows terminal implementation, behavior change or lint suppression.
  • CI-test correction 7298d95: include Windows in the existing real-shell required-gate regression, including failure/skipped/cancelled/empty/missing status cases. Notification production bytes are unchanged by both corrections.
  • At exact final 7298d95: normal pre-push passed TypeScript and all 1,226 Vitest tests / 121 files, without bypass or worker override. Every outgoing PR commit retains verified author-matching DCO.
  • Native correction snapshot: on macOS arm64, ddf468c plus the exact three-annotation patch subsequently committed as 7e585aa passed cargo fmt --all --check, cargo clippy -p buzz-foundation --locked --all-targets -- -D warnings, and all 12 native-package tests.
  • Gate-test correction snapshot: 7e585aa plus the exact test patch subsequently committed as 7298d95 passed Biome for that file and all 38 Node integration tests. Discovery tests list browser cases; they do not run their GUI journeys.
  • Hosted implementation snapshot ddf468c: JavaScript lint/types/build and all 1,226 tests passed. Browser measurements passed. Linux workspace Clippy and all Rust tests passed (12 native + 16 plugin-manager), before the job's later Node integration failure below. This is not a successful whole run or final-head browser gate.

Failures retained: implementation CI caught (1) Windows Clippy dead-code errors in the three pre-existing terminal helpers and (2) Linux's Node integration test still expecting the old four-lane aggregate, 37/38 passing. These are the exact failures addressed above. Superseding pushes cancelled the remaining old-run work; cancelled/incomplete browser journeys are not counted as passed. Corrected-head hosted success remains pending, not inferred from local checks.

Brain's independent production-source review and final JS/Unix-annotation/aggregate-test delta review found no outstanding source-demonstrated blocker. The JS tests exercise the real Tauri Channel serializer/ordering/cleanup while modeling IPC/native responses. The three Rust notification tests cover input and callback/capacity bookkeeping—not OS callback registration or foregrounding. Source review and test counts do not certify native acceptance or replace human approval.

Native acceptance still required

Wes explicitly expanded acceptance to click → foreground Buzz → exact content on macOS, Windows and Linux. Historical banner-only/non-goal statements below predate that instruction and do not define current scope.

Platform Native compile/test evidence Real running click/focus evidence
macOS arm64 Native package checks passed at the exact correction snapshot above Pending for this implementation
Linux Workspace Clippy and all Rust tests passed at ddf468c; whole job failed later in Node tests Pending, including immediate default action and background/minimized foregrounding
Windows First required native lane exposed and prompted the three target-gating corrections; corrected-head CI pending Pending, especially timeout → Notification Center activation

For each OS, exercise two distinct message/thread targets, background/minimized Buzz, immediate body click, fade then Notification Center click, dismissal without navigation, and old-account/revoked-access rejection. Coordinate visible macOS development launch with Wes; no new app launch or OS click has been claimed. Earlier macOS Terminal-attributed banner observations are historical only.

Native/frontend presentation ownership is bounded to 128 and rejects new alerts rather than evicting an actionable old target. Account changes/disposal invalidate navigation callbacks; old alerts may still foreground the current app. Native waits remain bounded until terminal resolution. OS permission/sound, visible delivery, history retention, compositor focus and branded installed identity require their own platform evidence. No actual permission receipt, uniform withdrawal, reload/cold restoration or closed-app push is promised.

Historical validation and failure records (earlier snapshots; not current status or scope)

The sections below preserve their original snapshot reports, including then-current pending checks and platform limits. They are historical, superseded by the delivery/status/acceptance sections above. Run 34767821040 subsequently passed at 8cabf9e; it did not include the native-click bridge.

JavaScript failure correction — 2026-09-13

Current pushed head: 8cabf9ef39b6c8127d64b9fd4489b0b310191405. Not merge-ready: new hosted CI is pending, and required human/code-owner approval remains outstanding. DCO passed at this head.

  • Previous JavaScript job failed a notification assertion, not lint/types/build or a timeout: the kind-40002 “future” fixture generated an alert. All other lanes in that run passed.
  • The fixture used floor(Date.now()/1000)+31 while the production policy rejects only over 30 seconds ahead. Crossing the next second between fixture creation and admission can make it legitimately eligible—even after just 1ms. A controlled +1ms crossing on CI merge b773762 reproduced the identical assertion for both message kinds, with the other 1,207 tests passing. The original CI log does not record clock samples; this proves the test race and matching failure mechanism, not the runner's precise timing.
  • Test-only correction: pin Date.now in the existing case and add eight real-session/binding boundary cases (both kinds, inclusive +30s / 120s and 1ms outside). Production code, real timers, timeouts and CI configuration are unchanged. Brain independently reviewed the correction and diagnostic evidence; no source-review blocker.
  • Merge-snapshot validation on macOS arm64: b773762 plus the exact test patch passed CI=true pnpm check, pnpm exec vite build, and all 1,217 Vitest tests with --maxWorkers=2. The first default-concurrency suite passed 1,216 tests but timed out the unrelated app-composition test at 5 seconds; retain this failure, do not describe the default lane as passing. That case passed in the prior hosted job. Cause of local timeout remains unproven.
  • Actual branch head 8cabf9e: the unmodified pre-push hook passed TypeScript and all 1,203 Vitest tests, without worker override. Remote and local heads match. The 14-test difference is newer main's tests in the merge snapshot, not skipped coverage. Earlier browser/native results below remain attributed to their original snapshots, not this test-only head.

This is a missed deterministic-test requirement. The same class had already been repaired in the observer fixture; successful local runs did not justify overlooking the equivalent clock dependency in notification tests. No approval or merge performed.

Earlier simplification — 2026-09-13

Pushed head: bcb5aae03b7edb1d70d099afeddf180157f4109b (follow-up diff). Local and hosted heads match.

  • Removed the separate message-detail reader and UI. Loaded top-level exact targets reveal in ChannelTimeline; replies and off-window targets reuse ThreadPanel and the session-owned thread reader. Each navigation attempt completes after its actual row is visible and focused; reclick/Back gets a fresh attempt. Exact lookup never supplies a history cursor or seeds ordinary channel history.
  • Preserve ordinary read dwell, cancellation/access authority, thread drafts, and reading/focus through background repair. A deleted selected reply leaves valid root/siblings/composer available. Retain known tombstones after cache eviction; stage finite edit overlays until delete-of-edit closure; admit selected content through the existing shared access gate before retention.
  • The replacement is 177 fewer non-test production source lines than the prior pushed head. The follow-up adds targeted regressions rather than a second navigation subsystem. Original kind-40002/read-marker fixes remain included; Carl's published re-review cleared that earlier follow-up at 0e34478 (see review history for exact evidence).

Validation at clean bcb5aae on macOS arm64: all batch stages completed separately, not a successful monolithic just scan:

  • 1,195 Vitest tests / 119 files; 35 Node integration tests.
  • 326 application-browser checks: 159 Chromium + 159 WebKit functional journeys and 8 serial opening/scroll measurements, including local-only WebKit cases; no browser retries.
  • 14 design-browser checks, frozen install, Biome, app/design types and design rules, frontend/design builds, workspace Rust formatting/Clippy, 16 plugin-manager Rust tests and 9 native Rust tests.
  • Unmodified pre-push hook then passed TypeScript and all 1,195 Vitest tests at the same head. All 10 PR commits retain verified Pinky authorship/DCO; existing Brain co-author credit remains intact.

Failures retained: the earlier e16865e scan hit the 20-minute tool limit after a WebKit test asserted on an offscreen virtual live row; the test was corrected and the later full browser lanes passed. At bcb5aae, an unrestricted Vitest batch hit the app-composition 5-second and read-state-growth 15-second limits (1,193 passed, 2 timed out). The unchanged full suite passed with two workers, then again in the normal hook without that override. No timeout inflation, source change, skipped test or hook bypass; the original timeout cause remains unproven. Non-failing build advisories remain.

Brain's independent source-review sequence reports no outstanding source blockers at bcb5aae. That is AI source review, not independent runtime certification or human/code-owner approval. DCO Check passed; hosted CI subsequently failed JavaScript (see correction above). Required human/code-owner review remains outstanding; no approval or merge performed.

This branch still integrates main through c9810f3, not the three newer main commits through 61274d4. A read-only merge-tree check found no textual conflict, but is not runtime integration validation. GitHub reports mergeable; the hosted gate remains authoritative for its tested state. Native/platform acceptance limits below are unchanged.

Earlier review comments addressed — 2026-09-13

Earlier head: 0e34478ae39621dd33cafcbc95d8648b5f2b6641 (follow-up diff).

Both defects in Carl's review are fixed:

  • Kind 40002 now shares kind 9's live provenance, access, peer-author and dedup guards. Its structured content is decoded using the same body rule as message rows, before bounding the arrival preview.
  • The app-global notification binding starts the existing shared unread observation after roster readiness, even when Channels is not mounted. Remote-capable hosts retain fresh candidates until bounded/snapshot marker observation has decoded and merged; loading/error/unknown readiness stays quiet. Unsupported hosts retain local-only behavior. No new socket, unread engine, persistence format or native machinery.

Verification at clean 0e34478: full just scan passed on macOS arm64, 2026-09-13 13:51–14:10 UTC: 35 Node integration tests; 1,190 Vitest tests / 119 files; 316 Chromium/WebKit application checks, including serial measurements and notification journeys; 14 design-browser checks; 16 plugin-manager Rust tests; 9 native Rust tests; frozen install, lint/types/design rules, frontend/design builds, formatting and workspace Clippy. No retry or hook bypass. Normal pre-push then passed TypeScript and all 1,190 Vitest tests. Local and remote heads match.

The notification owner suite now has 33 cases (17 new): both content kinds, decoded/bounded/empty/plain-text previews, delayed remote response and decode/merge, already-read suppression versus genuinely unread release, bounded/snapshot hosts with/without a modeled Channels consumer, failure/cancellation/switch/disposal, and deferred roster discovery. Sixteen new cases were first observed failing on unchanged pre-fix production code; the final roster-ordering regression was added during self-review. The host decoder is modeled in these owner tests; they do not establish deployed encryption or OS delivery.

Brain independently reviewed the four-file follow-up and its owning call paths; no source blockers at 0e34478. This is AI source review, not human/code-owner approval. DCO Check passed at that head; hosted CI was pending at that handoff (historical, not current status). GitHub now shows the PR as non-draft; no approval or merge was performed. Existing native/platform acceptance limits below remain unchanged.

Earlier verification history

Integrated main: c9810f3293266b5741ce16c08aa52710847d5e77.
PR head: 5bf72fbcd52ae2baa51477b2fe7f2dc4b288497c.

Full just scan passed at clean 7fcbe6c01a4211c53e46391988c3a701a21f25df on macOS arm64, 2026-09-12 17:33–17:51 UTC:

  • 35 Node integration tests; 1,173 Vitest tests / 119 files.
  • 316 Chromium/WebKit checks, including serial channel-opening/scroll measurements and notification/exact-message journeys; no retries.
  • 16 plugin-manager Rust tests; 9 native Rust tests; 14 design-browser checks.
  • Frozen-lock install, lint/format/app/design types, design rules, frontend/design builds and workspace Clippy.

The final commit differs from that scanned snapshot only in dev/agent-observer.test.mjs: freeze its clock during freshness rejection assertions, then restore it on test completion. Production source and all browser/native inputs are unchanged. At final head, pnpm check and the normal push hook passed TypeScript plus all 1,173 tests / 119 files. Local and remote heads match. All five PR commits retain Pinky's verified authorship/DCO; Brain's material exact-opening contribution retains co-author credit.

Brain's independent source review found no actionable blockers in the integration and final two-test delta. This is AI source review, not human/code-owner approval or independent test execution.

Failures retained, not hidden

  • The initial scan at 8f57e01 timed out in the inherited signer/shared-cooldown test. An unchanged full Vitest rerun and the later full scan passed; its original timeout cause remains unproven. No auth change or timeout inflation.
  • The first browser run passed 314/316; both failures were a stale Settings Tab sequence. The test now explicitly traverses Notifications between Appearance and Display name, preserving keyboard/viewport coverage. All six focused Settings cases and the final full browser suite passed.
  • The first push hook exposed the inherited observer test's live-clock boundary. A controlled probe demonstrated that its future +301s fixture becomes valid at +300s one second later. The aggregated original failure did not identify its individual fixture, but freezing this test's clock removes that concrete race without weakening rejection policy. The subsequent normal hook passed; no bypass.

Origin

buzz://message?channel=c2bfe438-e8e7-4dc1-8b58-f3898136ba90&id=6d76a020c5b4c14fbc0eed368ea8f2d6b110c18795a867719eebc8301875a30b

Pinky and others added 5 commits September 12, 2026 10:53
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Use existing live/unread policy for account-local notification settings,
plugin categories, and sender/conversation previews. Deliver desktop banners
through the official Tauri plugin; retain verified browser exact-message
opening without a custom native click or delivery subsystem.

Co-authored-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Compose observer isolation and membership traffic with replay/live provenance. Preserve current image-scroll behavior and both branches’ regression coverage.

Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
@wesbillman
wesbillman marked this pull request as ready for review September 12, 2026 21:40

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes required: two P2 notification eligibility defects

Reviewed head 5bf72fbcd52ae2baa51477b2fe7f2dc4b288497c against base c9810f3293266b5741ce16c08aa52710847d5e77.

P2: Include supported kind-40002 messages in live notification admission

session.ts:958–970

The new candidate filter accepts only event.kind === 9, but the existing live route, message fold, unread/attention engine, and exact-message reader support both 9 and 40002 (live.ts:116, fold.ts:148–150, unread.ts:61–62,207–243, message-detail.ts:64). A verified, fresh kind-40002 post from another client with the viewer's p tag appears as an unread mention but never reaches subscribeIncoming, so the built-in notification adapter cannot alert. Participating-thread replies have the same exclusion. The documented built-in mention/thread contract does not exclude this supported content kind.

Exit criterion: admit both supported content kinds through the same provenance/access/dedup guards; preserve the decoded message-body preview for structured 40002 content. Cover a live 40002 mention/reply, while keeping replay, own messages, and auxiliary events quiet.

P2: Wait for initial remote read-state observation on capable hosts

messages.ts:128–134

The readiness gate checks status === "loading", but that status ends when the local journal loads (read-state.ts:161–171). The first remote marker request does not restore loading while it awaits its response (read-state.ts:284–336). Thus a remote-capable session can be local / completeness: "unknown" with a marker request still in flight. A fresh live mention already covered by another device's read frontier then passes the gate as unread and can display before that frontier arrives. This also occurs with Channels mounted and unread.ensure() already running; it is not limited to a Home-only session. Outside Channels, this binding only subscribes and does not initiate that observation at all.

A focused probe using the unchanged read-state, unread, and notification-binding owners confirmed the gate returns true while the initial remote response is delayed, then false after the covering frontier is ingested. The existing startup test delays local storage, not remote markers. This contradicts “Unknown read readiness waits” in docs/notifications.md:29–31; it is an avoidable startup already-read alert, not a request for durable cross-device exactly-once delivery.

Exit criterion: own bounded initial marker observation independently of the Channels page, and retain/revalidate fresh candidates while that initial observation is unresolved on capable hosts. Preserve local-only behavior for capability: "unsupported" and bounded-observation hosts; do not require an unavailable complete snapshot. Add a delayed-remote-marker regression showing that an already-read candidate stays quiet and a genuinely unread candidate is reconsidered after readiness.

Validation and scope

All three independent source-review lanes returned and were integrated. Hosted CI is green at the reviewed head. I ran focused policy probes only, not broad suites or app/service launches; the source tree remained clean. The probes begin after wire verification and do not establish actual browser/native banner delivery. Navigation/cancellation/reading dwell, platform boundaries, and normal preferences/plugin lifetimes had no additional actionable findings. Packaged OS identity, real permission/sound behavior, and native asynchronous delivery remain outside this source review's acceptance evidence, as documented.

GitHub state limitation: GitHub rejected REQUEST_CHANGES because the authenticated account is also the PR author. This COMMENT review records the changes-required verdict and findings; it is not an approval or a successful blocking GitHub review.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

CHANGES REQUESTED

GitHub rejected a self-authored REQUEST_CHANGES review, so this is published as COMMENTED. The code-review verdict remains blocking, not an approval.

Reviewed head 5bf72fbcd52ae2baa51477b2fe7f2dc4b288497c against base c9810f3293266b5741ce16c08aa52710847d5e77. Two P2 integration defects need correction before this is clear:

1. Include supported kind-40002 content in live notification candidates

src/features/relay/session.ts:963 admits only kind 9. A verified peer-authored kind 40002 message received after channel EOSE, with one matching h and a viewer mention (or eligible DM/thread relationship), is reconciled into message/unread state but never emitted to notification consumers. The existing live route, unread engine and message projection already support 9 and 40002 (live.ts:116, unread.ts:61-62, fold.ts:148-150); the notification contract does not exclude the latter. These supported messages silently lose alerts.

Admit both supported content kinds and normalize the kind-40002 envelope's text before the bounded preview, rather than showing its JSON wrapper. Cover live eligibility plus replay/duplicate suppression and envelope handling for kind 40002.

2. Await initial remote read-state observation before releasing alerts

src/features/notifications/messages.ts:128-134 treats any sync state other than loading/error as ready. But read-state.ts:161-171 changes status to local as soon as the local journal loads; refresh():284-336 leaves that status while awaiting remote markers. The app-global notification binding also never initiates unread.ensure(); the only production caller found in src/ is ChannelsPage.tsx:145, so starting on another page need not initiate that observation at all.

On a remote-read-state-capable host with an empty/stale local journal, hold the initial remote marker response/merge and deliver a fresh, post-EOSE eligible message already covered by another device's marker. attention.unread is temporarily true and the banner is emitted before reconciliation can suppress it. This violates the explicit “Unknown read readiness waits” contract; the current readiness test blocks local storage only, not remote marker observation.

Have the notification consumer establish and await the shared initial read-state observation, retaining fresh candidates until that initial attempt is resolved and merged, with failure/cancellation still suppressing delivery. Preserve local-only behavior for unsupported hosts; do not require complete message history or add a second unread engine. Add a delayed-remote-marker regression both with Channels mounted and when starting on another page: an already-read message stays quiet, while a genuinely unread fresh candidate can proceed after readiness.

Evidence and scope

Source/metadata review only on Wes’s Mac Studio (BLKD2G9MWNRWY.local), reading the pinned Git objects; no checkout, builds, tests, imports or PR-code execution. Existing exact-head GitHub Actions run 34709672297 passed all eight jobs. CI success does not exercise the two missing contracts above.

The accepted design remains one host-owned notification policy and scoped browser exact-message navigation with ordinary focused dwell, not click-as-read. Stock Tauri permission, delivery/withdrawal, sound and click-navigation limitations are intentional and are not blockers. Real packaged OS banner behavior was not validated by this review.

Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Pinky is commenting on Wes's behalf.

Both review findings are addressed in 0e34478ae39621dd33cafcbc95d8648b5f2b6641:

  1. Kind 40002 admission and preview: the existing live candidate filter now admits both supported message kinds through identical provenance/access/dedup guards. Structured bodies are decoded before the bounded plain-text preview.
  2. Initial remote marker readiness: the app-global binding starts the shared unread observation after roster discovery, independent of Channels. Remote-capable hosts wait until bounded/snapshot markers are decoded and merged; retained fresh candidates are then revalidated. Local-only hosts remain supported, while failed/cancelled observation cannot release alerts.

The notification integration suite now has 33 tests (17 new), including delayed response/merge with and without the Channels consumer, already-read suppression versus unread release, lifecycle/failure cases, roster ordering, and kind-40002 mention/DM/thread previews. The initial regression run failed on pre-fix production code as expected.

Full just scan passed at the exact clean new head: 1,190 Vitest tests, 35 Node integrations, 316 application-browser + 14 design-browser checks, Rust tests, builds/types/lint/Clippy. Normal pre-push passed without bypass. Brain's independent source review found no blockers. DCO is green; new-head hosted CI is still running. No approval/merge or new OS-banner acceptance claim.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

REVIEW CLEAR: both prior blockers resolved

Reviewed head 0e34478ae39621dd33cafcbc95d8648b5f2b6641 against exact base c9810f3293266b5741ce16c08aa52710847d5e77, focusing on the four-file follow-up from previously reviewed 5bf72fbcd52ae2baa51477b2fe7f2dc4b288497c. No actionable blockers found in this re-review.

  • Supported content kinds: kind 40002 now shares kind 9’s live provenance, peer-author, channel-access and dedup guards. Its structured content uses the message-row body decoder before the 4,096-character arrival-preview bound. The regression source covers both kinds, replay/history/own-message suppression, DM/thread categories, and decoded/empty/long/plain-text previews. Admission and preview.
  • Initial remote read-state: the app-global binding initiates the existing shared observation after roster readiness, independently of Channels. Capable hosts wait while completeness is unknown; read-state.refresh() only publishes bounded/snapshot completeness after decode and durable merge. Pending candidates are revalidated, so a covering frontier suppresses an alert while a genuinely unread fresh candidate can proceed. Unsupported hosts retain local-only readiness. Binding, merge boundary, regressions.

Both independent lanes returned clear and were checked against the responsible owners. The production trace included shared observation/coalescing, roster startup, local/remote readiness, pending-candidate lifetime, permission-await rechecks, access loss, session selection/disposal, and reconnect invalidation. It introduces neither a second unread engine nor notification-generated read intent. The accepted unchanged platform/navigation design was not reopened: stock desktop click/permission/sound/delivery limits and packaged OS validation remain documented non-goals/acceptance gaps, not new blockers.

Validation limits: source/metadata-only review on Wes’s Mac Studio (BLKD2G9MWNRWY.local) using pinned Git objects. No checkout, builds, tests, imports, or execution of PR code. Remote git diff --check passed for the follow-up; the unrelated existing checkout remained clean. Regression tests were inspected, not run. Their host decoder and Channels consumer are modeled; they do not certify deployed encryption, actual React mounting, or OS delivery. The required CI-tool preflight failed, so current hosted CI and the PR’s reported full-scan results were not independently certified by this review. This is an automated COMMENTED review, not approval, merge authorization, or release certification.

Pinky added 4 commits September 13, 2026 08:45
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Pinky is commenting on Wes's behalf.

Pushed bcb5aae03b7edb1d70d099afeddf180157f4109b. The requested simplification removes the separate message-detail reader/UI: browser exact-message navigation now reuses the channel timeline or existing thread panel (177 fewer non-test production source lines versus 0e34478).

The original kind-40002 and initial remote read-marker fixes remain included. Follow-up regressions cover repeat/Back reveal, virtual mounting, cancellation/session replacement, ordinary dwell, retained tombstones, repair preserving reading/composer focus, selected deletion preserving valid context, atomic edit/tombstone closure, and shared admission rejecting a dual-tag denied target.

At the exact clean head, split batch validation passed 1,195 Vitest tests, 35 Node integrations, 326 Chromium/WebKit application checks, 14 design-browser checks, builds/types/lint, Rust formatting/Clippy, 16 plugin-manager and 9 native Rust tests. The unmodified pre-push hook also passed TypeScript and all 1,195 Vitest tests. Two earlier same-head Vitest cases timed out; the unchanged full suite passed with two workers and again under the normal hook. No limits changed or checks bypassed; the initial timeout cause remains unproven. The earlier interrupted scan is not counted as passed.

Brain's source-review sequence has no outstanding blockers at this head. DCO is green; hosted CI is running. Human/code-owner approval and merge remain outstanding. Existing native click/OS-banner/package acceptance limits are unchanged. The PR body now records the exact evidence and failure history.

Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Pinky is commenting on Wes's behalf.

JavaScript failure correction — 2026-09-13

Current pushed head: 8cabf9ef39b6c8127d64b9fd4489b0b310191405. Not merge-ready: new hosted CI is pending, and required human/code-owner approval remains outstanding. DCO passed at this head.

  • Previous JavaScript job failed a notification assertion, not lint/types/build or a timeout: the kind-40002 “future” fixture generated an alert. All other lanes in that run passed.
  • The fixture used floor(Date.now()/1000)+31 while the production policy rejects only over 30 seconds ahead. Crossing the next second between fixture creation and admission can make it legitimately eligible—even after just 1ms. A controlled +1ms crossing on CI merge b773762 reproduced the identical assertion for both message kinds, with the other 1,207 tests passing. The original CI log does not record clock samples; this proves the test race and matching failure mechanism, not the runner's precise timing.
  • Test-only correction: pin Date.now in the existing case and add eight real-session/binding boundary cases (both kinds, inclusive +30s / 120s and 1ms outside). Production code, real timers, timeouts and CI configuration are unchanged. Brain independently reviewed the correction and diagnostic evidence; no source-review blocker.
  • Merge-snapshot validation on macOS arm64: b773762 plus the exact test patch passed CI=true pnpm check, pnpm exec vite build, and all 1,217 Vitest tests with --maxWorkers=2. The first default-concurrency suite passed 1,216 tests but timed out the unrelated app-composition test at 5 seconds; retain this failure, do not describe the default lane as passing. That case passed in the prior hosted job. Cause of local timeout remains unproven.
  • Actual branch head 8cabf9e: the unmodified pre-push hook passed TypeScript and all 1,203 Vitest tests, without worker override. Remote and local heads match. The 14-test difference is newer main's tests in the merge snapshot, not skipped coverage. Earlier browser/native results below remain attributed to their original snapshots, not this test-only head.

This is a missed deterministic-test requirement. The same class had already been repaired in the observer fixture; successful local runs did not justify overlooking the equivalent clock dependency in notification tests. No approval or merge performed.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear: no actionable blockers found

Reviewed head 8cabf9ef39b6c8127d64b9fd4489b0b310191405 against base c9810f3293266b5741ce16c08aa52710847d5e77, focusing on the follow-up since cleared head 0e34478ae39621dd33cafcbc95d8648b5f2b6641.

  • Exact-message navigation now reuses the ordinary timeline/thread owner. The reviewed paths preserve target-visible-and-focused completion, reclick/Back behavior, cancellation/session/access fences, bounded context and tombstone retention, repair/draft stability, and ordinary dwell rather than click-as-read. Exact target IDs do not become channel-history cursors.
  • The earlier live-message admission and remote read-readiness fixes remain intact. Immutable selected content retains the existing shared observation policy; incomplete finite overlays are staged until closure. The deterministic notification tests cover both message kinds at the exact freshness boundaries.
  • Hosted CI run 34767821040 succeeded at this exact head, including all eight workflow jobs; DCO, Semgrep OSS and zizmor also passed. This was Studio-only source/metadata review with independent storage, session and UI lanes. Reviewers did not execute PR code or rerun tests/CI. Real OS permission, banner delivery and native packaging behavior were not revalidated; the documented stock-desktop limitations remain accepted non-goals.

This is a review comment, not GitHub approval or a claim of native release acceptance.

Pinky added 4 commits September 13, 2026 10:35
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
@wesbillman
wesbillman marked this pull request as draft September 13, 2026 17:05
@wesbillman
wesbillman marked this pull request as ready for review September 13, 2026 17:19

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes required

GitHub rejected self-requested changes because this account also authored the PR. This is an explicitly blocking COMMENTED review, not approval.

Reviewed head 7298d95ba35c72b1bf576c5dbc8a8783baff8d55 against base a67102aa1201adfa47a03be7d668a62ac748c152, concentrating on the native-click delta since cleared head 8cabf9ef39b6c8127d64b9fd4489b0b310191405. Running native click → foreground Buzz → exact message/thread is now required on macOS, Windows and Linux; the older banner-only limitation is superseded.

P2: Register the Linux native signal receiver before displaying the notification

At src-tauri/src/notifications.rs:208–228, show_async().await completes before wait_for_action_async() starts. The pre-registered Tauri Channel protects the Rust → JavaScript hop, but does not register the Linux daemon → Rust listener.

This is a concrete gap in the locked dependencies, not merely missing manual acceptance:

  • notify-rust 4.18.0/src/xdg/zbus_rs.rs:171–185 creates a fresh connection and calls Notify before returning the handle. Only the later wait installs the ActionInvoked and NotificationClosed match rules (:236–259) and constructs MessageStream (:261).
  • zbus 5.19.0/src/connection/mod.rs:1086–1102 creates its generic receiver inactive with set_await_active(false). src/message_stream.rs:235–253 activates a receiver only at stream construction; src/connection/socket_reader.rs:93–107 drops the no-active-receiver case. Therefore even destination-addressed signals are not retained for the later wait. Broadcast signals before AddMatch are also unavailable.

Failure sequence: display a notification and click or dismiss it while the Notify response or the subsequent listener setup is delayed. Its action/close signals arrive before the stream exists. A lost action does not foreground or navigate. If the terminal close signal is also lost, the wait has no remaining completion event and retains its native/frontend reservation until connection/process teardown. This violates the explicitly required immediate-click contract; keeping the connection alive does not fix it.

Fix/exit criterion: establish a live, buffering native action/close receiver on the same connection before calling Notify, then correlate the returned notification ID. Use a narrowly scoped backend correction/integration, not a delay or arbitrary expiry. Add a regression at the real DBus/backend boundary that emits activation and dismissal during Notify/listener setup, asserting the correct single activation or close and capacity release. The JS immediate-callback tests cannot catch this ordering.

Validation and remaining acceptance

Source/metadata-only review on Wes’s Studio (BLKD2G9MWNRWY.local) using pinned Git objects and lock-checksum-verified dependency archives. No PR code, builds, tests, native notifications or GUI were executed. git diff --check passed against the stated base. Existing exact-navigation and read-readiness contracts were reconciled with the delta rather than reopened. Mongo independently confirmed the Linux race; Mordecai’s frontend lifecycle/IPC lane found no additional defect. Both lanes returned before this consolidated publication.

CI run 34770449097 succeeded at this exact head: JavaScript, Linux/Rust/tool integration, Windows native, browser measurements, all four browser journey shards and the required aggregate; DCO/Semgrep/zizmor also passed. This clears the prior compile/gate failures, not the daemon-listener race.

Real native acceptance remains open on each OS: distinct message/thread targets, background/minimized focus, immediate click, fade → notification-center click, dismiss-without-navigation, stale-account and revoked-access rejection. In particular Windows timeout → later activation and Linux compositor foregrounding are not certified by compilation or mocked IPC. No approval or merge authorization is granted.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes required: one P2 on Linux

Reviewed head 7298d95ba35c72b1bf576c5dbc8a8783baff8d55 against base a67102aa1201adfa47a03be7d668a62ac748c152. This automated pass confirms the existing exact-head finding, not a new defect. GitHub already rejected self-requested changes for this account/PR; this COMMENTED review is explicitly blocking, not approval.

The current contract is running native click → foreground Buzz → exact message/thread on macOS, Windows and Linux. The older banner-only limitation is superseded; existing navigation/read-readiness behavior is not reopened.

P2: Install the Linux action/close receiver before showing the notification

src-tauri/src/notifications.rs:208–228 awaits show_async() before starting wait_for_action_async(). The pre-registered Tauri Channel protects Rust → JavaScript, not daemon → Rust.

In locked notify-rust 4.18.0, src/xdg/zbus_rs.rs:171–185 creates a fresh connection and sends Notify; only the later wait adds ActionInvoked/NotificationClosed match rules (:236–259) and constructs MessageStream (:261). Locked zbus 5.19.0 starts its generic receiver inactive (connection/mod.rs:1086–1102), activates it at stream construction (message_stream.rs:235–253), and discards signals with no active receiver (connection/socket_reader.rs:93–107). Holding the connection does not buffer them.

Failure sequence: click or dismiss the displayed notification while its Notify response or subsequent listener setup is delayed. An early action is lost, so Buzz does not foreground/navigate. If the terminal close is also lost, the wait retains its reservation until connection/process teardown; repeated losses can exhaust the 128-slot cap. Pending::drop cannot release a slot while the live wait still owns it.

Exit criterion: establish a live, buffering action/close receiver on the same connection before Notify, then correlate the returned notification ID. Keep the correction at the backend boundary. A timeout alone does not repair missed activation and must not strand otherwise actionable alerts. Add a real DBus/backend-boundary regression emitting early activation and dismissal, checking once-only correct delivery and capacity release; mocked JS callbacks cannot cover this ordering.

Evidence and limits

Studio-only source/metadata review using pinned Git objects and checksum-verified dependency archives; git diff --check passed. Mongo reconfirmed the native evidence; the completed exact-head frontend lane was reused. All assigned lanes returned. No additional source blocker was found in the reviewed native-click/lifecycle delta.

Existing CI run 34770449097 succeeded at this head, including JavaScript, Linux/Rust/tool integration, Windows native, browser measurements, all four browser shards and the required aggregate. No PR code, tests, builds or native GUI were executed in this automated pass. Real per-OS acceptance remains open: distinct targets, background/minimized foregrounding, immediate click, fade then Notification Center click, dismissal, stale-account and revoked-access rejection. Green CI does not establish those native behaviors.

Pinky added 2 commits September 13, 2026 11:43
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
@wesbillman
wesbillman marked this pull request as draft September 13, 2026 18:22
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Pinky is commenting on Wes’s behalf.

Addressed the Linux listener-order finding in d78a7d9494dbcf43f0af45435a7db8d2aca6f218 (the two latest reviews describe the same defect). The earlier permission-only cleanup is separately committed as 27289d9; it did not itself fix this race.

  • Replaced only the Linux send-then-listen wrapper with the already-locked standard zbus API: one same-connection filtered receiver before Notify, one pinned reply future, concurrent draining, first-terminal ID correlation and existing once-only capacity release. No fork, delay/expiry workaround, navigation changes or other-OS backend changes.
  • Added six Linux-only real-bus tests for pre-reply unicast/broadcast click/close, unrelated IDs, duplicates, >64-signal bursts, overflow, unsupported actions and Notify failure. The overflow fixture holds the reply pending to avoid a scheduler-dependent assertion.
  • Brain’s independent source review is clear after that test correction. Local batch stages passed: 1,227 Vitest; 38 Node integration; 338 application-browser cases; 14 design-browser cases; macOS workspace Clippy/tests. The normal push hook passed at this exact head; DCO Check passed.

Exit criterion still pending: the real D-Bus tests have not run on Linux locally; new CI https://github.com/block/buzz-app/actions/runs/34774442281 is running. Exact copies compile on Mac, which is not Linux runtime evidence. Real OS click/foreground acceptance remains open on macOS, Windows and Linux. PR is draft, with corrected current status and preserved historical failures; no approval or merge claimed.

Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Pinky is commenting on Wes's behalf.

Corrective follow-up to my previous delivery report: 0548649db0a887313264e8ab460e9dca9fefe7cd is pushed; PR remains draft.

I caught a D-Bus wire-type bug after the prior push: the fixed Rust array serialized actions as (ss), not the required as. The one-line Vec correction fixes that without changing policy, timeouts or tests. Brain independently checked the correction.

The previous Linux job confirms the defect at d78a7d9 / CI merge b95f413: Clippy passed, but 5 of 18 native tests failed, with click/close/burst reporting the exact signature mismatch; Node integration was skipped. Those failures remain in the evidence. Windows native, JavaScript and measurements passed at that older head—not proof of corrected-head CI or OS interaction.

Validation of the correction: an executable exact-expression wire probe passes with Vec and fails when mutated back to the old array; exact Linux source/test copies compile and pass Clippy on macOS. At exact pushed 0548649, normal hooks passed TypeScript and all 1,227 Vitest tests / 121 files; all 18 PR commits have author-matching DCO, and hosted DCO passed.

New CI run 34774763484 is running. Passing Linux real-bus execution, real click → foreground → exact target acceptance on all three OSes, and human/code-owner approval remain open. No native GUI launched, approval, or merge. PR body updated to the corrected head and retained failures.

@wesbillman
wesbillman marked this pull request as ready for review September 13, 2026 18:32

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Source review clear: prior Linux P2 resolved

Reviewed head 0548649db0a887313264e8ab460e9dca9fefe7cd against base a67102aa1201adfa47a03be7d668a62ac748c152, focusing on the three follow-up commits since 7298d95ba35c72b1bf576c5dbc8a8783baff8d55. No actionable source defect found in that delta and its integration boundaries. This is a COMMENTED review, not approval or release acceptance.

The single Linux receiver-ordering P2 in review 5191572194, repeated in 5191603119, is cleared at this head:

  • linux.rs:23–101 establishes a live sender/path/interface-filtered receiver on the same connection before Notify. It drains signals alongside one pinned reply future, preserves the first terminal response by returned notification ID, and handles the bounded early-response overflow explicitly. The Vec actions argument has the required D-Bus array shape. I checked the locked zbus/futures-lite source, including active receiver construction and polling order, rather than assuming holding a connection buffers signals.
  • linux/tests.rs:67–270 exercises the production operation through isolated real D-Bus connections. Its fake daemon emits early unicast/broadcast clicks and closes; the tests cover queue pressure, first-terminal behavior, overflow, unsupported actions and Notify failure, asserting a single callback and released capacity. The fixture does not pre-arm a replacement listener. I reviewed these test oracles; I did not execute them in this pass.
  • notifications.rs:23–100 retains once-only completion/capacity release and foreground-before-IPC delivery. The removed notification plugin's JS/Rust dependencies, initialization and permission grants are gone from the checked paths; the custom bridge remains registered. Desktop settings now accurately describe unknown/system-managed permission without ineffective controls, while the browser adapter and existing host policy/account-generation/navigation/read ownership remain unchanged.

Validation and remaining gates

Source-only review on Wes’s Studio using pinned Git objects and Cargo.lock-checksum-verified dependency archives. git diff --check passed for both the full PR and follow-up ranges. No code changes, builds, tests or visible native app launches in this pass. Existing navigation/read-readiness design was reconciled with the delta, not reopened. Mongo independently reviewed Linux subscription/correlation/test oracles; Princess Donut reviewed plugin/permission removal. Both lanes returned before publication and their evidence was reconciled with my own trace.

Non-blocking test follow-up: add an explicit Notify-reply-first, later-click/later-close scenario. Current fixtures emit terminal signals before their reply barrier. They target the reported race well, but do not deliberately establish the ordinary post-reply interaction case. No defect was found in that wait loop; this does not extend the P2 exit criteria.

CI is not green: a fresh GitHub check snapshot at this exact head for run 34774763484 shows JavaScript, Rust/tool integration, Windows native, browser measurements and three browser shards passed. Chromium shard 1/2 failed its Functional journeys step, so CI required failed. DCO, Semgrep and zizmor passed. The specific browser failure is not diagnosed in this review and must be resolved or dispositioned before integration; I am not labeling it unrelated or flaky. These are hosted job results, not locally rerun tests. GitHub reports mergeable but BLOCKED, distinct from a source finding.

Native GUI acceptance remains open on macOS, Linux and Windows: distinct message/thread targets, background/minimized foregrounding, immediate click, fade then notification-center click, dismissal without navigation, and old-account/revoked-access rejection. In particular packaged macOS identity, Linux compositor foregrounding and Windows timeout-to-later-activation are not certified by compilation, mocked IPC or isolated D-Bus tests. Source clearance does not close those release-validation gates.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Source review clear: previous Linux P2 resolved

Reviewed head 0548649db0a887313264e8ab460e9dca9fefe7cd against base a67102aa1201adfa47a03be7d668a62ac748c152. This automatic pass verifies and reuses the existing exact-head source review, rather than reopening settled navigation/read-readiness design. COMMENTED is not approval or release acceptance.

The contract remains running-app alerts on browser/macOS/Linux/Windows, with native click → foreground Buzz → existing exact account/community-scoped navigation. Host policy and ordinary focused-visible dwell remain the sole notification/read owners; closed-app push, cold-start receipt restoration and uniform OS withdrawal are not promised.

  • The prior Linux receiver-ordering P2 is cleared. linux.rs:23–101 establishes the live same-connection receiver before Notify, drains alongside one pinned reply future and retains the first terminal response by returned ID with explicit bounded overflow. The actions Vec has the required D-Bus array signature. I reread the production path, isolated real-D-Bus regression oracles and retained locked dependency source. Tests exercise early unicast/broadcast clicks and closes, queue pressure, once-only completion/capacity release, overflow and failure. No PR code was executed in this review.
  • The cleanup preserves integration ownership. The permission-only plugin is removed from the checked JS/Rust dependency, initialization and capability paths; notification_show registration remains. Desktop settings accurately report unknown/system-managed permission without ineffective controls. The browser adapter and shared admission/navigation/read paths are unchanged by these follow-up commits. Frontend pre-registration, native foreground-before-IPC and account-generation fences remain intact.
  • Acceptance is still separate. The existing nonblocking suggestion for explicit Notify-reply-first/later-action coverage remains follow-up, not a reopened P2. Real GUI acceptance on all three desktop OSes remains open: distinct targets, background/minimized foregrounding, immediate and notification-center clicks, dismissal, stale-account/revoked-access rejection and packaged identity. Source clearance does not certify these workflows.

Studio-only pinned-object source review; full-PR and follow-up git diff --check passed. No checkout, installation, build/test execution or native launch. Prior integrated source lanes were reused; Princess Donut independently inspected the retained failed browser gate, with findings integrated before publication.

Hosted CI: unresolved panel-focus gate

Run 34774763484 is not green. JavaScript, Rust/tool integration, Windows native, measurements and three browser shards passed; Chromium shard 1 and CI required failed. The retained Chromium artifact identifies the failing assertion at the synthetic checkout’s layout.spec.mjs:340 (corresponding to layout.spec.mjs:295–299 at the reviewed head). It waits for the GitHub link to regain focus after closing the channel panel, but it remained inactive for the full 10-second wait.

The layout test and relevant trigger-capture/close-focus bodies are unchanged between the pinned base and head; this journey uses ordinary channel/link navigation, not an exact-message notification target. This does not establish a PR #38 regression, nor prove an unrelated flake. The recorded synthetic checkout is bf8039507de29aff3beff303179a7b4df8703f49, merging this head into 541694c1cad4dccc081702bacc0fccfb34869a45; its tree differs from the reviewed head and its first parent differs from the pinned base. These hosted results therefore describe that integration snapshot, not an exact-head-tree run. Resolve or explicitly disposition the panel-focus failure before integration. I am not adding an unsupported changed-code blocker or claiming merge readiness.

Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No remaining code blockers

Reviewed head 622cc50dcdc635153c8f805c3c858894286d4018 against base a67102aa1201adfa47a03be7d668a62ac748c152, concentrating on the three-file focus repair since the source-cleared 0548649 review. No approval submitted.

  • The Linux P2 remains resolved. The native operation and real-D-Bus tests are byte-identical to the previously reviewed correction. The receiver is armed before Notify and drained alongside its single reply future; the actions argument retains the correct D-Bus array representation. This delta does not change native, permission or notification-policy behavior.
  • The reflow correction belongs at the virtualizer boundary. ChannelTimeline retains one focused row by message ID, maps it to the current grouped-row index after prepend, and releases it on focus exit/history focus. Removed rows do not produce an invalid index; the existing session/scope/channel remount key fences retargeting. Virtua 0.51.0's actual package-root implementation unions these indices with its rendered range using existing child keys. Panel close still owns focus restoration, and offscreen retained rows still fail the reading hook's viewport checks. No second focus/navigation owner is introduced.
  • The regressions exercise the relevant boundary. The browser case waits for a neighboring unpinned row to be evicted, then requires the offscreen trigger to retain focus and its row to be evicted after focus moves to history. The existing panel test now asserts focus after layout settles while retaining its zero-scroll-delta and reading-anchor checks. Unit coverage complements this with ID/index remapping and release/removal transitions.

Validation and limits

Studio-only pinned-source re-review; full-PR and follow-up git diff --check passed. Princess Donut independently reviewed the test/accessibility lane before publication. No code edits, installations, test/build reruns or app launches in this pass.

The three changed files match the hashes in the retained implementation/review evidence. Author logs show both browser engines failing with the production repair removed and all 16 layout cases passing on the prior CI merge plus the repair; those are author-run results, not executions performed by this reviewer or an exact-head-tree full suite. The final hook report records all 1,228 Vitest tests passing at 622cc50.

A fresh hosted status snapshot for this head is all green, including JavaScript, Linux/Rust/tool integration, Windows native, measurements, all Chromium/WebKit shards, required aggregate, DCO and security checks: https://github.com/block/buzz-app/actions/runs/34776824117 . The previously failed Chromium gate is no longer outstanding. This describes hosted PR checks, not a locally rerun suite.

The PR and retained acceptance log record Wes's successful running macOS and Chrome notification → foreground → exact-reply checks. Those human-confirmed happy paths count; this source pass did not independently execute them. Windows/Linux GUI, packaged macOS identity and the documented minimized/notification-center/dismissal/stale-authority edge cases remain unverified. These are disclosed acceptance limits, not new source findings or a request to restart the settled design. Human/code-owner approval remains separate.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear: no remaining code blockers

Reviewed head 622cc50dcdc635153c8f805c3c858894286d4018 against base a67102aa1201adfa47a03be7d668a62ac748c152. This automated pass reuses the existing exact-head review, including its independent test/accessibility lane, and re-verifies the three-file focus repair since 0548649. No approval submitted.

  • Contract preserved: one host notification policy for browser and standard desktop, account-scoped activation through existing navigation, and ordinary visible/focused dwell rather than click-to-mark-read. Cold-start recovery, durable exactly-once delivery, and uniform OS withdrawal remain out of scope.
  • Focus repair verified: ChannelTimeline retains at most one focused row by message ID, remaps its index after prepend, and releases it when focus leaves or moves to the history region. Removed rows yield no invalid pin; session/scope/channel remounting fences retargeting. Panel close retains focus-restoration ownership, and offscreen retained rows remain excluded by the reading hook's geometry checks. The regression proves neighboring-row eviction, continued offscreen focus, and eventual release; the panel assertion now runs after reflow settles. The previously fixed Linux operation and real-D-Bus tests are byte-identical to the source-cleared revision.
  • Validation: strict-verified Studio source review, exact-base contribution/product/browser-testing documents, complete focus and reading owners, and full-PR/follow-up git diff --check passed. A fresh hosted snapshot at this head shows all 12 checks successful, including all Chromium/WebKit shards, Windows native, Rust/tool integration, JavaScript, measurements, aggregate, DCO and security: https://github.com/block/buzz-app/actions/runs/34776824117 . No checkout, code edits, installations, builds, tests or app launches were performed in this pass.

The prior review records Wes's successful running macOS and Chrome notification-to-exact-reply happy paths; those remain human-reported evidence, not executions by this reviewer. Windows/Linux GUI, packaged macOS identity, and minimized/Notification Center/dismissal/stale-authority edge cases remain unverified acceptance limits. Hosted checks do not establish those outcomes. Human/code-owner approval remains separate.

@wesbillman
wesbillman merged commit 1ba1544 into main Sep 13, 2026
12 checks passed
@wesbillman
wesbillman deleted the pinky/notifications-simple branch September 13, 2026 20:23
klopez4212 pushed a commit that referenced this pull request Sep 14, 2026
Resolve conflicts after main gained shared browser + desktop
notifications (#38), which rewrote the thread-target navigation
foundation (threads.ts, session.ts { exact }, use-message-reveal.ts).

- docs/plugin-architecture.md: keep the branch paragraph describing the
  shipped legacy-link recognition and message-target navigation.
- ThreadPanel.tsx (+ test): take main's newer implementation, which the
  merged relay foundation requires; the branch's parallel presented-ref
  navigation is superseded by main's exact-target/useMessageReveal design.
- ChannelsPage.tsx: take main's exactOpening navigation, then re-apply
  the two additive branch pieces onto it: Kenny's rich-link activation
  (isBuzzLink/buzzLinkTarget in openLink) and the two-panels fix
  (retire a navigation-owned thread via select() before activating a
  panel, matching main's own closeThread retirement).

tsc --noEmit clean; biome clean; affected vitest suites pass
(buzz-links, MessageLink, BuzzLinkPreview, ThreadPanel, ChannelsPage).
The 5 virtua-compensation failures are pre-existing and reproduce on
clean origin/main (jsdom does not apply CSS overflow/priority).

Signed-off-by: Mongo <9cfd347903944d5b85aa6c93d2ab67381b978a92a31914bca69998968752a1d7@buzz.block.builderlab.xyz>
delkc added a commit that referenced this pull request Sep 14, 2026
* origin/main:
  fix(messages): prevent native WebKit prepend blanking (#49)
  feat: add shared browser and standard desktop notifications (#38)
  Prevent whole-window scroll bounce (#41)
  test: replace timing races with explicit lifecycle barriers (#45)
  test(browser): isolate fixture optimizer caches (#46)
  test(browser): settle startup catch-up before request baselines (#43)
  feat(messages): show message-carried blurhash before decoded originals (#40)
  Add a shared full-page surface (#29)
  fix(messages): prevent image-driven timeline scroll snap-back (#31)
  Restore grouped channel membership activity (#30)
  feat: add broker-backed Agent Activity panel (#36)

Signed-off-by: Clay Delk <clay.delk@gmail.com>

# Conflicts:
#	src/bundled/channels/ChannelsPage.tsx
#	src/features/messages/ChannelTimeline.tsx
#	src/features/messages/MessageRow.tsx
#	src/features/relay/contracts.ts
#	src/features/relay/fold.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant