From edaec99edc28c2fe08e311748cf4bce9edefdacb Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Sat, 25 Jul 2026 07:31:46 -0700 Subject: [PATCH 01/11] fix(mobile): invalidate DM directory providers at the community boundary (#2842) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### What changed? Made the mobile new-message directory providers (`relayDirectoryUsersProvider`, `relayDirectorySearchProvider`) `autoDispose`, and both now watch `relayConfigProvider` so they refetch when the active relay/community configuration changes. ### Why? Follow-up to #2810 (Codex P1 review flag: "Invalidate the directory when the community changes"). Both providers previously cached results for the whole app session. They watched only the relay-session notifier (a stable instance that survives dependency rebuilds) and the current pubkey, which keeps its value when two communities share a signing key. Switching between such communities could reopen the New message sheet showing the previous relay's people, and submit their pubkeys to the current relay. The search provider was also a non-autoDispose family keyed by raw query strings, so every distinct typed query leaked a cached provider entry for the session. Watching `relayConfigProvider` (which rebuilds on every community switch via `activeCommunityProvider`) invalidates cached browse and search results at the community boundary, and `autoDispose` releases the cache when the sheet closes. ### How is it tested? Full mobile suite green (585 passed / 1 skipped), `flutter analyze` clean. Added tests: - [`channel_management_provider_test.dart`](https://github.com/block/buzz/blob/gated/directory-provider-invalidation/mobile/test/features/channels/channel_management_provider_test.dart) — browse and search refetch on relay-config change with an unchanged session notifier and pubkey; cached search families are released once unlistened. Signed-off-by: npub1kqarnt4re38nuttqnml3mrqp8cnm6wzpywl2kesc2ejasp0luc5q275nkx Co-authored-by: npub1kqarnt4re38nuttqnml3mrqp8cnm6wzpywl2kesc2ejasp0luc5q275nkx --- .../channels/channel_management_provider.dart | 128 ++++++++------- .../channel_management_provider_test.dart | 147 ++++++++++++++++++ 2 files changed, 218 insertions(+), 57 deletions(-) diff --git a/mobile/lib/features/channels/channel_management_provider.dart b/mobile/lib/features/channels/channel_management_provider.dart index eb1f2b429..d88de261c 100644 --- a/mobile/lib/features/channels/channel_management_provider.dart +++ b/mobile/lib/features/channels/channel_management_provider.dart @@ -235,68 +235,80 @@ List directoryUsersFromProfileEvents(List events) { /// This mirrors desktop's empty-query people directory by listing kind:0 /// profiles through the HTTP bridge. The relay membership snapshot remains a /// fallback for older relays that do not support directory listing. -final relayDirectoryUsersProvider = FutureProvider>(( - ref, -) async { - if (mockDmDirectoryEnabled) { - return dmDirectoryPreviewUsers; - } +/// +/// autoDispose so the cached listing is dropped when the New message sheet +/// closes, and the [relayConfigProvider] watch invalidates it at the +/// community boundary. [relaySessionProvider.notifier] is a stable Notifier +/// instance and [currentPubkeyProvider] keeps its value when two communities +/// share a signing key, so neither triggers a refetch on its own. +final relayDirectoryUsersProvider = + FutureProvider.autoDispose>((ref) async { + if (mockDmDirectoryEnabled) { + return dmDirectoryPreviewUsers; + } - final session = ref.watch(relaySessionProvider.notifier); - final currentPubkey = ref.watch(currentPubkeyProvider)?.toLowerCase(); - final directoryEvents = await session.queryRelay([ - const NostrFilter(kinds: [0], limit: 50, extensions: {'page': 1}), - ]); - var users = directoryUsersFromProfileEvents(directoryEvents); - if (users.isNotEmpty) { - return users - .where((user) => user.pubkey.toLowerCase() != currentPubkey) - .toList(); - } + // Rebuild whenever the active relay/community configuration changes. + ref.watch(relayConfigProvider); + final session = ref.watch(relaySessionProvider.notifier); + final currentPubkey = ref.watch(currentPubkeyProvider)?.toLowerCase(); + final directoryEvents = await session.queryRelay([ + const NostrFilter(kinds: [0], limit: 50, extensions: {'page': 1}), + ]); + var users = directoryUsersFromProfileEvents(directoryEvents); + if (users.isNotEmpty) { + return users + .where((user) => user.pubkey.toLowerCase() != currentPubkey) + .toList(); + } - final membershipEvents = await session.fetchHistory( - NostrFilters.relayMembers(), - ); - final memberPubkeys = relayMemberPubkeysFromEvents( - membershipEvents, - ).where((pubkey) => pubkey != currentPubkey).toList(); - if (memberPubkeys.isEmpty) { - return const []; - } + final membershipEvents = await session.fetchHistory( + NostrFilters.relayMembers(), + ); + final memberPubkeys = relayMemberPubkeysFromEvents( + membershipEvents, + ).where((pubkey) => pubkey != currentPubkey).toList(); + if (memberPubkeys.isEmpty) { + return const []; + } - final profileEvents = await session.queryRelay([ - NostrFilters.profilesBatch(memberPubkeys), - ]); - final profilesByPubkey = { - for (final event in profileEvents) - event.pubkey.toLowerCase(): ProfileData.fromEvent(event), - }; - users = - [ - for (final pubkey in memberPubkeys) - if (profilesByPubkey[pubkey] case final profile?) - DirectoryUser( - pubkey: pubkey, - displayName: profile.displayName, - avatarUrl: profile.avatarUrl, - nip05Handle: profile.nip05, - ) - else - DirectoryUser(pubkey: pubkey), - ]..sort((a, b) { - final labelComparison = a.label.toLowerCase().compareTo( - b.label.toLowerCase(), - ); - return labelComparison != 0 - ? labelComparison - : a.pubkey.compareTo(b.pubkey); - }); - return users; -}); + final profileEvents = await session.queryRelay([ + NostrFilters.profilesBatch(memberPubkeys), + ]); + final profilesByPubkey = { + for (final event in profileEvents) + event.pubkey.toLowerCase(): ProfileData.fromEvent(event), + }; + users = + [ + for (final pubkey in memberPubkeys) + if (profilesByPubkey[pubkey] case final profile?) + DirectoryUser( + pubkey: pubkey, + displayName: profile.displayName, + avatarUrl: profile.avatarUrl, + nip05Handle: profile.nip05, + ) + else + DirectoryUser(pubkey: pubkey), + ]..sort((a, b) { + final labelComparison = a.label.toLowerCase().compareTo( + b.label.toLowerCase(), + ); + return labelComparison != 0 + ? labelComparison + : a.pubkey.compareTo(b.pubkey); + }); + return users; + }); /// Prefix-searches the active relay's kind:0 people directory. -final relayDirectorySearchProvider = - FutureProvider.family, String>((ref, query) async { +/// +/// autoDispose family: each distinct query would otherwise cache a provider +/// instance for the whole session (the mention search provider is autoDispose +/// for the same reason). The [relayConfigProvider] watch invalidates cached +/// results at the community boundary. +final relayDirectorySearchProvider = FutureProvider.autoDispose + .family, String>((ref, query) async { final trimmed = query.trim(); if (mockDmDirectoryEnabled) { final normalizedQuery = trimmed.toLowerCase(); @@ -312,6 +324,8 @@ final relayDirectorySearchProvider = return ref.watch(relayDirectoryUsersProvider.future); } + // Rebuild whenever the active relay/community configuration changes. + ref.watch(relayConfigProvider); final session = ref.watch(relaySessionProvider.notifier); final currentPubkey = ref.watch(currentPubkeyProvider)?.toLowerCase(); final events = await session.queryRelay([ diff --git a/mobile/test/features/channels/channel_management_provider_test.dart b/mobile/test/features/channels/channel_management_provider_test.dart index 7bd0429a8..4b31de18e 100644 --- a/mobile/test/features/channels/channel_management_provider_test.dart +++ b/mobile/test/features/channels/channel_management_provider_test.dart @@ -1,4 +1,5 @@ import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:buzz/features/channels/channel_management_provider.dart'; import 'package:buzz/shared/relay/relay.dart'; @@ -198,4 +199,150 @@ void main() { ]); }); }); + + group('directory providers relay-config invalidation', () { + NostrEvent profile(String pubkey, String name) => NostrEvent( + id: '$pubkey-profile', + pubkey: pubkey, + createdAt: 1700000000, + kind: 0, + tags: const [], + content: '{"display_name":"$name"}', + sig: 'sig', + ); + + ProviderContainer buildContainer(_DirectoryFakeRelaySession session) { + return ProviderContainer( + retry: (_, _) => null, + overrides: [ + relaySessionProvider.overrideWith(() => session), + myPubkeyProvider.overrideWithValue('me'), + ], + ); + } + + test('browse directory refetches when the relay config changes', () async { + final session = _DirectoryFakeRelaySession( + profileEvents: [profile('alice', 'Alice')], + ); + final container = buildContainer(session); + addTearDown(container.dispose); + + // Keep the autoDispose provider alive across the config change, the + // way an open New message sheet would. + final subscription = container.listen( + relayDirectoryUsersProvider, + (_, _) {}, + ); + addTearDown(subscription.close); + + final firstUsers = await container.read( + relayDirectoryUsersProvider.future, + ); + expect(firstUsers.map((user) => user.label), ['Alice']); + expect(session.directoryQueryCount, 1); + + // Simulate switching to a community that shares the same signing key: + // session notifier instance and pubkey both survive; only the relay + // config changes. + session.profileEvents = [profile('bob', 'Bob')]; + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'http://other-community.example', nsec: null); + await container.pump(); + + final secondUsers = await container.read( + relayDirectoryUsersProvider.future, + ); + expect(secondUsers.map((user) => user.label), ['Bob']); + expect(session.directoryQueryCount, 2); + }); + + test('search results refetch when the relay config changes', () async { + final session = _DirectoryFakeRelaySession( + profileEvents: [profile('alice', 'Alice')], + ); + final container = buildContainer(session); + addTearDown(container.dispose); + + final subscription = container.listen( + relayDirectorySearchProvider('ali'), + (_, _) {}, + ); + addTearDown(subscription.close); + + final firstResults = await container.read( + relayDirectorySearchProvider('ali').future, + ); + expect(firstResults.map((user) => user.label), ['Alice']); + expect(session.searchQueryCount, 1); + + session.profileEvents = [profile('alina', 'Alina')]; + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'http://other-community.example', nsec: null); + await container.pump(); + + final secondResults = await container.read( + relayDirectorySearchProvider('ali').future, + ); + expect(secondResults.map((user) => user.label), ['Alina']); + expect(session.searchQueryCount, 2); + }); + + test('cached search families are released once unlistened', () async { + final session = _DirectoryFakeRelaySession( + profileEvents: [profile('alice', 'Alice')], + ); + final container = buildContainer(session); + addTearDown(container.dispose); + + final subscription = container.listen( + relayDirectorySearchProvider('ali'), + (_, _) {}, + ); + await container.read(relayDirectorySearchProvider('ali').future); + subscription.close(); + await container.pump(); + + // A fresh read after disposal hits the relay again instead of reusing + // a session-lifetime cache entry. + await container.read(relayDirectorySearchProvider('ali').future); + expect(session.searchQueryCount, 2); + }); + }); +} + +/// Fake [RelaySessionNotifier] that serves canned kind:0 profile events from +/// [queryRelay] and counts directory vs. search queries. +class _DirectoryFakeRelaySession extends RelaySessionNotifier { + _DirectoryFakeRelaySession({required this.profileEvents}); + + List profileEvents; + int directoryQueryCount = 0; + int searchQueryCount = 0; + + @override + SessionState build() => const SessionState(status: SessionStatus.connected); + + @override + Future> queryRelay( + List filters, { + Duration timeout = const Duration(seconds: 8), + }) async { + if (filters.any((filter) => filter.search != null)) { + searchQueryCount++; + } else { + directoryQueryCount++; + } + return profileEvents; + } + + @override + Future> fetchHistory( + NostrFilter filter, { + Duration timeout = const Duration(seconds: 8), + }) async { + return const []; + } } From 5e2e132a4ba0f5268d3689f6068dd61adb3d0168 Mon Sep 17 00:00:00 2001 From: Onno Date: Sat, 25 Jul 2026 17:21:47 +0200 Subject: [PATCH 02/11] fix(docker): create /data/git so the compose volume inherits buzz ownership (#2840) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem A fresh `deploy/compose` install never starts. The relay exits during config validation and crash-loops under `restart: unless-stopped`: ``` Error: Configuration error: invalid config: BUZZ_GIT_PACK_CACHE_PATH=/data/git/.pack-cache could not be created: Permission denied (os error 13) ``` ## Cause The image runs as `buzz:buzz`. Docker seeds a volume's ownership from the image **only when the mount point already exists there** — otherwise it creates the mount point as `root:root`. `compose.yml` mounts `buzz-git-data` at `/data/git`, which the image doesn't create, so the relay can't write the pack cache. `USER buzz:buzz` and `BUZZ_GIT_REPO_PATH=/data/git` arrived in the same commit (426497a), so there was never a working state to regress from. The other deployment paths are unaffected, which is probably why this went unnoticed: the code default (`./repos`, under the `buzz`-owned home) works unmounted, and the Helm chart gets volume ownership from `fsGroup: 65532`. ## Fix Create `/data/git` as `buzz:buzz` in the image, the way the official `postgres` image prepares `$PGDATA`. `compose.yml`, `run.sh` and the volume layout are untouched, so existing deployments need no migration and the `run.sh backup-hint` output stays accurate. ## Testing `ghcr.io/block/buzz@sha256:ec96e029` on arm64 with this change applied as a derived layer, running **unmodified** `deploy/compose/compose.yml` against clean volumes: | Image | `/data/git` owner | Result | |---|---|---| | upstream | `root:root` | `Permission denied` — crash loop | | patched | `buzz:buzz` | relay healthy | ``` relay health: healthy /data/git = buzz:buzz (contains .pack-cache) /_readiness -> 200 WS upgrade -> 101 ``` Relay log shows migrations applied, the A3 git object-store conformance probe passing, and `buzz-relay TCP listening`. I have not run `just ci` / `just test` — this touches no Rust, JS, or mobile code. Happy to run them if you'd like. ## Alternatives - A `git-init` one-shot in `compose.yml` that chowns the volume. Works, but adds a container to every `up` and doesn't help `docker run` or Kubernetes without an `fsGroup`. - Moving `BUZZ_GIT_REPO_PATH` to the code default `/var/lib/buzz/repos` and pre-creating that instead. Arguably tidier, since `/data/git` then only lives in the compose bundle — but it moves the volume mount point, so existing installs would have to relocate their git data. Happy to do it that way if you prefer; it seemed the wrong trade for a bug fix. Signed-off-by: Onno Klein Hofmeijer --- Dockerfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Dockerfile b/Dockerfile index 23904ab9e..661be6c3a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -150,6 +150,9 @@ ENV BUZZ_WEB_DIR=/srv/buzz/web \ # 3000: app (WS + REST) · 8080: /_liveness, /_readiness · 9102: /metrics EXPOSE 3000 8080 9102 +# deploy/compose mounts a volume here; pre-created so it inherits buzz:buzz. +RUN mkdir -p /data/git && chown buzz:buzz /data/git + USER buzz:buzz WORKDIR /var/lib/buzz From 5e3d2e4849c0f2512330801d804fb96f4ab72d28 Mon Sep 17 00:00:00 2001 From: Ayobami Adegoke Date: Sat, 25 Jul 2026 16:28:56 +0100 Subject: [PATCH 03/11] fix(desktop): surface agent rename relay profile sync failure as a warning toast (#2279) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1822 ## Problem Renaming an agent via the agent settings dialog performs a local save (always succeeds) and a best-effort relay kind:0 republish (can fail: network blip, auth expiry, relay unreachable). On sync failure the dialog `console.warn`'d the error and closed as a clean success — the user had no signal that the relay still holds the old name, which breaks `@mention` resolution and shows the stale name in other agents' `From:` lines (discovered via #1743). ## Fix Surface `profileSyncError` as a `toast.warning` in `AgentInstanceEditDialog`, matching the treatment the create and persona-save paths already give the same field (`useManagedAgentActions.ts`, `UserProfilePanelPersonaSubmit.ts`). The save is not blocked — the local rename is valid and persists, per the issue's guidance. The toast points at the retry path that actually works: **restarting the agent**. Re-saving the same name does not retry — `update_managed_agent` computes `name_changed` against the already-updated record, so a second identical save skips the sync — but `start_managed_agent` fires `reconcile_agent_profile`, which queries the relay's kind:0 and republishes when the display name diverges. Scope notes: - `EditRespondToDialog` (the third caller of the update mutation) never changes the name, and the Rust side only sets `profile_sync_error` when the name changed — so no change needed there. - The alternative fix in the issue (retry-with-backoff in `sync_managed_agent_profile`) is not taken here; `reconcile_agent_profile` on agent start already provides self-healing, and this change makes that path discoverable at the moment of failure. ## Testing - `just desktop-check` — biome + file-size + px-text + pubkey-truncation guards clean. - `just desktop-test` — 3331 passed, 0 failed. - The handler branch is a straight conditional on the mutation result; the repo's `.test.mjs` convention covers extracted pure logic, and there is no extracted logic here to unit-test (consistent with the equivalent toast branches on the create/persona paths). --------- Signed-off-by: ayobamiseun Signed-off-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta Co-authored-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta --- .../features/agents/ui/AgentInstanceEditDialog.tsx | 6 +++--- .../src/features/agents/ui/agentProfileSyncWarning.ts | 11 +++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 desktop/src/features/agents/ui/agentProfileSyncWarning.ts diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 5cd5c7a01..86a70c3db 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -1,7 +1,6 @@ import * as React from "react"; import { ChevronDown } from "lucide-react"; import { AnimatePresence, motion, useReducedMotion } from "motion/react"; - import { toast } from "sonner"; import { @@ -83,6 +82,7 @@ import { AgentDefaultsDialog } from "./AgentDefaultsDialog"; import { useProviderApiKeyFieldState } from "./providerApiKeyFieldState"; import { resolveModelFieldStatusMessage } from "./agentConfigControls"; import { AdvancedRequiredBadge } from "./AdvancedRequiredBadge"; +import { showAgentProfileSyncWarning } from "./agentProfileSyncWarning"; const ADVANCED_FIELDS_MOTION_TRANSITION = { duration: 0.18, @@ -101,8 +101,7 @@ export function AgentInstanceEditDialog({ /** Optional field to scroll/focus when the dialog opens from a card deep-link. */ initialFocus?: EditAgentFocusTarget; open: boolean; - /** Present only when the linked definition is editable (non-built-in, - * resolved). Caller closes this dialog and enters definition-edit. */ + /** Present only when the linked definition is editable (non-built-in, resolved). Caller closes this dialog and enters definition-edit. */ onEditLinkedPersona?: () => void; onOpenChange: (open: boolean) => void; onUpdated?: (agent: ManagedAgent) => void; @@ -706,6 +705,7 @@ export function AgentInstanceEditDialog({ autoRestartOnConfigChange, ); } + showAgentProfileSyncWarning(result.agent.name, result.profileSyncError); handleOpenChange(false); onUpdated?.(result.agent); // The auto-restart policy deliberately never fires for a stopped or diff --git a/desktop/src/features/agents/ui/agentProfileSyncWarning.ts b/desktop/src/features/agents/ui/agentProfileSyncWarning.ts new file mode 100644 index 000000000..91be216fd --- /dev/null +++ b/desktop/src/features/agents/ui/agentProfileSyncWarning.ts @@ -0,0 +1,11 @@ +import { toast } from "sonner"; + +export function showAgentProfileSyncWarning( + agentName: string, + profileSyncError: string | null, +) { + if (!profileSyncError) return; + toast.warning( + `${agentName} was saved, but relay profile sync failed: ${profileSyncError}. The relay may still show the old name — restart the agent to retry the sync.`, + ); +} From a64cc71f6c1605279b1a6fbd0fe904a2984cbdb0 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Sat, 25 Jul 2026 09:02:39 -0700 Subject: [PATCH 04/11] fix(desktop): clear stale working badges on agent stop/restart (#2803) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Desktop stops or restarts a managed-agent pair, the harness is SIGKILLed after 1s and `turn_completed` never lands. `activeAgentTurnsStore` then sees the "all turns silent at once" pattern and delays badge cleanup for up to 3 min (the pause that protects live badges during transient relay-stream gaps). This is unnecessary when Desktop itself issued the kill — there is no relay-gap ambiguity. ## What changed **`activeAgentTurnsStore.ts`** — new `clearActiveTurnsForAgent(pubkey)` - Tombstones every live turn for the agent via `recordTerminal` (blocks in-flight `turn_liveness` frames from resurrecting them via `resurrectTurn`) - Removes the agent's entry from `activeTurnsByAgent` - Preserves `lastProcessed` (watermark) — full-buffer replay after the clear is a no-op - Preserves `clockOffsetByAgent` — still valid, harmless **`managedAgentRuntimeHooks.ts`** — clearing at the successful-stop boundary - New `clearActiveTurnsForAgentOnStop(pubkey, relayUrl?)` — relay-scope gate: only clears when the stopped pair's relay matches the active community (pair-scoped), or when an active community is configured (agent-wide ops) - New `restartManagedAgentPair(pubkey, relayUrl, stop, clear, start)` — dependency-injected stop → clear → start sequence; the `restart` branch of `useManagedAgentRuntimeAction`'s `mutationFn` is a single call into it. The clear fires after a successful stop and before start begins, so the badge is gone even when start fails, a failed stop clears nothing, and no clear can run after the new process is spawned (genuinely-new turns are never wiped) - `useManagedAgentRuntimeAction.onSuccess` clears for `stop` actions, before the query-cache update **`managedAgentControlActions.ts`** — `onStopped` callback on `respawnManagedAgentWithRules`, invoked after the stop promise resolves and before start begins **`welcomeKickoff.ts`** — same `onStopped` boundary on `restartWelcomeTeammate` **Call sites covered (all stop/restart UI paths):** - `useManagedAgentRuntimeAction` — pair-scoped stop (`onSuccess`) and restart (`restartManagedAgentPair` in `mutationFn`); Members-sidebar + settings card - `useMembersSidebarActions.handleRespawnAll` — via `onStopped` - `useMembersSidebarActions.handleStopAll` — direct local-stop branch - `useMembersSidebarActions.handleLifecycleAction` — local-stop fallback branch - `useAgentLifecycleActions.handleAgentPrimaryAction` — Agents-tab stop - `useAgentLifecycleActions.handleAgentRestart` — via `onStopped` - `useManagedAgentActions.handleStop` / `handleBulkStopRunning` — Agents screen - `useAutoRestartPolicy` — inline, between stop and start - `restartWelcomeTeammate` call site — via `onStopped` Provider agents are excluded at each site: they go through `!shutdown` (relay message), not a direct harness kill. ## Tests Twelve behavior tests across three files: - `activeAgentTurnsStore.test.mjs` (6) — clear removes the agent's turns and notifies subscribers, other agents untouched; full-buffer replay after clear is a no-op (watermark preserved); late `turn_liveness` frame with timestamp ≤ clear time does not resurrect (tombstone); new `turn_started` after clear is tracked normally; badge gone when stop succeeds even if start fails; new frame during start-pending does not resurrect the cleared badge - `managedAgentControlActions.test.mjs` (3) — `onStopped` fires on stop-success/start-failure; does not fire on stop-failure; strict stop → `onStopped` → start ordering - `managedAgentRuntimeHooks.test.mjs` (3) — pair-restart seam: clear ran when start fails (rejection propagates); stop failure invokes neither clear nor start; strict stop → clear → start ordering --------- Signed-off-by: Will Pfleger Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 --- .../agents/activeAgentTurnsStore.test.mjs | 214 ++++++++++++++++++ .../features/agents/activeAgentTurnsStore.ts | 30 +++ .../lib/managedAgentControlActions.test.mjs | 91 +++++++- .../agents/lib/managedAgentControlActions.ts | 5 + .../agents/lib/useAutoRestartPolicy.ts | 2 + .../agents/managedAgentRuntimeHooks.test.mjs | 110 +++++++++ .../agents/managedAgentRuntimeHooks.ts | 97 +++++++- .../agents/ui/useManagedAgentActions.ts | 14 +- .../channels/ui/useMembersSidebarActions.ts | 20 +- .../src/features/onboarding/welcomeKickoff.ts | 7 +- .../profile/ui/useAgentLifecycleActions.ts | 5 + 11 files changed, 582 insertions(+), 13 deletions(-) create mode 100644 desktop/src/features/agents/managedAgentRuntimeHooks.test.mjs diff --git a/desktop/src/features/agents/activeAgentTurnsStore.test.mjs b/desktop/src/features/agents/activeAgentTurnsStore.test.mjs index c0e0b5c01..f182b7b5f 100644 --- a/desktop/src/features/agents/activeAgentTurnsStore.test.mjs +++ b/desktop/src/features/agents/activeAgentTurnsStore.test.mjs @@ -11,6 +11,7 @@ import { saveActiveAgentTurnsForCommunity, restoreActiveAgentTurnsForCommunity, clearSavedCommunitySnapshot, + clearActiveTurnsForAgent, } from "./activeAgentTurnsStore.ts"; import { injectObserverEventsForE2E, @@ -1737,3 +1738,216 @@ describe("community-switch save / restore", () => { ); }); }); + +describe("clearActiveTurnsForAgent", () => { + const EPOCH = Date.parse("2024-01-01T00:00:00Z"); + const at = (ms) => new Date(EPOCH + ms).toISOString(); + + beforeEach(() => { + resetActiveAgentTurnsStore(); + }); + + it("clear removes the agent turns and notifies subscribers; other agents untouched", () => { + // Give AGENT two turns and AGENT_2 one turn. + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ seq: 1, turnId: "t1", channelId: "c1" }), + makeEvent({ seq: 2, turnId: "t2", channelId: "c2" }), + ]); + syncAgentTurnsFromEvents(AGENT_2, [ + makeEvent({ seq: 1, turnId: "t3", channelId: "c3" }), + ]); + + let notified = 0; + const unsub = subscribeActiveAgentTurns(() => { + notified++; + }); + clearActiveTurnsForAgent(AGENT); + unsub(); + + assert.equal( + getActiveTurnsForAgent(AGENT).length, + 0, + "cleared agent must have no turns", + ); + assert.equal(notified, 1, "must notify listeners exactly once"); + + // AGENT_2 is unaffected. + const a2channels = channelIdsOf(getActiveTurnsForAgent(AGENT_2)); + assert.ok(a2channels.has("c3"), "other agent's turns must survive clear"); + }); + + it("full-buffer replay after clear is a no-op (watermark preserved — badge stays gone)", () => { + // Process initial events to set the watermark at seq 2. + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ seq: 1, turnId: "t1", channelId: "c1", timestamp: at(0) }), + makeEvent({ + seq: 2, + turnId: "t2", + channelId: "c2", + timestamp: at(1_000), + }), + ]); + clearActiveTurnsForAgent(AGENT); + assert.equal( + getActiveTurnsForAgent(AGENT).length, + 0, + "should be empty after clear", + ); + + // Replay the identical buffer — every event is at or below the watermark. + let notified = 0; + const unsub = subscribeActiveAgentTurns(() => { + notified++; + }); + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ seq: 1, turnId: "t1", channelId: "c1", timestamp: at(0) }), + makeEvent({ + seq: 2, + turnId: "t2", + channelId: "c2", + timestamp: at(1_000), + }), + ]); + unsub(); + + assert.equal( + notified, + 0, + "replay must not notify — watermark must be preserved", + ); + assert.equal( + getActiveTurnsForAgent(AGENT).length, + 0, + "badge must stay gone", + ); + }); + + it("late turn_liveness frame with timestamp ≤ clear time does not resurrect (tombstone)", () => { + mock.timers.enable({ apis: ["Date"], now: EPOCH }); + + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ seq: 1, turnId: "t1", channelId: "c1", timestamp: at(0) }), + ]); + + // Clear at EPOCH (t=0 in agent-host clock). + clearActiveTurnsForAgent(AGENT); + assert.equal(getActiveTurnsForAgent(AGENT).length, 0); + + // A liveness frame whose timestamp is at or before the clear time must not + // resurrect the badge (tombstone blocks it). Advance seq past the + // watermark by using a higher seq than the initial turn_started. + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ + seq: 2, + kind: "turn_liveness", + turnId: "t1", + channelId: "c1", + timestamp: at(0), // equal to clear time — must NOT resurrect + }), + ]); + + assert.equal( + getActiveTurnsForAgent(AGENT).length, + 0, + "liveness at or before clear time must not resurrect the cleared turn", + ); + + mock.timers.reset(); + }); + + it("new turn_started after clear (restart picked up new work) is tracked normally", () => { + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ seq: 1, turnId: "t1", channelId: "c1", timestamp: at(0) }), + ]); + clearActiveTurnsForAgent(AGENT); + assert.equal(getActiveTurnsForAgent(AGENT).length, 0); + + // A genuinely new turn arrives after the clear with a later timestamp and + // a new turnId — it must be tracked normally. + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ + seq: 2, + turnId: "t2", + channelId: "c1", + timestamp: at(5_000), // strictly newer than the cleared turn's timestamp + }), + ]); + + const turns = getActiveTurnsForAgent(AGENT); + assert.equal(turns.length, 1, "new turn after clear must be tracked"); + assert.ok(channelIdsOf(turns).has("c1"), "new turn must surface c1"); + }); + + it("badge is gone when stop succeeds even if start subsequently fails (stop-boundary clear)", () => { + // Arrange: agent has an active turn. + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ seq: 1, turnId: "t1", channelId: "c1", timestamp: at(0) }), + ]); + assert.equal( + getActiveTurnsForAgent(AGENT).length, + 1, + "turn must be active before stop", + ); + + // Act: simulate what onStopped does — clear at the stop-success boundary, + // before start is called. Start fails (not called here). + clearActiveTurnsForAgent(AGENT); + + // Assert: the badge is gone regardless of what happens to start. + assert.equal( + getActiveTurnsForAgent(AGENT).length, + 0, + "badge must clear at stop-success boundary, not waiting for start to resolve", + ); + }); + + it("new frame arriving while start is pending does not resurrect the cleared badge (tombstone boundary)", () => { + // Simulate: agent was active, stop succeeded and clear ran (onStopped + // fired), start is now in-flight. A stale liveness frame for the OLD + // turn arrives on the wire during the start-pending window. It must NOT + // resurrect the badge — the clear tombstoned it. + mock.timers.enable({ apis: ["Date"], now: EPOCH }); + + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ seq: 1, turnId: "t1", channelId: "c1", timestamp: at(0) }), + ]); + + // onStopped fires: clear at stop boundary (agent-host clock = EPOCH). + clearActiveTurnsForAgent(AGENT); + + // Stale liveness for t1 arrives with timestamp ≤ clear time (on-wire + // frame from before the kill). Must be blocked by the tombstone. + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ + seq: 2, + kind: "turn_liveness", + turnId: "t1", + channelId: "c1", + timestamp: at(0), + }), + ]); + assert.equal( + getActiveTurnsForAgent(AGENT).length, + 0, + "stale liveness during start-pending must not resurrect the cleared badge", + ); + + // Genuine new turn from the restarted agent arrives later with a new id + // and strictly newer timestamp — must be tracked normally. + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ + seq: 3, + turnId: "t2-new", + channelId: "c1", + timestamp: at(3_000), + }), + ]); + assert.equal( + getActiveTurnsForAgent(AGENT).length, + 1, + "genuine new turn from restarted agent must be tracked", + ); + + mock.timers.reset(); + }); +}); diff --git a/desktop/src/features/agents/activeAgentTurnsStore.ts b/desktop/src/features/agents/activeAgentTurnsStore.ts index 07ad4fa6b..b28192950 100644 --- a/desktop/src/features/agents/activeAgentTurnsStore.ts +++ b/desktop/src/features/agents/activeAgentTurnsStore.ts @@ -574,6 +574,36 @@ export function useActiveAgentTurnsBridge( }, [agents]); } +/** + * Immediately clear all active turns for a specific agent — called when + * Desktop itself stops or restarts the agent, so the turn store doesn't + * have to wait for the 3-minute prune-pause backstop. + * + * Preserves `lastProcessed` (the watermark) so a full-buffer replay after + * the clear is still a no-op — without the watermark a replayed + * `turn_started` would immediately resurrect the badge. Preserves + * `clockOffsetByAgent` — the offset remains valid and harmless. + * + * Tombstones every cleared turn (C) so an in-flight `turn_liveness` frame + * already on the wire at kill time cannot resurrect the badge via + * `resurrectTurn`. A restarted agent's genuinely new turns carry new + * turnIds / newer timestamps, so the tombstones don't block them. + */ +export function clearActiveTurnsForAgent(agentPubkey: string): void { + const key = normalizePubkey(agentPubkey); + const agentTurns = activeTurnsByAgent.get(key); + if (!agentTurns || agentTurns.size === 0) return; + + const agentClockNow = Date.now() - (clockOffsetByAgent.get(key) ?? 0); + for (const turnId of agentTurns.keys()) { + recordTerminal(key, turnId, agentClockNow); + } + + activeTurnsByAgent.delete(key); + invalidateCache(key); + notifyListeners(); +} + /** * Clears all live turn state (active turns, offsets, watermarks, tombstones). * Intentionally preserves `savedByCommunity` — community-switch snapshots diff --git a/desktop/src/features/agents/lib/managedAgentControlActions.test.mjs b/desktop/src/features/agents/lib/managedAgentControlActions.test.mjs index f2c42b94e..e6926b36d 100644 --- a/desktop/src/features/agents/lib/managedAgentControlActions.test.mjs +++ b/desktop/src/features/agents/lib/managedAgentControlActions.test.mjs @@ -1,7 +1,10 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { startManagedAgentWithRules } from "./managedAgentControlActions.ts"; +import { + startManagedAgentWithRules, + respawnManagedAgentWithRules, +} from "./managedAgentControlActions.ts"; function agent(overrides = {}) { return { @@ -77,3 +80,89 @@ test("ordinary local agents still start normally", async () => { }); assert.equal(calledWith, "deadbeef".repeat(8)); }); + +// --- respawnManagedAgentWithRules: stop→clear→start boundary tests ----------- + +test("test_respawn_stop_success_start_failure_onStopped_still_fires", async () => { + // Prove: onStopped fires at the stop-success boundary even when start later + // throws. This is the key discriminator: on round-1 code the clear only + // ran after the full respawn, so a failed start left the badge intact. + const runningAgent = agent({ status: "running" }); + let onStoppedFired = false; + + await assert.rejects( + respawnManagedAgentWithRules({ + agent: runningAgent, + stopManagedAgent: async () => { + /* stop succeeds */ + }, + startManagedAgent: async () => { + throw new Error("start failed"); + }, + onStopped: () => { + onStoppedFired = true; + }, + }), + /start failed/, + ); + + assert.ok( + onStoppedFired, + "onStopped must fire at stop-success boundary even when start subsequently fails", + ); +}); + +test("test_respawn_stop_failure_onStopped_not_called", async () => { + // Prove: onStopped does NOT fire when stop itself throws. Clearing on a + // failed stop would remove a badge that is still legitimately active. + const runningAgent = agent({ status: "running" }); + let onStoppedFired = false; + + await assert.rejects( + respawnManagedAgentWithRules({ + agent: runningAgent, + stopManagedAgent: async () => { + throw new Error("stop failed"); + }, + startManagedAgent: async () => { + /* should not be reached */ + }, + onStopped: () => { + onStoppedFired = true; + }, + }), + /stop failed/, + ); + + assert.ok( + !onStoppedFired, + "onStopped must NOT fire when stop itself fails — badge is still active", + ); +}); + +test("test_respawn_onStopped_fires_before_start_resolves", async () => { + // Prove: onStopped fires strictly between stop resolution and start + // invocation. A clear that fires after start begins can tombstone genuine + // new turns from the freshly spawned process. + const runningAgent = agent({ status: "running" }); + const events = []; + + await respawnManagedAgentWithRules({ + agent: runningAgent, + stopManagedAgent: async () => { + events.push("stop"); + }, + startManagedAgent: async () => { + events.push("start"); + }, + onStopped: () => { + events.push("onStopped"); + }, + }); + + assert.deepEqual( + events, + ["stop", "onStopped", "start"], + "onStopped must fire after stop resolves and before start is called", + ); +}); diff --git a/desktop/src/features/agents/lib/managedAgentControlActions.ts b/desktop/src/features/agents/lib/managedAgentControlActions.ts index cdde26300..dbaaaba80 100644 --- a/desktop/src/features/agents/lib/managedAgentControlActions.ts +++ b/desktop/src/features/agents/lib/managedAgentControlActions.ts @@ -92,13 +92,18 @@ export async function respawnManagedAgentWithRules({ agent, startManagedAgent, stopManagedAgent, + onStopped, }: { agent: ManagedAgent; startManagedAgent: StartManagedAgent; stopManagedAgent: StopManagedAgent; + /** Called after a successful stop and before start begins — use this to + * clear stale working badges at the right boundary. */ + onStopped?: () => void; }) { if (agent.backend.type === "local" && isManagedAgentActive(agent)) { await stopManagedAgent(agent.pubkey); + onStopped?.(); } await startManagedAgent(agent.pubkey); diff --git a/desktop/src/features/agents/lib/useAutoRestartPolicy.ts b/desktop/src/features/agents/lib/useAutoRestartPolicy.ts index 7e3093090..e8e149ccd 100644 --- a/desktop/src/features/agents/lib/useAutoRestartPolicy.ts +++ b/desktop/src/features/agents/lib/useAutoRestartPolicy.ts @@ -5,6 +5,7 @@ import { managedAgentsQueryKey, useManagedAgentsQuery, } from "@/features/agents/hooks"; +import { clearActiveTurnsForAgentOnStop } from "@/features/agents/managedAgentRuntimeHooks"; import { startManagedAgent, stopManagedAgent, @@ -109,6 +110,7 @@ export function useAutoRestartPolicy() { return; } await stopManagedAgent(agent.pubkey); + clearActiveTurnsForAgentOnStop(agent.pubkey); await startManagedAgent(agent.pubkey); } catch { // Failed attempt: edge stays consumed — badge-only until the diff --git a/desktop/src/features/agents/managedAgentRuntimeHooks.test.mjs b/desktop/src/features/agents/managedAgentRuntimeHooks.test.mjs new file mode 100644 index 000000000..3e961b585 --- /dev/null +++ b/desktop/src/features/agents/managedAgentRuntimeHooks.test.mjs @@ -0,0 +1,110 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { restartManagedAgentPair } from "./managedAgentRuntimeHooks.ts"; + +// --------------------------------------------------------------------------- +// restartManagedAgentPair: discriminating regression tests for the pair +// restart lifecycle boundary (stop → relay-scoped clear → start). +// +// These tests exercise the exact function called by useManagedAgentRuntimeAction's +// mutationFn restart branch, so reverting to the old combined Rust command +// (which cleared only in onSuccess, after the new process was already running) +// would make tests (a) and (c) fail. +// --------------------------------------------------------------------------- + +const PUBKEY = "deadbeef".repeat(8); +const RELAY = "wss://relay.example"; + +/** Returns a resolved-status stub sufficient for the return-type assertion. */ +function makeStatus() { + return { + pubkey: PUBKEY, + relayUrl: RELAY, + localSetup: true, + lifecycle: "running", + }; +} + +test("test_pair_restart_stop_success_start_failure_clear_still_ran", async () => { + // Stop succeeds, start throws. The clear must have fired — badge is gone + // regardless of the start failure. On the old combined-command approach, + // a rejected command meant onSuccess never ran and the badge survived. + let clearFired = false; + + await assert.rejects( + restartManagedAgentPair( + PUBKEY, + RELAY, + async () => makeStatus(), // stop succeeds + (_pubkey, _relayUrl) => { + clearFired = true; + }, + async () => { + throw new Error("start failed"); + }, + ), + /start failed/, + ); + + assert.ok( + clearFired, + "clear must fire at stop-success boundary even when start subsequently fails", + ); +}); + +test("test_pair_restart_stop_failure_neither_clear_nor_start_called", async () => { + // Stop throws. Neither clear nor start should run — clearing on a failed + // stop would remove a badge that is still legitimately active. + let clearFired = false; + let startCalled = false; + + await assert.rejects( + restartManagedAgentPair( + PUBKEY, + RELAY, + async () => { + throw new Error("stop failed"); + }, + (_pubkey, _relayUrl) => { + clearFired = true; + }, + async () => { + startCalled = true; + return makeStatus(); + }, + ), + /stop failed/, + ); + + assert.ok(!clearFired, "clear must NOT fire when stop itself fails"); + assert.ok(!startCalled, "start must NOT be called when stop fails"); +}); + +test("test_pair_restart_strict_stop_clear_start_ordering", async () => { + // Verify the operations fire in the guaranteed order: stop → clear → start. + // A clear that fires after start begins can tombstone genuine new turns. + const events = []; + + await restartManagedAgentPair( + PUBKEY, + RELAY, + async () => { + events.push("stop"); + return makeStatus(); + }, + (_pubkey, _relayUrl) => { + events.push("clear"); + }, + async () => { + events.push("start"); + return makeStatus(); + }, + ); + + assert.deepEqual( + events, + ["stop", "clear", "start"], + "operations must fire in stop → clear → start order", + ); +}); diff --git a/desktop/src/features/agents/managedAgentRuntimeHooks.ts b/desktop/src/features/agents/managedAgentRuntimeHooks.ts index aef9c8c1e..96a3abc78 100644 --- a/desktop/src/features/agents/managedAgentRuntimeHooks.ts +++ b/desktop/src/features/agents/managedAgentRuntimeHooks.ts @@ -5,15 +5,19 @@ import { type QueryClient, } from "@tanstack/react-query"; -import { loadCommunities } from "@/features/communities/communityStorage"; +import { clearActiveTurnsForAgent } from "@/features/agents/activeAgentTurnsStore"; +import { + loadActiveCommunityId, + loadCommunities, +} from "@/features/communities/communityStorage"; import { listManagedAgentRuntimes, reconcileManagedAgentRuntimes, - restartManagedAgentRuntime, startManagedAgentRuntime, stopManagedAgentRuntime, } from "@/shared/api/tauriManagedAgents"; import type { ManagedAgentRuntimeStatus } from "@/shared/api/types"; +import { canonicalRelayUrl } from "./managedAgentRuntimeStatus"; export const managedAgentRuntimesQueryKey = ["managed-agent-runtimes"] as const; @@ -101,6 +105,79 @@ export function useManagedAgentRuntimesQuery(options?: { enabled?: boolean }) { }); } +/** + * Clear the active community's working badges for an agent when Desktop + * performs an agent-wide stop or restart that does not go through the + * pair-scoped `useManagedAgentRuntimeAction` mutation. Applies the same + * relay-scope gate: only wipes the store when the active community relay + * matches `relayUrl`, or — for agent-wide operations with no known relay — + * when any of the agent's configured pairs is in the active community. + * + * Pass `relayUrl` when a specific pair relay is known (preferred). Omit it + * (pass null/undefined) for agent-wide operations: the function then clears + * whenever the active community is configured, since the agent-wide stop + * affects all pairs, including the one in the active community. + */ +export function clearActiveTurnsForAgentOnStop( + pubkey: string, + relayUrl?: string | null, +): void { + const activeId = loadActiveCommunityId(); + if (!activeId) return; + const activeCommunity = loadCommunities().find((c) => c.id === activeId); + if (!activeCommunity) return; + + if (relayUrl != null) { + // Pair-scoped: only clear when the stopped pair's relay matches the active + // community. A mismatch means the stop targets a different community's + // store — leave it alone. + const activeCanonical = canonicalRelayUrl(activeCommunity.relayUrl); + const stoppedCanonical = canonicalRelayUrl(relayUrl); + if ( + activeCanonical === null || + stoppedCanonical === null || + activeCanonical !== stoppedCanonical + ) { + return; + } + } + // Agent-wide (relayUrl omitted): active community is confirmed to exist, so + // the stop affects the active pair among others — clear. + + clearActiveTurnsForAgent(pubkey); +} + +/** + * Execute a pair restart as stop → relay-scoped badge clear → start. + * + * Extracted from `useManagedAgentRuntimeAction`'s `mutationFn` so the + * three-step lifecycle boundary can be tested directly without a hook-render + * harness. All three operations are injected, keeping this function free of + * React and Tauri imports. + * + * Guarantees: + * - Clear fires only when stop succeeds. + * - A failed start occurs after the clear — the badge is already gone. + * - No clear can fire after start begins, so genuinely-new turns are safe. + */ +export async function restartManagedAgentPair( + pubkey: string, + relayUrl: string, + stop: ( + pubkey: string, + relayUrl: string, + ) => Promise, + clear: (pubkey: string, relayUrl: string) => void, + start: ( + pubkey: string, + relayUrl: string, + ) => Promise, +): Promise { + await stop(pubkey, relayUrl); + clear(pubkey, relayUrl); + return start(pubkey, relayUrl); +} + export function useManagedAgentRuntimeAction() { const queryClient = useQueryClient(); return useMutation({ @@ -115,11 +192,23 @@ export function useManagedAgentRuntimeAction() { }) => { if (action === "stop") return stopManagedAgentRuntime(pubkey, relayUrl); if (action === "restart") { - return restartManagedAgentRuntime(pubkey, relayUrl); + return restartManagedAgentPair( + pubkey, + relayUrl, + stopManagedAgentRuntime, + clearActiveTurnsForAgentOnStop, + startManagedAgentRuntime, + ); } return startManagedAgentRuntime(pubkey, relayUrl); }, - onSuccess: (runtime) => { + onSuccess: (runtime, { action }) => { + // For stop-only: clear stale working badges immediately. The restart + // path already clears at the stop-success boundary inside mutationFn. + if (action === "stop") { + clearActiveTurnsForAgentOnStop(runtime.pubkey, runtime.relayUrl); + } + queryClient.setQueryData( managedAgentRuntimesQueryKey, (current = []) => { diff --git a/desktop/src/features/agents/ui/useManagedAgentActions.ts b/desktop/src/features/agents/ui/useManagedAgentActions.ts index 4eb9b6ed0..e1c2e9c9f 100644 --- a/desktop/src/features/agents/ui/useManagedAgentActions.ts +++ b/desktop/src/features/agents/ui/useManagedAgentActions.ts @@ -29,6 +29,7 @@ import { startManagedAgentWithRules, stopManagedAgentWithRules, } from "../lib/managedAgentControlActions"; +import { clearActiveTurnsForAgentOnStop } from "../managedAgentRuntimeHooks"; import { availableRuntimesForStart, buildInstanceInputForDefinition, @@ -248,6 +249,9 @@ export function useManagedAgentActions() { relayAgents: relayAgentsQuery.data ?? [], stopManagedAgent: stopMutation.mutateAsync, }); + if (agent.backend.type === "local") { + clearActiveTurnsForAgentOnStop(pubkey); + } if (result.noticeMessage) { setActionNoticeMessage(result.noticeMessage); } @@ -368,13 +372,17 @@ export function useManagedAgentActions() { managedAgents.filter((a) => isManagedAgentActive(a)), "Stop", "stop", - (a) => - stopManagedAgentWithRules({ + async (a) => { + await stopManagedAgentWithRules({ agent: a, channels: channelsQuery.data ?? [], relayAgents: relayAgentsQuery.data ?? [], stopManagedAgent: stopMutation.mutateAsync, - }), + }); + if (a.backend.type === "local") { + clearActiveTurnsForAgentOnStop(a.pubkey); + } + }, ); } diff --git a/desktop/src/features/channels/ui/useMembersSidebarActions.ts b/desktop/src/features/channels/ui/useMembersSidebarActions.ts index 2586f7921..cc8f40622 100644 --- a/desktop/src/features/channels/ui/useMembersSidebarActions.ts +++ b/desktop/src/features/channels/ui/useMembersSidebarActions.ts @@ -11,7 +11,10 @@ import { startManagedAgentWithRules, stopManagedAgentWithRules, } from "@/features/agents/lib/managedAgentControlActions"; -import { useManagedAgentRuntimeAction } from "@/features/agents/managedAgentRuntimeHooks"; +import { + clearActiveTurnsForAgentOnStop, + useManagedAgentRuntimeAction, +} from "@/features/agents/managedAgentRuntimeHooks"; import { managedAgentPairAction } from "@/features/agents/managedAgentRuntimeStatus"; import { channelsQueryKey, @@ -174,6 +177,9 @@ export function useMembersSidebarActions({ preferredChannelId: channelId, stopManagedAgent: stopManagedAgentMutation.mutateAsync, }); + if (agent.backend.type === "local") { + clearActiveTurnsForAgentOnStop(agent.pubkey); + } setActionNoticeMessage( agent.backend.type === "provider" ? `Shutdown command sent to ${agent.name}.` @@ -203,6 +209,7 @@ export function useMembersSidebarActions({ agent, startManagedAgent: startManagedAgentMutation.mutateAsync, stopManagedAgent: stopManagedAgentMutation.mutateAsync, + onStopped: () => clearActiveTurnsForAgentOnStop(agent.pubkey), }); return undefined; }, @@ -216,13 +223,18 @@ export function useMembersSidebarActions({ async function handleStopAll() { await runBulkAgentAction({ - action: (agent) => - stopManagedAgentWithRules({ + action: async (agent) => { + const result = await stopManagedAgentWithRules({ agent, ...EMPTY_AGENT_CONTEXT, preferredChannelId: channelId, stopManagedAgent: stopManagedAgentMutation.mutateAsync, - }), + }); + if (agent.backend.type === "local") { + clearActiveTurnsForAgentOnStop(agent.pubkey); + } + return result; + }, actionKey: "bulk-stop", agents: stoppableManagedBots, failureMessage: "Failed to stop agent.", diff --git a/desktop/src/features/onboarding/welcomeKickoff.ts b/desktop/src/features/onboarding/welcomeKickoff.ts index fd5dac394..57352b529 100644 --- a/desktop/src/features/onboarding/welcomeKickoff.ts +++ b/desktop/src/features/onboarding/welcomeKickoff.ts @@ -6,6 +6,7 @@ import { useManagedAgentsQuery, } from "@/features/agents/hooks"; import { useGlobalAgentConfig } from "@/features/agents/useGlobalAgentConfig"; +import { clearActiveTurnsForAgentOnStop } from "@/features/agents/managedAgentRuntimeHooks"; import { useCommunities } from "@/features/communities/useCommunities"; import { welcomeKickoffMarker } from "@/features/onboarding/devFreshOnboarding"; import { resolveAgentReadiness } from "@/features/onboarding/ui/agentReadiness"; @@ -450,12 +451,14 @@ export async function restartWelcomeTeammate( options: { stopAgent?: typeof stopManagedAgent; startAgent?: typeof startManagedAgent; + onStopped?: () => void; } = {}, ) { const stopAgent = options.stopAgent ?? stopManagedAgent; const startAgent = options.startAgent ?? startManagedAgent; if (agent.status === "running") { await stopAgent(agent.pubkey); + options.onStopped?.(); } return startAgent(agent.pubkey); } @@ -609,7 +612,9 @@ export function useWelcomeKickoff( isTeammate && welcomeTeammateNeedsRestart(agent, resolvedAgentSet.lead.pubkey) ) { - return restartWelcomeTeammate(agent); + return restartWelcomeTeammate(agent, { + onStopped: () => clearActiveTurnsForAgentOnStop(agent.pubkey), + }); } return agent.status === "running" || agent.status === "deployed" ? Promise.resolve(agent) diff --git a/desktop/src/features/profile/ui/useAgentLifecycleActions.ts b/desktop/src/features/profile/ui/useAgentLifecycleActions.ts index f84e149c1..62d0d3c7a 100644 --- a/desktop/src/features/profile/ui/useAgentLifecycleActions.ts +++ b/desktop/src/features/profile/ui/useAgentLifecycleActions.ts @@ -7,6 +7,7 @@ import { startManagedAgentWithRules, stopManagedAgentWithRules, } from "@/features/agents/lib/managedAgentControlActions"; +import { clearActiveTurnsForAgentOnStop } from "@/features/agents/managedAgentRuntimeHooks"; import type { Channel, ManagedAgent, RelayAgent } from "@/shared/api/types"; export function useAgentLifecycleActions({ @@ -33,6 +34,9 @@ export function useAgentLifecycleActions({ relayAgents: relayAgents ?? [], stopManagedAgent, }); + if (managedAgent.backend.type === "local") { + clearActiveTurnsForAgentOnStop(managedAgent.pubkey); + } toast.success(result.noticeMessage ?? `Stopped ${managedAgent.name}.`); return; } @@ -67,6 +71,7 @@ export function useAgentLifecycleActions({ agent: managedAgent, startManagedAgent, stopManagedAgent, + onStopped: () => clearActiveTurnsForAgentOnStop(managedAgent.pubkey), }); toast.success(`Restarted ${managedAgent.name}.`); } catch (error) { From 2f0041595d72529c06885680d2bd07ddb6a0beb4 Mon Sep 17 00:00:00 2001 From: Cameron Hotchkies Date: Sat, 25 Jul 2026 09:26:21 -0700 Subject: [PATCH 05/11] refactor(relay): expose reconnect timing policy (#2310) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Prepare the relay reconnect controller for an isolated behavior-fix PR without changing runtime behavior in this one. ## What - Export the current reconnect timing policy from `RelayReconnectController` - Allow controller tests to inject the complete timing policy - Add characterization coverage for the production timing values and injected fast-path/poll/backstop timers ## Risk Assessment Low — this preserves the existing production timing values and only replaces private module constants with a default policy object used by the controller. The weak reconnect-timer and backstop wrapper extractions were removed from this PR. ## References - `cd desktop && pnpm typecheck` - `cd desktop && pnpm check` - `cd desktop && pnpm test` (3373 pass) - `git diff --check` - Push hooks were bypassed after the requested desktop validation because the broad pre-push hook runs without Hermit here and fails on Node 20/pnpm 11 plus unhealthy local Postgres services. Generated with Codex Signed-off-by: Wes Co-authored-by: Wes Co-authored-by: npub1m0vvn9qm5md0a080p27qzkm9uaw49e699ukwfq7fc0756xq0y5zqhzhdk2 Co-authored-by: Carl --- .../api/relayReconnectController.test.mjs | 35 +++++++++++++++++- .../shared/api/relayReconnectController.ts | 36 ++++++++++++++----- 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/desktop/src/shared/api/relayReconnectController.test.mjs b/desktop/src/shared/api/relayReconnectController.test.mjs index 37b942434..0a3a1c935 100644 --- a/desktop/src/shared/api/relayReconnectController.test.mjs +++ b/desktop/src/shared/api/relayReconnectController.test.mjs @@ -9,7 +9,10 @@ import assert from "node:assert/strict"; import test, { mock } from "node:test"; -import { RelayReconnectController } from "./relayReconnectController.ts"; +import { + DEFAULT_RECONNECT_TIMING_POLICY, + RelayReconnectController, +} from "./relayReconnectController.ts"; // ── Dep builder helpers ─────────────────────────────────────────────────────── @@ -62,6 +65,36 @@ function makeDeps({ return deps; } +// ── Timing policy ───────────────────────────────────────────────────────────── + +test("default timing policy preserves current reconnect timings", () => { + assert.deepEqual(DEFAULT_RECONNECT_TIMING_POLICY, { + fastPathTimeoutMs: 4_000, + pollIntervalMs: 3_000, + backstopMs: 120_000, + }); +}); + +test("injected timing policy drives fast-path, poll, and backstop timers", async () => { + const ctrl = new RelayReconnectController({ + fastPathTimeoutMs: 11, + pollIntervalMs: 22, + backstopMs: 33, + }); + const deps = makeDeps({ + preconnectResult: async () => { + throw new Error("relay unreachable"); + }, + hookConfiguredResult: async () => false, + }); + + await ctrl.start(deps); + + assert.equal(deps.setTimeout.mock.calls[0].arguments[1], 11); + assert.equal(deps.setInterval.mock.calls[0].arguments[1], 22); + assert.equal(deps.setTimeout.mock.calls[1].arguments[1], 33); +}); + // ── Phase 1: fast path ──────────────────────────────────────────────────────── test("fast-path success — hook never invoked, onSuccess fires, state resets", async () => { diff --git a/desktop/src/shared/api/relayReconnectController.ts b/desktop/src/shared/api/relayReconnectController.ts index 45bba3666..fcea2f29d 100644 --- a/desktop/src/shared/api/relayReconnectController.ts +++ b/desktop/src/shared/api/relayReconnectController.ts @@ -28,12 +28,22 @@ * testable without React or Tauri. */ -/** Short deadline for the optimistic fast-path attempt. */ -const FAST_PATH_TIMEOUT_MS = 4_000; -/** Interval between poll attempts during phase 3. */ -const POLL_INTERVAL_MS = 3_000; -/** Maximum total time to keep polling before giving up. */ -const BACKSTOP_MS = 120_000; +/** Timer durations used by the reconnect controller. */ +export type ReconnectTimingPolicy = { + /** Short deadline for the optimistic fast-path attempt. */ + fastPathTimeoutMs: number; + /** Interval between poll attempts during phase 3. */ + pollIntervalMs: number; + /** Maximum total time to keep polling before giving up. */ + backstopMs: number; +}; + +/** Current production reconnect timings. */ +export const DEFAULT_RECONNECT_TIMING_POLICY: ReconnectTimingPolicy = { + fastPathTimeoutMs: 4_000, + pollIntervalMs: 3_000, + backstopMs: 120_000, +}; export type ReconnectState = { isPending: boolean; @@ -74,6 +84,14 @@ function withDeadline( } export class RelayReconnectController { + private readonly timingPolicy: ReconnectTimingPolicy; + + constructor( + timingPolicy: ReconnectTimingPolicy = DEFAULT_RECONNECT_TIMING_POLICY, + ) { + this.timingPolicy = timingPolicy; + } + private state: ReconnectState = { isPending: false, isWaitingOnReconnectHook: false, @@ -136,7 +154,7 @@ export class RelayReconnectController { try { await withDeadline( deps.preconnect(), - FAST_PATH_TIMEOUT_MS, + this.timingPolicy.fastPathTimeoutMs, "fast-path", deps.setTimeout, deps.clearTimeout, @@ -226,7 +244,7 @@ export class RelayReconnectController { .catch(() => { // Poll failed — keep trying. }); - }, POLL_INTERVAL_MS); + }, this.timingPolicy.pollIntervalMs); this.backstopId = deps.setTimeout(() => { if (resolved || cancelled()) return; @@ -235,7 +253,7 @@ export class RelayReconnectController { // background retry loop keeps running. The notification is soft. onBackstop(); this.finish(() => {}, false); - }, BACKSTOP_MS); + }, this.timingPolicy.backstopMs); // Return false now; async success is delivered via state updates. return false; From 499c5d349dab13bc906b1af5fe1fcb09ce2afa81 Mon Sep 17 00:00:00 2001 From: Cameron Hotchkies Date: Sat, 25 Jul 2026 10:00:47 -0700 Subject: [PATCH 06/11] fix(relay): preserve reconnect backoff (#2759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Manual relay recovery polls every three seconds, repeatedly consuming the session's exponential-backoff timer and leaving reconnect behavior stuck or noisy on degraded networks. ## What - Replace fixed-cadence Phase 3 polling with observation of the RelayClient background reconnect loop - Raise the fast-path deadline above the native websocket timeout and enforce that contract in a regression test - Keep the existing 120-second backstop as a soft UI timeout without stopping background retries ## Risk Assessment Medium — this changes live relay recovery timing, but removes a competing retry loop rather than adding one. The existing connection-state subscription remains the success signal. ## References - Stacked on #2310 (`lazyjoe/reconnect-testability-refactor`) - Investigation: `RESEARCH/BUG_RELAY_RECONNECT_HANG.md` - `just ci` - `cd desktop && pnpm typecheck && pnpm check && pnpm test` (3485 pass) - `git diff --check` Generated with Codex Signed-off-by: npub1x4hk035p3p9q39a3fcrd2fe30lpkrhr5dwe0cqzzjphxyyh8m0gsq4vqap <356f67c681884a0897b14e06d527317fc361dc746bb2fc0042906e6212e7dbd1@sprout-oss.stage.blox.sqprod.co> Co-authored-by: npub1x4hk035p3p9q39a3fcrd2fe30lpkrhr5dwe0cqzzjphxyyh8m0gsq4vqap <356f67c681884a0897b14e06d527317fc361dc746bb2fc0042906e6212e7dbd1@sprout-oss.stage.blox.sqprod.co> --- .../api/relayReconnectController.test.mjs | 106 ++++++------------ .../shared/api/relayReconnectController.ts | 58 +++------- desktop/src/shared/api/useReconnectRelay.ts | 2 - 3 files changed, 50 insertions(+), 116 deletions(-) diff --git a/desktop/src/shared/api/relayReconnectController.test.mjs b/desktop/src/shared/api/relayReconnectController.test.mjs index 0a3a1c935..3a3890f5a 100644 --- a/desktop/src/shared/api/relayReconnectController.test.mjs +++ b/desktop/src/shared/api/relayReconnectController.test.mjs @@ -7,7 +7,10 @@ */ import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import path from "node:path"; import test, { mock } from "node:test"; +import { fileURLToPath } from "node:url"; import { DEFAULT_RECONNECT_TIMING_POLICY, @@ -24,9 +27,7 @@ function makeDeps({ connectionStateListener = null, } = {}) { const timers = []; - const intervals = []; let timerSeq = 0; - let intervalSeq = 0; const deps = { preconnect: mock.fn(preconnectResult), @@ -47,38 +48,44 @@ function makeDeps({ const idx = timers.findIndex((t) => t.id === id); if (idx !== -1) timers.splice(idx, 1); }), - setInterval: mock.fn((fn, ms) => { - const id = ++intervalSeq; - intervals.push({ id, fn, ms }); - return id; - }), - clearInterval: mock.fn((id) => { - const idx = intervals.findIndex((t) => t.id === id); - if (idx !== -1) intervals.splice(idx, 1); - }), - // Test helpers to fire timers/intervals. + // Test helper to fire timers. _fireTimer: (id) => timers.find((t) => t.id === id)?.fn(), - _fireInterval: (id) => intervals.find((t) => t.id === id)?.fn(), _timers: timers, - _intervals: intervals, }; return deps; } // ── Timing policy ───────────────────────────────────────────────────────────── +const testDir = path.dirname(fileURLToPath(import.meta.url)); + +test("fast path outlives the native websocket connection timeout", () => { + const rustSource = readFileSync( + path.resolve(testDir, "../../../src-tauri/src/native_websocket.rs"), + "utf8", + ); + const match = rustSource.match( + /const CONNECT_TIMEOUT: Duration = Duration::from_secs\((\d+)\);/, + ); + assert.ok(match, "native websocket connect timeout is declared in seconds"); + + const nativeConnectTimeoutMs = Number(match[1]) * 1_000; + assert.ok( + DEFAULT_RECONNECT_TIMING_POLICY.fastPathTimeoutMs > nativeConnectTimeoutMs, + "fast path must wait for the native websocket attempt to settle", + ); +}); + test("default timing policy preserves current reconnect timings", () => { assert.deepEqual(DEFAULT_RECONNECT_TIMING_POLICY, { - fastPathTimeoutMs: 4_000, - pollIntervalMs: 3_000, + fastPathTimeoutMs: 11_000, backstopMs: 120_000, }); }); -test("injected timing policy drives fast-path, poll, and backstop timers", async () => { +test("injected timing policy drives fast-path and backstop timers", async () => { const ctrl = new RelayReconnectController({ fastPathTimeoutMs: 11, - pollIntervalMs: 22, backstopMs: 33, }); const deps = makeDeps({ @@ -91,7 +98,6 @@ test("injected timing policy drives fast-path, poll, and backstop timers", async await ctrl.start(deps); assert.equal(deps.setTimeout.mock.calls[0].arguments[1], 11); - assert.equal(deps.setInterval.mock.calls[0].arguments[1], 22); assert.equal(deps.setTimeout.mock.calls[1].arguments[1], 33); }); @@ -113,13 +119,12 @@ test("fast-path success — hook never invoked, onSuccess fires, state resets", }); }); -test("fast-path success — no poll interval or backstop scheduled", async () => { +test("fast-path success — no backstop scheduled", async () => { const ctrl = new RelayReconnectController(); const deps = makeDeps({ preconnectResult: async () => {} }); await ctrl.start(deps); - assert.equal(deps.setInterval.mock.calls.length, 0, "no poll interval"); // The fast-path withDeadline schedules one timeout for the deadline, but // that is cleared by finally(). After success no backstop should remain. assert.equal( @@ -136,7 +141,7 @@ test("escalation fires only when fast path fails and hook is configured", async let callIdx = 0; const deps = makeDeps({ preconnectResult: async () => { - // First call (fast path) fails; subsequent calls (poll) succeed. + // First call (fast path) fails. if (++callIdx === 1) throw new Error("relay unreachable"); }, hookConfiguredResult: async () => true, @@ -174,7 +179,7 @@ test("escalation skipped when hook not configured", async () => { ); }); -test("hook failure is non-fatal — polling still starts", async () => { +test("hook failure is non-fatal — background wait still starts", async () => { const ctrl = new RelayReconnectController(); const deps = makeDeps({ preconnectResult: async () => { @@ -195,48 +200,12 @@ test("hook failure is non-fatal — polling still starts", async () => { } assert.equal(threw, false, "hook failure does not propagate"); - assert.ok( - deps._intervals.length > 0 || deps._timers.length > 0, - "phase 3 timers scheduled", - ); + assert.ok(deps._timers.length > 0, "phase 3 backstop scheduled"); }); -// ── Phase 3: poll-until-connected ───────────────────────────────────────────── +// ── Phase 3: wait for background reconnect ─────────────────────────────────── -test("poll preempts backstop — onSuccess fires when poll wins", async () => { - const ctrl = new RelayReconnectController(); - let callIdx = 0; - const deps = makeDeps({ - preconnectResult: async () => { - if (++callIdx <= 2) throw new Error("not yet"); - // Third call succeeds — simulates poll winning. - }, - hookConfiguredResult: async () => false, - }); - - // Phase 1 fails, enters phase 3. start() returns false. - const promise = ctrl.start(deps); - await promise; - - // Fire the poll interval twice to reach the succeeding call. - const intervalId = deps._intervals[0]?.id; - assert.ok(intervalId !== undefined, "poll interval is active"); - - await deps._fireInterval(intervalId); // call 2 — fails - await deps._fireInterval(intervalId); // call 3 — succeeds - - assert.equal( - deps.onSuccess.mock.calls.length, - 1, - "onSuccess fired after poll win", - ); - assert.deepEqual(ctrl.getState(), { - isPending: false, - isWaitingOnReconnectHook: false, - }); -}); - -test("connection-state emitter fires onSuccess and cancels poll/backstop", async () => { +test("connection-state emitter fires onSuccess and cancels backstop", async () => { let capturedListener = null; const ctrl = new RelayReconnectController(); const deps = makeDeps({ @@ -253,7 +222,7 @@ test("connection-state emitter fires onSuccess and cancels poll/backstop", async await ctrl.start(deps); - // Emitter fires "connected" — should resolve without waiting for poll. + // Emitter fires "connected" — should resolve without waiting for backstop. assert.ok(capturedListener !== null, "connection-state listener registered"); capturedListener("connected"); @@ -266,7 +235,6 @@ test("connection-state emitter fires onSuccess and cancels poll/backstop", async isPending: false, isWaitingOnReconnectHook: false, }); - assert.equal(deps._intervals.length, 0, "poll interval cancelled"); assert.equal(deps._timers.length, 0, "backstop cancelled"); }); @@ -299,7 +267,7 @@ test("backstop fires onBackstop, not onSuccess, and resets state", async () => { // ── Cancellation token ──────────────────────────────────────────────────────── -test("cancellation token — superseded poll callback does not call onSuccess", async () => { +test("cancellation token — superseded attempt does not call onSuccess", async () => { const ctrl = new RelayReconnectController(); let preconnectCallCount = 0; let resolveFirstPreconnect; @@ -375,11 +343,6 @@ test("cancel mid fast-path — preconnect resolves SUCCESSFULLY after cancel but "state is idle", ); assert.equal(deps._timers.length, 0, "no timers outstanding after cancel"); - assert.equal( - deps._intervals.length, - 0, - "no intervals outstanding after cancel", - ); }); test("last subscriber unsubscribe cancels in-flight attempt", async () => { @@ -505,7 +468,7 @@ test("OSS build (hookConfigured returns false) — runHook never called", async // ── Synchronous connected emission ─────────────────────────────────────────── -test("sync connected emission — onSuccess fires once, no interval/backstop installed, subscription cleaned up", async () => { +test("sync connected emission — onSuccess fires once, no backstop installed, subscription cleaned up", async () => { const ctrl = new RelayReconnectController(); // subscribeToConnectionState fake that invokes the listener synchronously @@ -538,7 +501,6 @@ test("sync connected emission — onSuccess fires once, no interval/backstop ins 1, "onSuccess fires exactly once", ); - assert.equal(deps._intervals.length, 0, "no poll interval installed"); assert.equal(deps._timers.length, 0, "no backstop timer installed"); assert.equal(cleanupCalled, true, "subscription cleanup handle was called"); assert.deepEqual( diff --git a/desktop/src/shared/api/relayReconnectController.ts b/desktop/src/shared/api/relayReconnectController.ts index fcea2f29d..b16cc73df 100644 --- a/desktop/src/shared/api/relayReconnectController.ts +++ b/desktop/src/shared/api/relayReconnectController.ts @@ -12,11 +12,9 @@ * returns quickly; any browser-based reconnect flow it triggers runs * asynchronously. * - * 3. **Poll-until-connected** — retry `preconnect()` every POLL_INTERVAL_MS. - * The moment the relay becomes reachable the next poll succeeds and - * success is declared — the backstop is a ceiling, not a delay. The - * connection-state emitter is also watched: if the session's background - * retry loop reconnects first, we catch it immediately. + * 3. **Wait for background reconnect** — observe the session's exponential- + * backoff reconnect loop. Success is declared when its connection-state + * emitter reports connected; the backstop is a UI ceiling, not a delay. * * The controller is a module-level singleton so all hook instances in the * same app share a single in-flight state. A cancellation token is bumped on @@ -32,16 +30,13 @@ export type ReconnectTimingPolicy = { /** Short deadline for the optimistic fast-path attempt. */ fastPathTimeoutMs: number; - /** Interval between poll attempts during phase 3. */ - pollIntervalMs: number; - /** Maximum total time to keep polling before giving up. */ + /** Maximum total time to wait before returning control to the UI. */ backstopMs: number; }; /** Current production reconnect timings. */ export const DEFAULT_RECONNECT_TIMING_POLICY: ReconnectTimingPolicy = { - fastPathTimeoutMs: 4_000, - pollIntervalMs: 3_000, + fastPathTimeoutMs: 11_000, backstopMs: 120_000, }; @@ -61,8 +56,6 @@ export type ReconnectDeps = { onBackstop: () => void; setTimeout: (fn: () => void, ms: number) => number; clearTimeout: (id: number) => void; - setInterval: (fn: () => void, ms: number) => number; - clearInterval: (id: number) => void; }; function withDeadline( @@ -101,14 +94,12 @@ export class RelayReconnectController { // All async continuations capture the token at their creation point and // bail if it has since been superseded. private attemptToken = 0; - // Active timers and subscription for the current attempt. The timer-clear - // functions are stored from the deps of the most recent start() call so - // that cancel() and teardown do not need the caller to supply deps again. - private pollIntervalId: number | null = null; + // Active timer and subscription for the current attempt. The timer-clear + // function is stored from the deps of the most recent start() call so that + // cancel() and teardown do not need the caller to supply deps again. private backstopId: number | null = null; private unsubscribeConnectionState: (() => void) | null = null; private clearTimeoutFn: ((id: number) => void) | null = null; - private clearIntervalFn: ((id: number) => void) | null = null; /** Subscribe to state changes. Fires immediately with the current state. */ subscribe(listener: Listener): () => void { @@ -117,7 +108,7 @@ export class RelayReconnectController { return () => { this.listeners.delete(listener); // Cancel the in-flight attempt when the last subscriber leaves — no UI - // is watching the result, so letting poll/backstop callbacks fire + // is watching the result, so letting backstop callbacks fire // (possibly mutating state or invoking onSuccess/onBackstop into a // stale closure) would be a resource and correctness leak. if (this.listeners.size === 0) { @@ -141,7 +132,6 @@ export class RelayReconnectController { // Store timer-clear fns so cancel() can clear timers without caller deps. this.clearTimeoutFn = deps.clearTimeout; - this.clearIntervalFn = deps.clearInterval; // Bump the cancellation token before any await. const token = ++this.attemptToken; @@ -164,7 +154,7 @@ export class RelayReconnectController { return true; } catch { if (cancelled()) return false; - // Fast path failed — continue to escalation or polling. + // Fast path failed — continue to escalation or background reconnect. } // ── Phase 2: escalation (hook-configured builds only) ─────────────────── @@ -184,7 +174,7 @@ export class RelayReconnectController { await deps.runHook(); } catch (err) { // Non-fatal — hook failure means the browser-based reconnect flow may - // not have opened, but we still poll for relay reachability. + // not have opened, but the session still retries in the background. console.warn( "[RelayReconnectController] transport recovery hook failed:", err, @@ -194,11 +184,10 @@ export class RelayReconnectController { this.setState({ isPending: true, isWaitingOnReconnectHook: true }); } - // ── Phase 3: poll-until-connected ──────────────────────────────────────── - // Retry preconnect on a fixed interval. The moment the relay becomes - // reachable the next poll fires success. The connection-state emitter is - // also watched; if the session's background retry loop reconnects first, - // we catch it here too. + // ── Phase 3: wait for background reconnect ────────────────────────────── + // The fast-path preconnect arms the session's exponential-backoff loop. + // Observe it rather than issuing fixed-cadence attempts that consume its + // pending retry timer. let resolved = false; // Capture onSuccess/onBackstop at phase-3 entry so that cancel() (which @@ -236,21 +225,10 @@ export class RelayReconnectController { return false; } - this.pollIntervalId = deps.setInterval(() => { - if (resolved || cancelled()) return; - void deps - .preconnect() - .then(onConnected) - .catch(() => { - // Poll failed — keep trying. - }); - }, this.timingPolicy.pollIntervalMs); - this.backstopId = deps.setTimeout(() => { if (resolved || cancelled()) return; resolved = true; - // keepAliveRequested remains true via preconnect() — the session's - // background retry loop keeps running. The notification is soft. + // The session's exponential-backoff reconnect loop remains armed. onBackstop(); this.finish(() => {}, false); }, this.timingPolicy.backstopMs); @@ -281,10 +259,6 @@ export class RelayReconnectController { } private cancelTimers(): void { - if (this.pollIntervalId !== null && this.clearIntervalFn !== null) { - this.clearIntervalFn(this.pollIntervalId); - this.pollIntervalId = null; - } if (this.backstopId !== null && this.clearTimeoutFn !== null) { this.clearTimeoutFn(this.backstopId); this.backstopId = null; diff --git a/desktop/src/shared/api/useReconnectRelay.ts b/desktop/src/shared/api/useReconnectRelay.ts index 0dbdb832f..6de4a8282 100644 --- a/desktop/src/shared/api/useReconnectRelay.ts +++ b/desktop/src/shared/api/useReconnectRelay.ts @@ -30,8 +30,6 @@ function buildDeps(onSuccess: () => void, onBackstop: () => void) { onBackstop, setTimeout: window.setTimeout.bind(window), clearTimeout: window.clearTimeout.bind(window), - setInterval: window.setInterval.bind(window), - clearInterval: window.clearInterval.bind(window), }; } From c0d7b52af8d5f3572790be697dd2c4bf3c6a3b8f Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Sat, 25 Jul 2026 11:01:28 -0700 Subject: [PATCH 07/11] docs(contributing): trim to goose-scale minimal intake surface (#2780) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the 5-file, ~180-line template surface with a 4-file, ~54-line goose-modeled set. ## What changed **`.github/PULL_REQUEST_TEMPLATE.md`** — rewritten to 8 lines: Summary, Related issue (with inline duplicate-check prompt), Testing. Checklist and AI-disclosure section removed. **`.github/ISSUE_TEMPLATE/bug-report.yml` → `bug-report.md`** — replaced YAML form with plain-markdown template (goose-style frontmatter). Fields: describe the bug, repro steps, expected behavior, version + OS, logs/context. Version guidance retained: Settings sidebar footer, "unknown" accepted. **`.github/ISSUE_TEMPLATE/feature-request.yml` → `feature-request.md`** — replaced YAML form with plain-markdown template. Fields: motivation, proposed solution, alternatives, additional context. Duplicate-check line at the bottom (goose-style). **`.github/ISSUE_TEMPLATE/question.yml`** — deleted. `config.yml` updated to `blank_issues_enabled: true` so questions have somewhere to go. **`CONTRIBUTING.md`** — "Before You Open a PR" compressed to four prose sentences: duplicate search, issue-first recommendation, AI ownership (absorbs the dropped PR-template field), review cadence. Intro link updated from the removed question form to plain `/issues/new`. ## Related issue none found --------- Signed-off-by: Will Pfleger Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 --- .github/ISSUE_TEMPLATE/bug-report.md | 35 ++++++-------- .github/ISSUE_TEMPLATE/config.yml | 5 +- .github/ISSUE_TEMPLATE/feature-request.md | 21 +++++++++ .github/PULL_REQUEST_TEMPLATE.md | 8 ++++ CONTRIBUTING.md | 57 +++++++++++------------ 5 files changed, 71 insertions(+), 55 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/feature-request.md create mode 100644 .github/PULL_REQUEST_TEMPLATE.md diff --git a/.github/ISSUE_TEMPLATE/bug-report.md b/.github/ISSUE_TEMPLATE/bug-report.md index 25f0bc5ed..e38331345 100644 --- a/.github/ISSUE_TEMPLATE/bug-report.md +++ b/.github/ISSUE_TEMPLATE/bug-report.md @@ -1,31 +1,24 @@ --- -name: 🐛 Bug Report -about: Thank you for taking the time, please report a reproducible bug -title: "[Bug] " +name: Bug report +about: Report a reproducible bug in Buzz labels: bug -assignees: add codeowner's @name here - --- **Describe the bug** -*A clear and concise description of what the bug is.* +A clear and concise description of what the bug is. -**To Reproduce:** -*Steps to reproduce the behavior:* +**Steps to reproduce** 1. Go to '...' -2. Click on '....' -3. Scroll down to '....' -4. See error +2. Click on '...' +3. See error -**Expected behavior:** -*A clear and concise description of what you expected to happen.* +**Expected behavior** +What did you expect to happen? -**Supporting Material** -*If applicable, add screenshots, output log and/or other documentation to help explain your problem.* +**Version and platform** +Find your version at the bottom of the Settings sidebar. Write "unknown" if you can't determine it. +- Buzz version: +- OS: -**Environment (please complete the following information):** - - OS: [ex: iOS] - - Version - -**Additional context** -Add any other context that you feel is relevant about the problem here. +**Logs / additional context** +Paste any relevant logs, error messages, or screenshots here. diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 0ba9db253..0086358db 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,4 +1 @@ -contact_links: - - name: ❓ Questions and Help 🤔 - url: https://discord.gg/block-opensource (/add your discord channel if applicable) - about: This issue tracker is not for support questions. Please refer to the community for more help. +blank_issues_enabled: true diff --git a/.github/ISSUE_TEMPLATE/feature-request.md b/.github/ISSUE_TEMPLATE/feature-request.md new file mode 100644 index 000000000..ba5132f0e --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature-request.md @@ -0,0 +1,21 @@ +--- +name: Feature request +about: Propose a new feature or improvement +labels: enhancement +--- + +**Motivation** +What problem does this solve? Who runs into it and when? + +**Proposed solution** +Describe what you'd like to see. + +**Alternatives considered** +What other approaches did you consider? + +**Additional context** +Anything else that helps — links, screenshots, prior art. + +--- + +Before opening: please [search open issues and PRs](https://github.com/block/buzz/issues?q=is%3Aopen) for duplicates — link the closest one, or say "none found". diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 000000000..325a48510 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,8 @@ +## Summary + + +### Related issue + + +### Testing + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d6278f5eb..565d74012 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,24 +4,24 @@ Welcome, and thank you for your interest in contributing! Buzz is an open-source project and we're glad you're here. This guide will help you get from zero to a merged pull request. -If you have questions that aren't answered here, open a GitHub Discussion or -reach out in the community channels. +If you have questions that aren't answered here, [open an issue](https://github.com/block/buzz/issues/new). --- ## Table of Contents 1. [Code of Conduct](#code-of-conduct) -2. [Setting Up the Development Environment](#setting-up-the-development-environment) -3. [Running Tests](#running-tests) -4. [Code Style](#code-style) -5. [Making a Pull Request](#making-a-pull-request) -6. [Architecture Overview](#architecture-overview) -7. [Ecosystem](#ecosystem) -8. [How to Add a New Event Kind](#how-to-add-a-new-event-kind) -9. [How to Add a New MCP Tool](#how-to-add-a-new-mcp-tool) -10. [How to Add a New API Endpoint](#how-to-add-a-new-api-endpoint) -11. [License and CLA](#license-and-cla) +2. [Before You Open a PR](#before-you-open-a-pr) +3. [Setting Up the Development Environment](#setting-up-the-development-environment) +4. [Running Tests](#running-tests) +5. [Code Style](#code-style) +6. [Making a Pull Request](#making-a-pull-request) +7. [Architecture Overview](#architecture-overview) +8. [Ecosystem](#ecosystem) +9. [How to Add a New Event Kind](#how-to-add-a-new-event-kind) +10. [How to Add a New MCP Tool](#how-to-add-a-new-mcp-tool) +11. [How to Add a New API Endpoint](#how-to-add-a-new-api-endpoint) +12. [License and CLA](#license-and-cla) --- @@ -33,6 +33,20 @@ unacceptable behavior to **conduct@buzz-relay.org**. --- +## Before You Open a PR + +Before starting, search [open PRs](https://github.com/block/buzz/pulls) and [open issues](https://github.com/block/buzz/issues) for duplicates — someone may already be working on the same thing. When you open your PR, link the closest existing one in the description (or say "none found"). + +For anything beyond a small fix, opening an issue first is strongly recommended. Describe the problem and proposed solution so a maintainer can acknowledge the approach before you build — it avoids two people building the same thing in parallel. + +Buzz is an agent platform, so AI-assisted PRs are welcome. No need to disclose the tools you used, but you own and must have reviewed the final code. Submissions that are clearly unreviewed may be closed with a pointer here. + +We squash-merge, so your PR title becomes the commit subject in `main`. Use [Conventional Commits](https://www.conventionalcommits.org/) format: `feat(mcp): add get_feed_actions tool`. The type prefix (`feat`, `fix`, `docs`, `refactor`, `test`, `chore`) is required. See the [Commit Messages](#commit-messages) section for the full reference. + +We review as capacity allows — focused PRs that follow this guide move fastest. + +--- + ## Setting Up the Development Environment ### Prerequisites @@ -248,13 +262,6 @@ required. The scope (in parentheses) is optional but encouraged. ## Making a Pull Request -### Before You Start - -- Check open issues and PRs to avoid duplicate work. -- For significant changes, open an issue first to discuss the approach. -- For small fixes (typos, doc improvements, obvious bugs), go ahead and open - a PR directly. - ### What a Good PR Looks Like 1. **Focused** — one logical change per PR. If you're fixing a bug and @@ -275,19 +282,9 @@ required. The scope (in parentheses) is optional but encouraged. - How to test it manually (if applicable) - Any follow-up work deferred to a future PR -### PR Checklist - -``` -- [ ] `just ci` passes (fmt + clippy + unit tests + mobile) -- [ ] Integration tests pass (`just test`) -- [ ] New public APIs / tools / endpoints are documented -- [ ] No new `unwrap()` in production code paths -- [ ] No new `unsafe` blocks -``` - ### Review Process -- A maintainer will review your PR within a few business days. +- We prioritize focused PRs that follow this guide and review as capacity allows. - Address review comments by pushing new commits (don't force-push during review; it makes it hard to see what changed). - Once approved, a maintainer will squash-merge your PR. From 20bff591023daffc5ee1032cff02b54b75da3567 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Sat, 25 Jul 2026 13:13:41 -0700 Subject: [PATCH 08/11] fix(desktop): track concurrent agent turns up to the harness maximum (#2882) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Desktop's active-turn store capped tracked concurrent turns per agent at `4` while the harness runs `DEFAULT_AGENT_PARALLELISM = 24` parallel agent subprocesses and accepts up to `32` (`--agents` / `BUZZ_ACP_AGENTS`, `value_parser range(1..=32)`). Turns above the cap were silently evicted, so a genuinely-running turn lost its working badge in the sidebar and the agents popover. The eviction also caused the badge set to rotate indefinitely. Evicted turns are still alive, so their hosts keep emitting `turn_liveness` every 10s; `recordActivity` can't find the evicted turn, `resurrectTurn` recreates it, and that eviction drops one of the surviving turns. With two live turns above the cap the visible set churned every 10 seconds. `MAX_TURNS_PER_AGENT` exists only to bound map growth, so it now sits at the harness's hard upper bound of `32` — unreachable for any legitimately-configured agent while still keeping the per-agent map bounded. `MAX_TERMINAL_TOMBSTONES` derives from it (`* 4`), so the tombstone cap moves from 16 to 128. Two regressions cover the reported symptom: a default-parallelism agent working in 24 channels keeps all 24 badges, and the tracked channel set stays stable as `turn_liveness` arrives for turns that previously would have been evicted. Signed-off-by: Will Pfleger Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 --- .../agents/activeAgentTurnsStore.test.mjs | 100 ++++++++++++++---- .../features/agents/activeAgentTurnsStore.ts | 8 +- 2 files changed, 88 insertions(+), 20 deletions(-) diff --git a/desktop/src/features/agents/activeAgentTurnsStore.test.mjs b/desktop/src/features/agents/activeAgentTurnsStore.test.mjs index f182b7b5f..48f735d17 100644 --- a/desktop/src/features/agents/activeAgentTurnsStore.test.mjs +++ b/desktop/src/features/agents/activeAgentTurnsStore.test.mjs @@ -154,25 +154,88 @@ describe("activeAgentTurnsStore", () => { }); describe("eviction at MAX_TURNS_PER_AGENT", () => { - it("evicts oldest turn when exceeding 4 concurrent turns", () => { + // Mirrors the store's private cap: the harness's hard upper bound on + // parallel agent subprocesses (`--agents` accepts 1..=32). + const CAP = 32; + /** Desktop's default harness parallelism (DEFAULT_AGENT_PARALLELISM). */ + const DEFAULT_PARALLELISM = 24; + const EPOCH = Date.parse("2024-01-01T00:00:00Z"); + const at = (ms) => new Date(EPOCH + ms).toISOString(); + + /** One turn_started per channel, minute apart so start order is unambiguous. */ + function startTurns(count, firstSeq = 1) { const events = []; - for (let i = 1; i <= 5; i++) { + for (let i = 1; i <= count; i++) { events.push( makeEvent({ - seq: i, + seq: firstSeq + i - 1, turnId: `t${i}`, channelId: `c${i}`, - timestamp: `2024-01-01T00:0${i}:00Z`, + timestamp: at(i * 60_000), }), ); } - syncAgentTurnsFromEvents(AGENT, events); + return events; + } + + it("evicts oldest turn when exceeding the cap", () => { + syncAgentTurnsFromEvents(AGENT, startTurns(CAP + 1)); const channels = channelIdsOf(getActiveTurnsForAgent(AGENT)); - // Should have evicted c1 (oldest) to make room for c5 - assert.equal(channels.size, 4); + // c1 (oldest) evicted to make room for the 33rd turn. + assert.equal(channels.size, CAP); assert.ok(!channels.has("c1"), "oldest turn should be evicted"); assert.ok(channels.has("c2")); - assert.ok(channels.has("c5")); + assert.ok(channels.has(`c${CAP + 1}`)); + }); + + it("tracks every turn of a default-parallelism agent working in 24 channels", () => { + // DEFAULT_AGENT_PARALLELISM is 24, so a single agent legitimately runs 24 + // concurrent turns. Every one must keep its working badge. + syncAgentTurnsFromEvents(AGENT, startTurns(DEFAULT_PARALLELISM)); + const channels = channelIdsOf(getActiveTurnsForAgent(AGENT)); + assert.equal( + channels.size, + DEFAULT_PARALLELISM, + "all 24 concurrently-worked channels must be tracked", + ); + for (let i = 1; i <= DEFAULT_PARALLELISM; i++) { + assert.ok(channels.has(`c${i}`), `c${i} must be tracked`); + } + }); + + it("keeps the tracked channel set stable as liveness arrives for every turn", () => { + // The flicker: with the cap below real parallelism, turns above it are + // evicted while still alive, and their 10s turn_liveness frames land on + // resurrectTurn — which evicts one of the survivors to make room. The set + // then rotates forever. Under a cap at the harness maximum, liveness for + // any live turn is a plain refresh and the set never moves. + const TURNS = 6; + syncAgentTurnsFromEvents(AGENT, startTurns(TURNS)); + const expected = channelIdsOf(getActiveTurnsForAgent(AGENT)); + + // Liveness for the two earliest-started turns — the first to be evicted + // under the old cap, hence the first to trigger a resurrection swap. + for (const [i, turnId] of ["t1", "t2"].entries()) { + syncAgentTurnsFromEvents(AGENT, [ + makeEvent({ + seq: TURNS + i + 1, + kind: "turn_liveness", + turnId, + channelId: turnId.replace("t", "c"), + timestamp: at((TURNS + i + 1) * 60_000), + }), + ]); + assert.deepEqual( + [...channelIdsOf(getActiveTurnsForAgent(AGENT))].sort(), + [...expected].sort(), + `liveness for ${turnId} must not change the tracked channel set`, + ); + } + assert.equal( + expected.size, + TURNS, + "all six live turns must be tracked, not a rotating subset", + ); }); }); @@ -1343,16 +1406,17 @@ describe("activeAgentTurnsStore", () => { }); it("evicts the oldest tombstone once past the cap so the map stays bounded", () => { - // The tombstone map is capped at MAX_TERMINAL_TOMBSTONES (16). Complete - // 18 distinct turns so eviction fires twice, dropping the two oldest by - // insertion order (t0, t1). Probe via the ONE behavior a tombstone gates - // that a strictly-newer frame cannot mask: an EQUAL-timestamp liveness - // (frameAt == terminalAt). All completions share timestamp T with rising - // seq, so the probe clears the per-agent watermark on the seq tiebreak - // (compareObserverEvents is timestamp-primary, seq-secondary) yet stays - // equal to the recorded terminal — reaching resurrectTurn's tombstone - // check rather than being shadowed by the watermark. - const CAP = 16; + // The tombstone map is capped at MAX_TERMINAL_TOMBSTONES (MAX_TURNS_PER_AGENT + // * 4 = 128). Complete 130 distinct turns so eviction fires twice, dropping + // the two oldest by insertion order (t0, t1). Probe via the ONE behavior a + // tombstone gates that a strictly-newer frame cannot mask: an + // EQUAL-timestamp liveness (frameAt == terminalAt). All completions share + // timestamp T with rising seq, so the probe clears the per-agent watermark + // on the seq tiebreak (compareObserverEvents is timestamp-primary, + // seq-secondary) yet stays equal to the recorded terminal — reaching + // resurrectTurn's tombstone check rather than being shadowed by the + // watermark. + const CAP = 128; const TOTAL = CAP + 2; const T = at(0); const completions = []; diff --git a/desktop/src/features/agents/activeAgentTurnsStore.ts b/desktop/src/features/agents/activeAgentTurnsStore.ts index b28192950..3af41ae07 100644 --- a/desktop/src/features/agents/activeAgentTurnsStore.ts +++ b/desktop/src/features/agents/activeAgentTurnsStore.ts @@ -23,8 +23,12 @@ const REMOVE_AFTER_MS = LIVENESS_INTERVAL_MS * 2.5; const FRAME_GAP_PAUSE_MS = LIVENESS_INTERVAL_MS * 2; /** A silent agent is treated as dead after this bounded prune pause. */ const PRUNE_PAUSE_MAX_MS = 3 * 60_000; -/** Maximum concurrent active turns tracked per agent (matches pool size). */ -const MAX_TURNS_PER_AGENT = 4; +/** Maximum concurrent active turns tracked per agent. Purely an unbounded-growth + * guard, so it sits at the harness's hard upper bound for parallel agent + * subprocesses (`--agents` / `BUZZ_ACP_AGENTS` accepts `1..=32`) rather than the + * Desktop default of 24 — any lower value silently evicts a live turn, dropping + * its working badge. */ +const MAX_TURNS_PER_AGENT = 32; /** Cap on per-agent terminal tombstones (A's resurrection guard). Only the * most recently completed turns can be raced by a late liveness frame; older * ones are already below the watermark, so a small multiple of the live cap is From c7089d3b52a6758596cb516f7b3e65989428d26b Mon Sep 17 00:00:00 2001 From: Mager Date: Sat, 25 Jul 2026 17:22:18 -0500 Subject: [PATCH 09/11] docs(buzz-acp): correct agent key generation instructions (#2875) --- crates/buzz-acp/README.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/buzz-acp/README.md b/crates/buzz-acp/README.md index d9cd362cb..d863729d5 100644 --- a/crates/buzz-acp/README.md +++ b/crates/buzz-acp/README.md @@ -25,13 +25,22 @@ export PATH="$PWD/target/release:$PATH" ## Generating Keys -Each agent needs a Nostr keypair — this is the agent's identity in Buzz. Use `buzz-admin` to mint one: +Each agent needs a Nostr keypair — this is the agent's identity in Buzz. Use `buzz-admin` to generate one: ```bash -cargo run -p buzz-admin -- mint-token --name "my-agent" --scopes "messages:read,messages:write,channels:read" +cargo run -p buzz-admin -- generate-key ``` -This prints an `nsec1...` private key and an API token. **Save both immediately — they're shown only once.** +This prints a public and secret key pair as hex. **Save the secret key immediately — it is not stored and cannot be recovered.** Set `BUZZ_PRIVATE_KEY` to the secret key to act as this identity. + +Then register the agent's public key as a relay member so it can read and publish: + +```bash +BUZZ_RELAY_PRIVATE_KEY= \ + cargo run -p buzz-admin -- add-member --pubkey +``` + +`add-member` publishes a kind:13534 membership event, so the relay needs a stable signing key: set `BUZZ_RELAY_PRIVATE_KEY` in the relay's environment (uncomment it in `.env`) and restart the relay before running this. > **Running multiple agents?** Mint a separate keypair for each. Every agent needs its own identity. From 25e7864b35f4dfd1c0ff31304a38555230a85f8d Mon Sep 17 00:00:00 2001 From: Wes Date: Sat, 25 Jul 2026 16:49:56 -0700 Subject: [PATCH 10/11] fix(desktop): consolidate prepend scroll correction (#2855) ## Summary - make Virtua the sole prepend geometry/correction owner by accumulating active prepend ResizeObserver corrections from the live DOM offset - retire prepend reconciliation on ordinary reader wheel input, while preserving it for Ctrl+wheel browser zoom - remove Buzz's competing three-second semantic-anchor watcher and corrective `scrollBy` loop - keep ESM/CJS Virtua patch behavior equivalent and update the patch lock hash ## Why Buzz admitted prepended rows using seeded estimates, then Virtua received multiple measurement corrections for the same transaction. Each correction was based on the same stale model offset, so a later write replaced an earlier correction instead of accumulating it. In the reproduced first page, that resurrected 452px of anchor drift; the app-level watcher merely corrected the lost virtualizer write afterward. This fixes the correction inside Virtua and deletes the competing app writer, following the single-owner geometry invariant used by Berd rather than copying its spacer implementation. ## Validation - watcher-off desktop virtualization matrix: 11/11 passed, including 15 cascading prepends, continued wheel input, detached rich-row growth, channel switching, bottom follow, and buffered live arrivals - focused cascading prepend/Ctrl+wheel regression passed - desktop typecheck passed - desktop unit suite passed: 3,495 tests - Biome passed on changed desktop files - `git diff --check` clean ## Manual behavior Load older history repeatedly while scrolling upward, then wheel downward during/after a prepend. The visible anchor should stay within the existing 5px contract during reconciliation, and deliberate reader movement should not be pulled back. Ctrl+wheel during the prepend commit must not cancel reconciliation. --------- Signed-off-by: Wes Co-authored-by: Carl Co-authored-by: Carl --- .../features/messages/ui/MessageTimeline.tsx | 1 + .../messages/ui/TimelineMessageList.tsx | 164 ++-------- .../ui/useAnchoredScroll.lifecycle.test.mjs | 115 +++++++ .../features/messages/ui/useAnchoredScroll.ts | 12 + .../ui/useSettleGatedPrependMessages.ts | 14 +- .../messages/ui/useUpwardPaginationWheel.ts | 3 + .../ui/useVirtualizedBottomSettle.test.mjs | 280 ++++++++++++++++++ .../messages/ui/useVirtualizedBottomSettle.ts | 140 ++++++--- .../messages/ui/virtuaWheelModePatch.test.mjs | 27 ++ desktop/tests/e2e/scroll-history.spec.ts | 76 ++++- desktop/tests/e2e/smoke.spec.ts | 3 + desktop/tests/e2e/timeline-no-shift.spec.ts | 1 + desktop/tests/e2e/virtualization.spec.ts | 39 ++- patches/virtua@0.49.3.patch | 77 ++++- pnpm-lock.yaml | 6 +- 15 files changed, 744 insertions(+), 214 deletions(-) create mode 100644 desktop/src/features/messages/ui/useVirtualizedBottomSettle.test.mjs create mode 100644 desktop/src/features/messages/ui/virtuaWheelModePatch.test.mjs diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index b66c97619..73526da5e 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -335,6 +335,7 @@ const MessageTimelineBase = React.forwardRef< scrollContainerRef: activeScrollContainerRef, splitPanelOpen: splitThreadPanelOpen, targetMessageId, + virtualCancelBottomIntent: timelineVirtualizerApi?.cancelBottomIntent, virtualScrollToMessage: timelineVirtualizerApi?.scrollToMessage, virtualScrollToBottom: timelineVirtualizerApi?.scrollToBottom, virtualSettleAtBottom: timelineVirtualizerApi?.settleAtBottom, diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index 5d3cf8fd5..3c35cb6d4 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -42,6 +42,7 @@ import { useUpwardPaginationWheel } from "./useUpwardPaginationWheel"; import { useVirtualizedBottomSettle } from "./useVirtualizedBottomSettle"; export type TimelineVirtualizerApi = { + cancelBottomIntent: () => void; scrollToBottom: (behavior?: ScrollBehavior) => void; settleAtBottom: () => void; scrollToMessage: ( @@ -454,145 +455,39 @@ function VirtualizedTimelineRows({ const keys = React.useMemo(() => items.map(virtualizedItemKey), [items]); itemsLengthRef.current = items.length; const previousKeysRef = React.useRef([]); - const prependAnchorRef = React.useRef<{ - itemKey: string; - top: number; - } | null>(null); - const prependWatcherFrameRef = React.useRef(null); const [prependShiftEpoch, clearPrependShift] = React.useReducer( (version: number) => version + 1, 0, ); - // Virtua's `shift` is a one-render instruction, not a persistent mode. If it - // stays true after a prepend, later measurement changes can keep anchoring - // from the end and leave a stale blank range until the next scroll event. + const { cancel: cancelBottomSettle, settle: settleAtBottom } = + useVirtualizedBottomSettle(hostRef, listRef, itemsLengthRef); + const { arm: armUpwardMomentum } = useUpwardPaginationWheel( + hostRef, + cancelBottomSettle, + ); + + React.useEffect( + () => () => { + cancelBottomSettle(); + }, + [cancelBottomSettle], + ); + const isPrepend = React.useMemo(() => { void prependShiftEpoch; return didPrependVirtualizedTimeline(previousKeysRef.current, keys); }, [keys, prependShiftEpoch]); - const retirePrependAnchor = React.useCallback(() => { - if (prependWatcherFrameRef.current !== null) { - cancelAnimationFrame(prependWatcherFrameRef.current); - } - prependWatcherFrameRef.current = null; - prependAnchorRef.current = null; - }, []); - const { cancel: cancelBottomSettle, settle: settleAtBottom } = - useVirtualizedBottomSettle(hostRef, listRef, itemsLengthRef); - const retireTimelineSettle = React.useCallback(() => { - retirePrependAnchor(); - cancelBottomSettle(); - }, [cancelBottomSettle, retirePrependAnchor]); - const { arm: armUpwardMomentum, clear: clearUpwardMomentum } = - useUpwardPaginationWheel(hostRef, retireTimelineSettle); - - const capturePrependAnchor = React.useCallback(() => { - // Keep the pending capture current while the fetch is in flight. Once the - // prepend commits and the watcher starts, its baseline is frozen. - if (prependWatcherFrameRef.current !== null) return; - const scroller = hostRef.current?.firstElementChild; - if (!(scroller instanceof HTMLDivElement)) return; - const scrollerTop = scroller.getBoundingClientRect().top; - const row = Array.from( - scroller.querySelectorAll("[data-timeline-item-key]"), - ).find( - (candidate) => candidate.getBoundingClientRect().top >= scrollerTop - 1, - ); - const itemKey = row?.dataset.timelineItemKey; - if (!row || !itemKey) return; - prependAnchorRef.current = { - itemKey, - top: row.getBoundingClientRect().top - scrollerTop, - }; - }, []); - - React.useLayoutEffect(() => { - if (!isPrepend || !prependAnchorRef.current) return; - // Virtua's shift mode correctly absorbs prepended measurements, but - // estimated offsets can misclassify late row growth deep in history. Keep - // the semantic row identity as a short-lived, deviation-gated backstop. - // Do not correct in this commit: Virtua has shifted its estimate but has not - // applied its ResizeObserver batch yet, so that delta is transient and its - // subsequent absolute correction would overwrite our relative write. - // This watcher deliberately survives a temporary row unmount and waits for - // stable geometry so Virtua remains the primary scroll owner. - if (prependWatcherFrameRef.current !== null) { - cancelAnimationFrame(prependWatcherFrameRef.current); - } - const anchor = prependAnchorRef.current; - const deadline = performance.now() + 3_000; - let previousScrollTop: number | null = null; - let settledFrames = 0; - - const watch = () => { - const scroller = hostRef.current?.firstElementChild; - if (!(scroller instanceof HTMLDivElement)) { - prependWatcherFrameRef.current = null; - prependAnchorRef.current = null; - return; - } - const atBottom = - scroller.scrollHeight - scroller.clientHeight - scroller.scrollTop <= - 32; - const row = Array.from( - scroller.querySelectorAll("[data-timeline-item-key]"), - ).find( - (candidate) => candidate.dataset.timelineItemKey === anchor.itemKey, - ); - const top = row - ? row.getBoundingClientRect().top - scroller.getBoundingClientRect().top - : null; - const scrollTop = scroller.scrollTop; - settledFrames = - previousScrollTop !== null && - Math.abs(scrollTop - previousScrollTop) < 0.5 - ? settledFrames + 1 - : 0; - previousScrollTop = scrollTop; - - if (row && top !== null && settledFrames >= 2) { - const delta = top - anchor.top; - if (Math.abs(delta) > 4) { - scroller.scrollBy({ top: delta }); - settledFrames = 0; - previousScrollTop = null; - } - } - - const retired = - performance.now() >= deadline || - atBottom || - (top !== null && top > scroller.clientHeight * 2); - if (retired) { - retirePrependAnchor(); - return; - } - prependWatcherFrameRef.current = requestAnimationFrame(watch); - }; - prependWatcherFrameRef.current = requestAnimationFrame(watch); - clearUpwardMomentum(); - }, [clearUpwardMomentum, isPrepend, retirePrependAnchor]); - - React.useEffect( - () => () => { - retirePrependAnchor(); - cancelBottomSettle(); - }, - [cancelBottomSettle, retirePrependAnchor], - ); - React.useLayoutEffect(() => { previousKeysRef.current = keys; if (isPrepend) { - cancelBottomSettle(); clearPrependShift(); } if (!hasInitialPositionedRef.current && items.length > 0) { hasInitialPositionedRef.current = true; settleAtBottom(); } - }, [cancelBottomSettle, isPrepend, items.length, keys, settleAtBottom]); + }, [isPrepend, items.length, keys, settleAtBottom]); const messageItemIndexById = React.useMemo(() => { const byId = new Map(); @@ -622,16 +517,13 @@ function VirtualizedTimelineRows({ React.useLayoutEffect(() => { if (!onVirtualizerApiChange) return; const api: TimelineVirtualizerApi = { + cancelBottomIntent: cancelBottomSettle, scrollToBottom() { - retireTimelineSettle(); - const lastIndex = itemsLengthRef.current - 1; - if (lastIndex >= 0) { - listRef.current?.scrollToIndex(lastIndex, { align: "end" }); - } + settleAtBottom(); }, settleAtBottom, scrollToMessage(messageId) { - retireTimelineSettle(); + cancelBottomSettle(); const index = messageItemIndexByIdRef.current.get(messageId); if (index === undefined) return false; listRef.current?.scrollToIndex(index, { align: "center" }); @@ -640,7 +532,7 @@ function VirtualizedTimelineRows({ }; onVirtualizerApiChange(api); return () => onVirtualizerApiChange(null); - }, [onVirtualizerApiChange, retireTimelineSettle, settleAtBottom]); + }, [cancelBottomSettle, onVirtualizerApiChange, settleAtBottom]); React.useLayoutEffect(() => { const host = hostRef.current; @@ -670,15 +562,13 @@ function VirtualizedTimelineRows({ if (!list || !(scroller instanceof HTMLDivElement)) return; onVirtualizerRangeChanged?.(); const distanceFromBottom = list.scrollSize - list.viewportSize - offset; - if (distanceFromBottom > 32) cancelBottomSettle(); + // Do not infer reader intent from an intermediate virtualizer offset. + // Initial channel positioning deliberately chases the floor while rows + // are measured; those measurements can briefly report a large gap and + // emit `onScroll` without any user input. Cancelling here strands the + // channel above its newest message. The settle hook's wheel, pointer, + // touch, and key listeners are the authoritative user-interaction gate. onAtBottomStateChange?.(distanceFromBottom <= 32); - if ( - prependAnchorRef.current !== null || - offset <= 200 || - prependWatcherFrameRef.current === null - ) { - capturePrependAnchor(); - } if (offset <= 200) { // Layout scrolls near the top must not poison the reader's next input. armUpwardMomentum(onStartReached?.() ?? false); @@ -686,8 +576,6 @@ function VirtualizedTimelineRows({ }, [ armUpwardMomentum, - cancelBottomSettle, - capturePrependAnchor, onAtBottomStateChange, onStartReached, onVirtualizerRangeChanged, diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs index 44854c0d5..2df5a22ef 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs +++ b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs @@ -48,6 +48,10 @@ function installDOMShim() { return this.children[0] ?? null; } + get firstElementChild() { + return this.children[0] ?? null; + } + get lastChild() { return this.children.at(-1) ?? null; } @@ -122,8 +126,16 @@ function installDOMShim() { } globalThis.document = new DocumentShim(); + const windowEvents = new EventTargetShim(); + globalThis.addEventListener = + windowEvents.addEventListener.bind(windowEvents); + globalThis.removeEventListener = + windowEvents.removeEventListener.bind(windowEvents); + globalThis.dispatchEvent = windowEvents.dispatchEvent.bind(windowEvents); globalThis.HTMLIFrameElement = NodeShim; + globalThis.HTMLDivElement = NodeShim; globalThis.HTMLElement = NodeShim; + globalThis.Node = NodeShim; globalThis.IS_REACT_ACT_ENVIRONMENT = true; process.env.IS_REACT_ACT_ENVIRONMENT = "true"; Object.defineProperty(globalThis, "window", { @@ -142,6 +154,7 @@ import { act } from "react"; import { createRoot } from "react-dom/client"; import { useAnchoredScroll } from "./useAnchoredScroll.ts"; +import { useVirtualizedBottomSettle } from "./useVirtualizedBottomSettle.ts"; function makePinnedCenterNodes() { const resizeObservers = []; @@ -232,6 +245,32 @@ function Harness({ channelId, refs }) { return null; } +function VirtualTargetHarness({ refs }) { + const didRun = React.useRef(false); + const bottomApi = useVirtualizedBottomSettle( + refs.host, + refs.list, + refs.itemsLength, + ); + const anchored = useAnchoredScroll({ + channelId: "conversation", + contentRef: refs.content, + isLoading: false, + messages: Array.from({ length: 5 }, (_, index) => ({ id: String(index) })), + scrollContainerRef: refs.scroller, + virtualCancelBottomIntent: bottomApi.cancel, + virtualizerOwnsPrependAnchoring: true, + virtualScrollToMessage: () => true, + }); + React.useLayoutEffect(() => { + if (didRun.current) return; + didRun.current = true; + bottomApi.settle(); + refs.targetResult.current = anchored.scrollToMessage("selected"); + }, [anchored.scrollToMessage, bottomApi.settle, refs.targetResult]); + return null; +} + test("channel change attaches pinned-center observers after refs mount", async () => { const refs = { container: { current: null }, @@ -272,3 +311,79 @@ test("channel change attaches pinned-center observers after refs mount", async ( root.unmount(); }); }); + +test("mounted virtual target retires bottom intent before direct centering", async () => { + const resizeObservers = []; + globalThis.ResizeObserver = class { + constructor(callback) { + this.callback = callback; + this.targets = []; + resizeObservers.push(this); + } + disconnect() {} + observe(target) { + this.targets.push(target); + } + }; + + const content = document.createElement("div"); + const scroller = document.createElement("div"); + const host = document.createElement("div"); + scroller.appendChild(content); + host.appendChild(scroller); + scroller.clientHeight = 400; + scroller.scrollHeight = 1_000; + scroller.scrollTop = 0; + scroller.getBoundingClientRect = () => ({ bottom: 400, top: 0 }); + const targetContentTop = 250; + const row = { + getBoundingClientRect: () => ({ + bottom: targetContentTop - scroller.scrollTop + 40, + height: 40, + top: targetContentTop - scroller.scrollTop, + }), + }; + scroller.querySelector = () => row; + scroller.querySelectorAll = () => []; + scroller.scrollTo = ({ top }) => { + scroller.scrollTop = top; + }; + + const bottomWrites = []; + const refs = { + content: { current: content }, + host: { current: host }, + itemsLength: { current: 5 }, + list: { + current: { + scrollToIndex: (index, options) => + bottomWrites.push({ index, options }), + }, + }, + scroller: { current: scroller }, + targetResult: { current: null }, + }; + const root = createRoot(document.createElement("div")); + await act(async () => { + root.render(React.createElement(VirtualTargetHarness, { refs })); + }); + + assert.deepEqual(bottomWrites, [{ index: 4, options: { align: "end" } }]); + assert.equal(refs.targetResult.current, true); + assert.equal(row.getBoundingClientRect().top, 180); + const bottomGeometryObserver = resizeObservers.find( + (observer) => + observer.targets.includes(content) && observer.targets.includes(scroller), + ); + assert.ok(bottomGeometryObserver); + bottomGeometryObserver.callback(); + await act(async () => new Promise((resolve) => setTimeout(resolve, 0))); + + assert.equal( + row.getBoundingClientRect().top, + 180, + "target remains centered after later virtual geometry activity", + ); + assert.equal(bottomWrites.length, 1, "geometry cannot re-pin to bottom"); + await act(async () => root.unmount()); +}); diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index b865260de..34929fc26 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -109,6 +109,7 @@ type UseAnchoredScrollOptions = { /** Keeps a targeted message centered until the user deliberately scrolls. */ pinTargetCentered?: boolean; onTargetReached?: (messageId: string) => void; + virtualCancelBottomIntent?: () => void; virtualScrollToMessage?: ( messageId: string, options?: { behavior?: ScrollBehavior }, @@ -229,6 +230,7 @@ export function useAnchoredScroll({ highlightTargetMessage = true, pinTargetCentered = false, onTargetReached, + virtualCancelBottomIntent, virtualScrollToMessage, virtualScrollToBottom, virtualSettleAtBottom, @@ -439,6 +441,10 @@ export function useAnchoredScroll({ `[data-message-id="${messageId}"]`, ); if (virtualizerOwnsPrependAnchoring && virtualScrollToMessage) { + // Target navigation owns the viewport before any movement strategy is + // chosen. The already-mounted fast path centers the DOM node directly + // and would otherwise leave durable bottom intent armed. + virtualCancelBottomIntent?.(); if (el) { const rect = el.getBoundingClientRect(); const containerRect = container.getBoundingClientRect(); @@ -534,6 +540,7 @@ export function useAnchoredScroll({ highlightMessage, pinTargetCentered, scrollContainerRef, + virtualCancelBottomIntent, virtualizerOwnsPrependAnchoring, writePinnedCenterScroll, virtualScrollToMessage, @@ -607,6 +614,11 @@ export function useAnchoredScroll({ // to the requested target message, or to the bottom by default. if (!hasInitializedRef.current) { if (isLoading) return; + // The virtualized list owns the actual scroll node. Its API registers in + // a child layout effect, after this parent hook's first pass; treating + // that API-less pass as initialized writes to the inert outer wrapper + // and permanently consumes the channel's initial bottom pin. + if (virtualizerOwnsPrependAnchoring && !virtualScrollToBottom) return; // Establish the initial position before the browser paints. The follow-up // frame is a settling pass for content whose measurements land with the // commit (fonts, deferred rows, media), not the first bottom pin. Keeping diff --git a/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts b/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts index 84663e5da..bad8d0f04 100644 --- a/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts +++ b/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts @@ -4,13 +4,13 @@ import * as React from "react"; * Holds an older-history prepend out of the rendered timeline until the * scroller is genuinely at rest, then admits it atomically. * - * Why: every prepend-compensation mechanism in this design — Virtua's shift - * correction, the pre-paint scrollBy, the semantic-anchor watcher — is a - * scrollTop write. On macOS WKWebView those writes can be dropped or - * overridden while trackpad momentum owns the committed offset, so a page - * commit that lands mid-fling displaces the viewport by the full prepended - * height with no reliable way to correct it. Committing only at rest keeps - * all three writers operating in the regime where they are exact. + * Why: Virtua reconciles an active prepend by correcting scrollTop as the + * inserted rows are measured. On macOS WKWebView those corrections can be + * dropped or overridden while trackpad momentum owns the committed offset, so + * a page admitted mid-fling can displace the viewport by the full prepended + * height. Admitting only at rest gives Virtua a stable geometry window in + * which to complete that reconciliation exactly; subsequent reader wheel input + * then retires it. * * The fetched store stays authoritative and fetches still start immediately; * this hook only delays when the fetched page joins the rendered snapshot. diff --git a/desktop/src/features/messages/ui/useUpwardPaginationWheel.ts b/desktop/src/features/messages/ui/useUpwardPaginationWheel.ts index 5c3c4fb22..910b251ac 100644 --- a/desktop/src/features/messages/ui/useUpwardPaginationWheel.ts +++ b/desktop/src/features/messages/ui/useUpwardPaginationWheel.ts @@ -15,6 +15,9 @@ export function useUpwardPaginationWheel( if (!(scroller instanceof HTMLDivElement)) return; let releaseTimer: number | null = null; const handleWheel = (event: WheelEvent) => { + // Ctrl+wheel belongs to browser zoom. It must not retire bottom intent or + // arm upward-pagination momentum because it does not move the reader. + if (event.ctrlKey) return; onWheel(); if (event.deltaY >= 0) { clear(); diff --git a/desktop/src/features/messages/ui/useVirtualizedBottomSettle.test.mjs b/desktop/src/features/messages/ui/useVirtualizedBottomSettle.test.mjs new file mode 100644 index 000000000..bc0aebe91 --- /dev/null +++ b/desktop/src/features/messages/ui/useVirtualizedBottomSettle.test.mjs @@ -0,0 +1,280 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +class EventTargetShim { + listeners = new Map(); + addEventListener(type, listener) { + this.listeners.set(type, [...(this.listeners.get(type) ?? []), listener]); + } + removeEventListener(type, listener) { + this.listeners.set( + type, + (this.listeners.get(type) ?? []).filter( + (current) => current !== listener, + ), + ); + } + dispatchEvent(event) { + for (const listener of this.listeners.get(event.type) ?? []) + listener(event); + return true; + } +} + +class ElementShim extends EventTargetShim { + constructor(firstElementChild = null) { + super(); + this.firstElementChild = firstElementChild; + this.children = []; + this.childNodes = []; + this.isContentEditable = false; + this.nodeName = "DIV"; + this.tagName = "DIV"; + this.nodeType = 1; + this.namespaceURI = "http://www.w3.org/1999/xhtml"; + } + get ownerDocument() { + return globalThis.document; + } + appendChild(child) { + this.children.push(child); + this.childNodes.push(child); + return child; + } + removeChild(child) { + this.children = this.children.filter((current) => current !== child); + this.childNodes = this.childNodes.filter((current) => current !== child); + return child; + } + insertBefore(child, reference) { + const index = this.children.indexOf(reference); + if (index < 0) return this.appendChild(child); + this.children.splice(index, 0, child); + this.childNodes.splice(index, 0, child); + return child; + } + closest() { + return null; + } + contains(target) { + return this === target || this.firstElementChild?.contains(target) === true; + } +} + +globalThis.document = { + addEventListener() {}, + createElement: () => new ElementShim(), + get defaultView() { + return globalThis.window; + }, + nodeType: 9, + removeEventListener() {}, +}; + +const animationFrames = new Map(); +let nextAnimationFrameId = 1; +globalThis.requestAnimationFrame = (callback) => { + const id = nextAnimationFrameId++; + animationFrames.set(id, callback); + return id; +}; +globalThis.cancelAnimationFrame = (id) => animationFrames.delete(id); +globalThis.HTMLElement = ElementShim; +globalThis.HTMLDivElement = ElementShim; +globalThis.Node = ElementShim; +globalThis.IS_REACT_ACT_ENVIRONMENT = true; +process.env.IS_REACT_ACT_ENVIRONMENT = "true"; +Object.defineProperty(globalThis, "window", { + configurable: true, + value: new EventTargetShim(), +}); +globalThis.window.HTMLIFrameElement = ElementShim; + +const resizeObservers = []; +globalThis.ResizeObserver = class { + constructor(callback) { + this.callback = callback; + resizeObservers.push(this); + } + disconnect() {} + observe(target) { + this.targets = [...(this.targets ?? []), target]; + } +}; + +import React from "react"; +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { useVirtualizedBottomSettle } from "./useVirtualizedBottomSettle.ts"; + +function flushAnimationFrames() { + const callbacks = [...animationFrames.values()]; + animationFrames.clear(); + for (const callback of callbacks) callback(performance.now()); +} + +function Harness({ apiRef, hostRef, itemsLengthRef, listRef }) { + apiRef.current = useVirtualizedBottomSettle(hostRef, listRef, itemsLengthRef); + return null; +} + +async function mountHarness() { + animationFrames.clear(); + resizeObservers.length = 0; + const content = new ElementShim(); + const scroller = new ElementShim(content); + const host = new ElementShim(scroller); + const writes = []; + const refs = { + api: { current: null }, + host: { current: host }, + itemsLength: { current: 5 }, + list: { + current: { + scrollToIndex(index, options) { + writes.push({ index, options }); + }, + }, + }, + }; + const root = createRoot(new ElementShim()); + await act(async () => { + root.render( + React.createElement(Harness, { + apiRef: refs.api, + hostRef: refs.host, + itemsLengthRef: refs.itemsLength, + listRef: refs.list, + }), + ); + }); + return { content, refs, root, scroller, writes }; +} + +test("bottom intent follows arbitrarily late virtual geometry changes", async () => { + const { content, refs, root, scroller, writes } = await mountHarness(); + refs.api.current.settle(); + assert.deepEqual(writes, [{ index: 4, options: { align: "end" } }]); + + const geometryObserver = resizeObservers.find((observer) => + observer.targets?.includes(content), + ); + assert.ok(geometryObserver); + geometryObserver.callback(); + flushAnimationFrames(); + assert.equal(writes.length, 2); + + // There is no deadline: another geometry change much later still re-pins. + geometryObserver.callback(); + flushAnimationFrames(); + assert.equal(writes.length, 3); + + // Viewport changes also move the physical floor without resizing content. + assert.ok(geometryObserver.targets.includes(scroller)); + geometryObserver.callback(); + flushAnimationFrames(); + assert.equal(writes.length, 4); + await act(async () => root.unmount()); +}); + +for (const eventType of ["pointerdown", "touchmove", "wheel", "keydown"]) { + test(`${eventType} transfers ownership away from bottom intent`, async () => { + const { content, refs, root, scroller, writes } = await mountHarness(); + refs.api.current.settle(); + const target = eventType === "keydown" ? window : scroller; + target.dispatchEvent({ + altKey: false, + ctrlKey: false, + key: eventType === "keydown" ? "PageUp" : undefined, + metaKey: false, + target: + eventType === "keydown" + ? scroller + : eventType === "pointerdown" + ? scroller + : undefined, + type: eventType, + }); + + resizeObservers + .find((observer) => observer.targets?.includes(content)) + .callback(); + flushAnimationFrames(); + assert.equal(writes.length, 1); + await act(async () => root.unmount()); + assert.equal( + scroller.listeners.get(eventType)?.length ?? 0, + 0, + `${eventType} listener is removed on unmount`, + ); + }); +} + +test("descendant interaction and outside navigation preserve bottom intent", async () => { + const { content, refs, root, scroller, writes } = await mountHarness(); + refs.api.current.settle(); + const rowControl = new ElementShim(); + scroller.firstElementChild.firstElementChild = rowControl; + + scroller.dispatchEvent({ target: rowControl, type: "pointerdown" }); + window.dispatchEvent({ + altKey: false, + ctrlKey: false, + key: "PageUp", + metaKey: false, + target: new ElementShim(), + type: "keydown", + }); + resizeObservers + .find((observer) => observer.targets?.includes(content)) + .callback(); + flushAnimationFrames(); + + assert.equal(writes.length, 2); + await act(async () => root.unmount()); +}); + +test("Ctrl+wheel zoom preserves bottom intent through geometry reflow", async () => { + const { content, refs, root, scroller, writes } = await mountHarness(); + refs.api.current.settle(); + + scroller.dispatchEvent({ ctrlKey: true, deltaY: -100, type: "wheel" }); + resizeObservers + .find((observer) => observer.targets?.includes(content)) + .callback(); + flushAnimationFrames(); + + assert.equal(writes.length, 2); + await act(async () => root.unmount()); +}); + +test("typing and editable navigation keys preserve bottom intent", async () => { + const { content, refs, root, writes } = await mountHarness(); + refs.api.current.settle(); + + window.dispatchEvent({ + altKey: false, + ctrlKey: false, + key: "a", + metaKey: false, + target: null, + type: "keydown", + }); + const editable = new ElementShim(); + editable.isContentEditable = true; + window.dispatchEvent({ + altKey: false, + ctrlKey: false, + key: "ArrowUp", + metaKey: false, + target: editable, + type: "keydown", + }); + + resizeObservers + .find((observer) => observer.targets?.includes(content)) + .callback(); + flushAnimationFrames(); + assert.equal(writes.length, 2); + await act(async () => root.unmount()); +}); diff --git a/desktop/src/features/messages/ui/useVirtualizedBottomSettle.ts b/desktop/src/features/messages/ui/useVirtualizedBottomSettle.ts index 2ff91de99..8c0535779 100644 --- a/desktop/src/features/messages/ui/useVirtualizedBottomSettle.ts +++ b/desktop/src/features/messages/ui/useVirtualizedBottomSettle.ts @@ -1,67 +1,129 @@ import * as React from "react"; import type { VListHandle } from "virtua"; -const BOTTOM_EPSILON_PX = 1; -const SETTLE_DEADLINE_MS = 250; +const SCROLL_INTENT_KEYS = new Set([ + "ArrowDown", + "ArrowUp", + "End", + "Home", + "PageDown", + "PageUp", + " ", +]); + +function isEditableKeyboardTarget(target: EventTarget | null) { + if (!(target instanceof HTMLElement)) return false; + if (target.isContentEditable) return true; + return ( + target.closest("input, textarea, select, [contenteditable='true']") !== null + ); +} export function useVirtualizedBottomSettle( hostRef: React.RefObject, listRef: React.RefObject, itemsLengthRef: React.RefObject, ) { + const bottomIntentRef = React.useRef(false); const frameRef = React.useRef(null); - const cancel = React.useCallback(() => { + + const cancelFrame = React.useCallback(() => { if (frameRef.current !== null) { cancelAnimationFrame(frameRef.current); frameRef.current = null; } }, []); + const cancel = React.useCallback(() => { + bottomIntentRef.current = false; + cancelFrame(); + }, [cancelFrame]); + + const pinToBottom = React.useCallback(() => { + if (!bottomIntentRef.current) return; + const scroller = hostRef.current?.firstElementChild; + const lastIndex = itemsLengthRef.current - 1; + if (!(scroller instanceof HTMLDivElement) || lastIndex < 0) return; + listRef.current?.scrollToIndex(lastIndex, { align: "end" }); + }, [hostRef, itemsLengthRef, listRef]); + + const schedulePinToBottom = React.useCallback(() => { + if (!bottomIntentRef.current || frameRef.current !== null) return; + frameRef.current = requestAnimationFrame(() => { + frameRef.current = null; + pinToBottom(); + }); + }, [pinToBottom]); + React.useLayoutEffect(() => { const scroller = hostRef.current?.firstElementChild; if (!(scroller instanceof HTMLDivElement)) return; const retire = () => cancel(); - scroller.addEventListener("pointerdown", retire, { passive: true }); - scroller.addEventListener("touchstart", retire, { passive: true }); - scroller.addEventListener("wheel", retire, { passive: true }); - window.addEventListener("keydown", retire, true); + const retireForPointer = (event: PointerEvent) => { + // A descendant pointerdown is ordinary row interaction (link, reaction, + // thread action), not evidence that the reader took scroll ownership. + // Direct scroller hits cover scrollbar/background drag initiation. + if (event.target === scroller) cancel(); + }; + const retireForWheel = (event: WheelEvent) => { + // Ctrl+wheel is browser zoom, not reader navigation. Keep bottom intent + // armed so the resulting viewport/content reflow can settle at the new + // physical floor. + if (!event.ctrlKey) cancel(); + }; + const retireForScrollKey = (event: KeyboardEvent) => { + if ( + !event.altKey && + !event.ctrlKey && + !event.metaKey && + event.target instanceof Node && + scroller.contains(event.target) && + !isEditableKeyboardTarget(event.target) && + SCROLL_INTENT_KEYS.has(event.key) + ) { + cancel(); + } + }; + scroller.addEventListener("pointerdown", retireForPointer, { + passive: true, + }); + scroller.addEventListener("touchmove", retire, { passive: true }); + scroller.addEventListener("wheel", retireForWheel, { passive: true }); + window.addEventListener("keydown", retireForScrollKey, true); return () => { - scroller.removeEventListener("pointerdown", retire); - scroller.removeEventListener("touchstart", retire); - scroller.removeEventListener("wheel", retire); - window.removeEventListener("keydown", retire, true); + scroller.removeEventListener("pointerdown", retireForPointer); + scroller.removeEventListener("touchmove", retire); + scroller.removeEventListener("wheel", retireForWheel); + window.removeEventListener("keydown", retireForScrollKey, true); }; }, [cancel, hostRef]); + React.useLayoutEffect(() => { + const scroller = hostRef.current?.firstElementChild; + if (!(scroller instanceof HTMLDivElement)) return; + const content = scroller.firstElementChild; + if ( + !(content instanceof HTMLElement) || + typeof ResizeObserver === "undefined" + ) { + return; + } + // Virtua updates this inner element's extent whenever measured row geometry + // changes. Bottom intent therefore follows the physical floor for as long + // as it remains active, rather than guessing that layout is done after an + // arbitrary timeout. Reader input and explicit target/prepend navigation + // retire the intent through `cancel`. + const observer = new ResizeObserver(schedulePinToBottom); + observer.observe(content); + observer.observe(scroller); + return () => observer.disconnect(); + }, [hostRef, schedulePinToBottom]); + const settle = React.useCallback(() => { - cancel(); - const deadline = performance.now() + SETTLE_DEADLINE_MS; - let settledFrames = 0; - let previousHeight = -1; - const next = () => { - const scroller = hostRef.current?.firstElementChild; - const lastIndex = itemsLengthRef.current - 1; - if (!(scroller instanceof HTMLDivElement) || lastIndex < 0) { - cancel(); - return; - } - listRef.current?.scrollToIndex(lastIndex, { align: "end" }); - const atBottom = - scroller.scrollHeight - scroller.clientHeight - scroller.scrollTop <= - BOTTOM_EPSILON_PX; - settledFrames = - atBottom && scroller.scrollHeight === previousHeight - ? settledFrames + 1 - : 0; - previousHeight = scroller.scrollHeight; - if (settledFrames >= 2 || performance.now() >= deadline) { - frameRef.current = null; - return; - } - frameRef.current = requestAnimationFrame(next); - }; - next(); - }, [cancel, hostRef, itemsLengthRef, listRef]); + bottomIntentRef.current = true; + cancelFrame(); + pinToBottom(); + }, [cancelFrame, pinToBottom]); React.useEffect(() => cancel, [cancel]); return { cancel, settle }; diff --git a/desktop/src/features/messages/ui/virtuaWheelModePatch.test.mjs b/desktop/src/features/messages/ui/virtuaWheelModePatch.test.mjs new file mode 100644 index 000000000..cf6c1fe5c --- /dev/null +++ b/desktop/src/features/messages/ui/virtuaWheelModePatch.test.mjs @@ -0,0 +1,27 @@ +import assert from "node:assert/strict"; +import { readFile } from "node:fs/promises"; +import test from "node:test"; + +const patch = await readFile( + new URL("../../../../../patches/virtua@0.49.3.patch", import.meta.url), + "utf8", +); + +test("reader wheel retires Virtua shift mode without publishing scroll end", () => { + // Virtua 0.49.3 has no public transition for leaving SCROLL_BY_SHIFT. + // Keep the CJS and ESM patch paths symmetric and deliberately narrower than + // ACTION_SCROLL_END (2), which also idles direction, clears frozen range, + // flushes pending jumps, and emits UPDATE_SCROLL_END_EVENT. + const addedActionBodies = [ + ...patch.matchAll(/\+\s+case 9:\n((?:\+.*\n)+?)(?=\s*})/g), + ].map(([, body]) => + body + .split("\n") + .map((line) => line.replace(/^\+\s*/, "").trim()) + .filter(Boolean), + ); + assert.deepEqual(addedActionBodies, [["I = 0;"], ["w = 0;"]]); + assert.match(patch, /\+\s+e\.q\(9\);/); + assert.match(patch, /\+\s+e\.B\(9\);/); + assert.doesNotMatch(patch, /\+\s+e\.(?:q|B)\(2\);/); +}); diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts index 515669407..5d7bb6e5b 100644 --- a/desktop/tests/e2e/scroll-history.spec.ts +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -68,6 +68,61 @@ async function getMessagePosition( }, messageId); } +test("channel switch settles at the newest message after virtualized rows measure", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + + await page.evaluate(() => { + const base = Math.floor(Date.now() / 1000); + for (let index = 0; index < 80; index += 1) { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `switch-bottom ${index} ${"variable-height ".repeat( + index % 7, + )}`, + createdAt: base + index, + }); + } + }); + + // Open another channel first so this exercises a real channel switch, where + // the virtualizer API is temporarily null while the keyed list remounts. + await page.getByTestId("channel-random").click(); + await expect(page.getByTestId("chat-title")).toHaveText("random"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).toContainText("switch-bottom 79"); + // Reflow a rendered row well after the removed 250ms settle deadline. The + // geometry-driven bottom intent must still chase the new physical floor. + await timeline.evaluate((element) => { + window.setTimeout(() => { + const row = element.querySelector("[data-message-id]"); + if (row) { + row.style.minHeight = `${row.getBoundingClientRect().height + 240}px`; + } + element.dataset.delayedBottomReflow = "complete"; + }, 600); + }); + await expect(timeline).toHaveAttribute( + "data-delayed-bottom-reflow", + "complete", + ); + await expect + .poll(async () => { + const metrics = await getTimelineMetrics(page); + return metrics.scrollHeight - metrics.clientHeight - metrics.scrollTop; + }) + .toBeLessThanOrEqual(1); + await expect(page.getByTestId("message-scroll-to-latest")).toHaveCount(0); +}); + test("first channel load paints the first window without waiting for the row-floor top-up", async ({ page, }) => { @@ -168,6 +223,7 @@ test("preserves user scroll while older channel history loads", async ({ const scrollToTop = async () => timeline.evaluate((element) => { const container = element as HTMLDivElement; + container.dispatchEvent(new WheelEvent("wheel", { deltaY: -1 })); container.scrollTop = 0; container.dispatchEvent(new Event("scroll", { bubbles: true })); }); @@ -454,11 +510,20 @@ test("does not teleport upward when user abandons fetch by jumping to bottom", a ) .toBe("resolved"); - const afterPrepend = await getTimelineMetrics(page); - // (a) Geometry: timeline still pinned to bottom. - expect( - afterPrepend.scrollTop + afterPrepend.clientHeight, - ).toBeGreaterThanOrEqual(afterPrepend.scrollHeight - 2); + // (a) Geometry: timeline settles back to bottom. The durable bottom owner + // batches ResizeObserver-driven geometry correction into requestAnimationFrame, + // so scrollHeight growth and the physical-floor write need not share a turn. + await expect + .poll( + async () => { + const metrics = await getTimelineMetrics(page); + return ( + metrics.scrollTop + metrics.clientHeight >= metrics.scrollHeight - 2 + ); + }, + { timeout: 2_000 }, + ) + .toBe(true); // (b) DOM: the last rendered [data-message-id] sits within 2px of the // timeline's bottom edge. This catches a class of bugs where the geometry @@ -1641,6 +1706,7 @@ test("channel intro stays hidden while paginating past the timeline cap", async const scrollToTop = async () => timeline.evaluate((element) => { const container = element as HTMLDivElement; + container.dispatchEvent(new WheelEvent("wheel", { deltaY: -1 })); container.scrollTop = 0; container.dispatchEvent(new Event("scroll", { bubbles: true })); }); diff --git a/desktop/tests/e2e/smoke.spec.ts b/desktop/tests/e2e/smoke.spec.ts index d5ae2046d..afe58d371 100644 --- a/desktop/tests/e2e/smoke.spec.ts +++ b/desktop/tests/e2e/smoke.spec.ts @@ -634,6 +634,9 @@ test("does not shift the timeline when the composer grows", async ({ await page.waitForTimeout(400); await page.getByTestId("message-timeline").evaluate((element) => { const timeline = element as HTMLDivElement; + // The raw position assignment sets up detached history, while wheel is the + // same ownership signal a real reader produces before composer reflow. + timeline.dispatchEvent(new WheelEvent("wheel", { deltaY: -100 })); timeline.scrollTop = 0; timeline.dispatchEvent(new Event("scroll")); }); diff --git a/desktop/tests/e2e/timeline-no-shift.spec.ts b/desktop/tests/e2e/timeline-no-shift.spec.ts index b123a6741..7065d2937 100644 --- a/desktop/tests/e2e/timeline-no-shift.spec.ts +++ b/desktop/tests/e2e/timeline-no-shift.spec.ts @@ -481,6 +481,7 @@ test("timeline prepend plus late row reflow keeps the reading row stable", async await page.waitForTimeout(100); await timeline.evaluate((element) => { const scroller = element as HTMLDivElement; + scroller.dispatchEvent(new WheelEvent("wheel", { deltaY: -1 })); scroller.scrollTop = 150; scroller.dispatchEvent(new Event("scroll", { bubbles: true })); }); diff --git a/desktop/tests/e2e/virtualization.spec.ts b/desktop/tests/e2e/virtualization.spec.ts index 31800ae7e..c8e84c05b 100644 --- a/desktop/tests/e2e/virtualization.spec.ts +++ b/desktop/tests/e2e/virtualization.spec.ts @@ -297,8 +297,25 @@ test.describe("list virtualization", () => { }); const committedAnchor = await sampleVisibleAnchor(before.id); const motion = await timeline.evaluate( - async (scroller, { anchorId, anchorTop, oldHeight }) => { + async (scroller, { anchorId, anchorTop, oldHeight, testCtrlWheel }) => { const s = scroller as HTMLElement; + let ctrlWheelDispatched = false; + const mutationObserver = testCtrlWheel + ? new MutationObserver(() => { + if (ctrlWheelDispatched) return; + ctrlWheelDispatched = true; + // Ctrl+wheel is browser zoom, not reader scroll intent. Fire it + // synchronously with the prepend DOM commit, before the + // ResizeObserver measurement batch reconciles estimated rows. + s.dispatchEvent( + new WheelEvent("wheel", { ctrlKey: true, deltaY: -100 }), + ); + }) + : null; + mutationObserver?.observe(s.firstElementChild ?? s, { + childList: true, + subtree: true, + }); let maxDrift = 0; let sawPrepend = false; let sawAnchorAfterPrepend = false; @@ -326,15 +343,20 @@ test.describe("list virtualization", () => { if (sawAnchorAfterPrepend && stableFrames >= 8) break; await new Promise((resolve) => requestAnimationFrame(resolve)); } - return { maxDrift, sawPrepend }; + mutationObserver?.disconnect(); + return { ctrlWheelDispatched, maxDrift, sawPrepend }; }, { anchorId: committedAnchor.id, anchorTop: committedAnchor.top, oldHeight: before.scrollHeight, + // Exercise browser-zoom input once while a real prepend transaction + // still owns its measurement reconciliation. + testCtrlWheel: pageIndex === 0, }, ); expect(motion.sawPrepend).toBe(true); + if (pageIndex === 0) expect(motion.ctrlWheelDispatched).toBe(true); expect(motion.maxDrift).toBeLessThan(5); await expect @@ -357,11 +379,10 @@ test.describe("list virtualization", () => { await timeline.evaluate((element) => element.clientHeight), ); - // Leave the boundary with real downward wheel input while this prepend's - // three-second semantic-anchor watcher is still alive. The watcher belongs - // only to the completed prepend: it must not reinterpret this deliberate - // reader movement as row drift and pull the viewport back toward its stale - // baseline before the next upward load. + // Leave the boundary with real downward wheel input after this prepend. + // That reader intent retires Virtua's active prepend reconciliation, so + // later row measurements must not pull the viewport back toward the + // completed prepend before the next upward load. const exitTracePromise = timeline.evaluate(async (scroller) => { const s = scroller as HTMLElement; const startScrollTop = s.scrollTop; @@ -667,6 +688,10 @@ test("offscreen rich-row resize preserves the viewport-center anchor", async ({ const result = await timeline.evaluate(async (element) => { const scroller = element as HTMLDivElement; + // Retire bottom-follow intent the same way real reader input does before + // moving into detached history. A raw scrollTop assignment alone is not + // user intent and would correctly leave bottom following armed. + scroller.dispatchEvent(new WheelEvent("wheel", { deltaY: -1 })); scroller.scrollTop = scroller.scrollHeight / 2; scroller.dispatchEvent(new Event("scroll", { bubbles: true })); await new Promise((resolve) => setTimeout(resolve, 250)); diff --git a/patches/virtua@0.49.3.patch b/patches/virtua@0.49.3.patch index d7432534b..bd5424617 100644 --- a/patches/virtua@0.49.3.patch +++ b/patches/virtua@0.49.3.patch @@ -1,5 +1,5 @@ diff --git a/lib/index.cjs b/lib/index.cjs -index e02dfd0b3db60faff2cc705d7ce0aa51cb958f5a..c3bbc7f4d8dd396f5329a40a2701f4f752e3c5cc 100644 +index e02dfd0b3db60faff2cc705d7ce0aa51cb958f5a..b12677a356f748ebccf6e16cb781efe1315bb55b 100644 --- a/lib/index.cjs +++ b/lib/index.cjs @@ -35,10 +35,18 @@ const r = null, {min: n, max: s, abs: i, floor: l} = Math, c = (e, t, o) => n(o, @@ -39,11 +39,39 @@ index e02dfd0b3db60faff2cc705d7ce0aa51cb958f5a..c3bbc7f4d8dd396f5329a40a2701f4f7 case 5: - t[1] ? (L(w(M, t[0], !0)), I = 2, l = 1) : (w(M, t[0]), l = 1); -+ t[1] ? (L(w(M, t[0], !0, t[2])), I = 2, l = 1) : (w(M, t[0], !1, t[2]), l = 1); ++ t[1] ? (L(w(M, t[0], !0, t[2])), I = 2, l = 1) : (w(M, t[0], !1, t[2]), I = 0, l = 1); break; case 6: -@@ -519,11 +527,11 @@ const r = null, {min: n, max: s, abs: i, floor: l} = Math, c = (e, t, o) => n(o, +@@ -160,6 +168,10 @@ const r = null, {min: n, max: s, abs: i, floor: l} = Math, c = (e, t, o) => n(o, + + case 8: + k = W(t, t + a), l = 1; ++ break; ++ ++ case 9: ++ I = 0; + } + l && (d = 1 + (2147483647 & d), o && z && ($ += z, z = 0), O.forEach(([e, t]) => { + l & e && t(n); +@@ -186,6 +198,7 @@ const r = null, {min: n, max: s, abs: i, floor: l} = Math, c = (e, t, o) => n(o, + c = l(), d && (a = !0), i && e.q(6, i()), e.q(1, n()), h(); + }, p = t => { + if (f || !e.M() || t.ctrlKey) return; ++ e.q(9); + const r = l() - c; + 150 > r && 50 < r && (o ? t.deltaX : t.deltaY) && (f = !0); + }, v = () => { +@@ -266,7 +279,7 @@ const r = null, {min: n, max: s, abs: i, floor: l} = Math, c = (e, t, o) => n(o, + e[l] = t; + }); + } +- f[i] = R(e.C() + t, s), o && u(); ++ f[i] = R((o ? R(f[i], s) : e.C()) + t, s), o && u(); + }), n[1](!0); + }, + v() { +@@ -519,11 +532,11 @@ const r = null, {min: n, max: s, abs: i, floor: l} = Math, c = (e, t, o) => n(o, })(e); return [ e => t[e], t.length ]; }, [ e, o ]), j = /*#__PURE__*/ t.forwardRef(({children: r, data: n, bufferSize: s, itemSize: i, shift: l, horizontal: c, keepMounted: d, cache: a, startMargin: h = 0, ssrCount: g, as: p = "div", item: v = "div", scrollRef: _, onScroll: w, onScrollEnd: S}, m) => { @@ -59,7 +87,7 @@ index e02dfd0b3db60faff2cc705d7ce0aa51cb958f5a..c3bbc7f4d8dd396f5329a40a2701f4f7 const o = $(t); return e.jsx(Y, { diff --git a/lib/index.js b/lib/index.js -index 110ac3858a002a6cdb698da2b56350bc1bf609d2..81d239dad48d4453efd5ce7f8397555c66c99d56 100644 +index 110ac3858a002a6cdb698da2b56350bc1bf609d2..db83efc83fb3b6d7aa07d83b7463d6ed7746287e 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,7 +1,7 @@ @@ -102,25 +130,44 @@ index 110ac3858a002a6cdb698da2b56350bc1bf609d2..81d239dad48d4453efd5ce7f8397555c if (n = d(n, e.l - 1), b(e, n) <= t) { const r = y(e, o, n); return [ y(e, t, n, r), r ]; -@@ -124,7 +132,7 @@ const u = null, {min: d, max: a, abs: h, floor: g} = Math, p = (e, t, o) => d(o, - if (!e.length) break; - N(e.reduce((e, [t, o]) => { - let n; -- if (2 === w) n = !0; else if (I && 1 === w) n = t < I[0]; else { -+ if (2 === w) n = J(t) < E(); else if (I && 1 === w) n = t < I[0]; else { - const e = E(), o = J(t), r = A(t); - n = 1 !== _ && 0 === w ? o + r <= e : o < e && o + r < e + l; - } @@ -151,7 +159,7 @@ const u = null, {min: d, max: a, abs: h, floor: g} = Math, p = (e, t, o) => d(o, break; case 5: - t[1] ? (N(x(M, t[0], !0)), w = 2, d = 1) : (x(M, t[0]), d = 1); -+ t[1] ? (N(x(M, t[0], !0, t[2])), w = 2, d = 1) : (x(M, t[0], !1, t[2]), d = 1); ++ t[1] ? (N(x(M, t[0], !0, t[2])), w = 2, d = 1) : (x(M, t[0], !1, t[2]), w = 0, d = 1); break; case 6: -@@ -523,11 +531,11 @@ const u = null, {min: d, max: a, abs: h, floor: g} = Math, p = (e, t, o) => d(o, +@@ -164,6 +172,10 @@ const u = null, {min: d, max: a, abs: h, floor: g} = Math, p = (e, t, o) => d(o, + + case 8: + I = W(t, t + l), d = 1; ++ break; ++ ++ case 9: ++ w = 0; + } + d && (i = 1 + (2147483647 & i), o && p && (g += p, p = 0), O.forEach(([e, t]) => { + d & e && t(n); +@@ -190,6 +202,7 @@ const u = null, {min: d, max: a, abs: h, floor: g} = Math, p = (e, t, o) => d(o, + l = i(), d && (a = !0), s && e.B(6, s()), e.B(1, n()), h(); + }, p = t => { + if (c || !e.M() || t.ctrlKey) return; ++ e.B(9); + const n = i() - l; + 150 > n && 50 < n && (o ? t.deltaX : t.deltaY) && (c = !0); + }, v = () => { +@@ -270,7 +283,7 @@ const u = null, {min: d, max: a, abs: h, floor: g} = Math, p = (e, t, o) => d(o, + e[l] = t; + }); + } +- u[i] = W(e.C() + t, s), o && f(); ++ u[i] = W((o ? W(u[i], s) : e.C()) + t, s), o && f(); + }), r[1](!0); + }, + v() { +@@ -523,11 +536,11 @@ const u = null, {min: d, max: a, abs: h, floor: g} = Math, p = (e, t, o) => d(o, })(e); return [ e => o[e], o.length ]; }, [ e, t ]), Z = /*#__PURE__*/ i(({children: t, data: o, bufferSize: r, itemSize: s, shift: i, horizontal: u, keepMounted: d, cache: a, startMargin: h = 0, ssrCount: g, as: p = "div", item: _ = "div", scrollRef: w, onScroll: S, onScrollEnd: $}, z) => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d8ed8ec31..4cedd9bbe 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -9,7 +9,7 @@ overrides: patchedDependencies: isomorphic-git: e9b414a60d4cf1d8aa18f7a779483984e821989967c235662566e94ef0238d3f - virtua@0.49.3: acef47b2cfcb8bfd36668c30a16f918fe492eb07a1ddeafc8cc3a22a3efbf71b + virtua@0.49.3: 63923c1f0c73f6fd487c788159fe2f1bd6930e5aeebe815cc511c1c4747294bc importers: @@ -232,7 +232,7 @@ importers: version: 2.1.0 virtua: specifier: 0.49.3 - version: 0.49.3(patch_hash=acef47b2cfcb8bfd36668c30a16f918fe492eb07a1ddeafc8cc3a22a3efbf71b)(react-dom@19.2.7(react@19.2.7))(react@19.2.7) + version: 0.49.3(patch_hash=63923c1f0c73f6fd487c788159fe2f1bd6930e5aeebe815cc511c1c4747294bc)(react-dom@19.2.7(react@19.2.7))(react@19.2.7) yaml: specifier: ^2.8.3 version: 2.9.0 @@ -6926,7 +6926,7 @@ snapshots: '@types/unist': 3.0.3 vfile-message: 4.0.3 - virtua@0.49.3(patch_hash=acef47b2cfcb8bfd36668c30a16f918fe492eb07a1ddeafc8cc3a22a3efbf71b)(react-dom@19.2.7(react@19.2.7))(react@19.2.7): + virtua@0.49.3(patch_hash=63923c1f0c73f6fd487c788159fe2f1bd6930e5aeebe815cc511c1c4747294bc)(react-dom@19.2.7(react@19.2.7))(react@19.2.7): optionalDependencies: react: 19.2.7 react-dom: 19.2.7(react@19.2.7) From 07d0265cfc212ef02e1c26153bf58ff46ce5ffe6 Mon Sep 17 00:00:00 2001 From: Wes Date: Sat, 25 Jul 2026 17:22:15 -0700 Subject: [PATCH 11/11] fix(desktop): retire prepend mode on every reader wheel (#2913) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - retire Virtua prepend reconciliation on every ordinary reader wheel event, including events suppressed from its separate wheel-timing heuristic - keep Ctrl+wheel browser zoom excluded from reader ownership - arm the Ctrl+wheel prepend probe before pagination begins so it cannot miss the commit - preserve ESM/CJS patch parity and update the patch lock hash ## Why main was red PR #2855 added the reader-wheel retirement action after Virtua's existing suppression guard. Once the first event set that guard, later events in the same wheel burst returned before retiring prepend mode. A late ResizeObserver correction could then pull the viewport backward by 20–40px. The same test had failed twice on #2855 but passed its final retry, so the PR job appeared green; the merge commit lost all three retries. The separate Ctrl+wheel failure was a test race: its MutationObserver was registered after the request had already been triggered and could miss the prepend commit. ## Validation - desktop full unit suite: 3,515 passed - pre-push desktop check: passed - `git diff --check`: passed - CI is the E2E verification; no local E2E was run Fixes the main-branch failure in CI run 30180099007. Signed-off-by: Wes Co-authored-by: Carl --- .../messages/ui/virtuaWheelModePatch.test.mjs | 14 +++++- desktop/tests/e2e/virtualization.spec.ts | 50 ++++++++++--------- patches/virtua@0.49.3.patch | 12 +++-- pnpm-lock.yaml | 6 +-- 4 files changed, 49 insertions(+), 33 deletions(-) diff --git a/desktop/src/features/messages/ui/virtuaWheelModePatch.test.mjs b/desktop/src/features/messages/ui/virtuaWheelModePatch.test.mjs index cf6c1fe5c..9dcb07d12 100644 --- a/desktop/src/features/messages/ui/virtuaWheelModePatch.test.mjs +++ b/desktop/src/features/messages/ui/virtuaWheelModePatch.test.mjs @@ -21,7 +21,17 @@ test("reader wheel retires Virtua shift mode without publishing scroll end", () .filter(Boolean), ); assert.deepEqual(addedActionBodies, [["I = 0;"], ["w = 0;"]]); - assert.match(patch, /\+\s+e\.q\(9\);/); - assert.match(patch, /\+\s+e\.B\(9\);/); + assert.match( + patch, + /if \(!e\.M\(\) \|\| t\.ctrlKey\) return;\n\+\s+e\.q\(9\);\n\+\s+if \(f\) return;/, + ); + assert.match( + patch, + /if \(!e\.M\(\) \|\| t\.ctrlKey\) return;\n\+\s+e\.B\(9\);\n\+\s+if \(c\) return;/, + ); + assert.doesNotMatch( + patch, + /\+\s+if \((?:f|c) \|\| !e\.M\(\) \|\| t\.ctrlKey\) return;/, + ); assert.doesNotMatch(patch, /\+\s+e\.(?:q|B)\(2\);/); }); diff --git a/desktop/tests/e2e/virtualization.spec.ts b/desktop/tests/e2e/virtualization.spec.ts index c8e84c05b..61a2753ba 100644 --- a/desktop/tests/e2e/virtualization.spec.ts +++ b/desktop/tests/e2e/virtualization.spec.ts @@ -252,6 +252,29 @@ test.describe("list virtualization", () => { }); await page.waitForTimeout(150); const before = await sampleVisibleAnchor(); + const ctrlWheelPromise = + pageIndex === 0 + ? timeline.evaluate( + (scroller) => + new Promise((resolve) => { + const s = scroller as HTMLElement; + const observer = new MutationObserver(() => { + observer.disconnect(); + // Ctrl+wheel is browser zoom, not reader scroll intent. Fire + // it synchronously with the prepend DOM commit, before the + // ResizeObserver measurement batch reconciles estimated rows. + s.dispatchEvent( + new WheelEvent("wheel", { ctrlKey: true, deltaY: -100 }), + ); + resolve(true); + }); + observer.observe(s.firstElementChild ?? s, { + childList: true, + subtree: true, + }); + }), + ) + : Promise.resolve(false); const wheelTracePromise = timeline.evaluate(async (scroller) => { const s = scroller as HTMLElement; let previousScrollTop = s.scrollTop; @@ -297,25 +320,8 @@ test.describe("list virtualization", () => { }); const committedAnchor = await sampleVisibleAnchor(before.id); const motion = await timeline.evaluate( - async (scroller, { anchorId, anchorTop, oldHeight, testCtrlWheel }) => { + async (scroller, { anchorId, anchorTop, oldHeight }) => { const s = scroller as HTMLElement; - let ctrlWheelDispatched = false; - const mutationObserver = testCtrlWheel - ? new MutationObserver(() => { - if (ctrlWheelDispatched) return; - ctrlWheelDispatched = true; - // Ctrl+wheel is browser zoom, not reader scroll intent. Fire it - // synchronously with the prepend DOM commit, before the - // ResizeObserver measurement batch reconciles estimated rows. - s.dispatchEvent( - new WheelEvent("wheel", { ctrlKey: true, deltaY: -100 }), - ); - }) - : null; - mutationObserver?.observe(s.firstElementChild ?? s, { - childList: true, - subtree: true, - }); let maxDrift = 0; let sawPrepend = false; let sawAnchorAfterPrepend = false; @@ -343,20 +349,16 @@ test.describe("list virtualization", () => { if (sawAnchorAfterPrepend && stableFrames >= 8) break; await new Promise((resolve) => requestAnimationFrame(resolve)); } - mutationObserver?.disconnect(); - return { ctrlWheelDispatched, maxDrift, sawPrepend }; + return { maxDrift, sawPrepend }; }, { anchorId: committedAnchor.id, anchorTop: committedAnchor.top, oldHeight: before.scrollHeight, - // Exercise browser-zoom input once while a real prepend transaction - // still owns its measurement reconciliation. - testCtrlWheel: pageIndex === 0, }, ); expect(motion.sawPrepend).toBe(true); - if (pageIndex === 0) expect(motion.ctrlWheelDispatched).toBe(true); + if (pageIndex === 0) expect(await ctrlWheelPromise).toBe(true); expect(motion.maxDrift).toBeLessThan(5); await expect diff --git a/patches/virtua@0.49.3.patch b/patches/virtua@0.49.3.patch index bd5424617..5debe418e 100644 --- a/patches/virtua@0.49.3.patch +++ b/patches/virtua@0.49.3.patch @@ -54,11 +54,13 @@ index e02dfd0b3db60faff2cc705d7ce0aa51cb958f5a..b12677a356f748ebccf6e16cb781efe1 } l && (d = 1 + (2147483647 & d), o && z && ($ += z, z = 0), O.forEach(([e, t]) => { l & e && t(n); -@@ -186,6 +198,7 @@ const r = null, {min: n, max: s, abs: i, floor: l} = Math, c = (e, t, o) => n(o, +@@ -186,6 +198,8 @@ const r = null, {min: n, max: s, abs: i, floor: l} = Math, c = (e, t, o) => n(o, c = l(), d && (a = !0), i && e.q(6, i()), e.q(1, n()), h(); }, p = t => { - if (f || !e.M() || t.ctrlKey) return; +- if (f || !e.M() || t.ctrlKey) return; ++ if (!e.M() || t.ctrlKey) return; + e.q(9); ++ if (f) return; const r = l() - c; 150 > r && 50 < r && (o ? t.deltaX : t.deltaY) && (f = !0); }, v = () => { @@ -150,11 +152,13 @@ index 110ac3858a002a6cdb698da2b56350bc1bf609d2..db83efc83fb3b6d7aa07d83b7463d6ed } d && (i = 1 + (2147483647 & i), o && p && (g += p, p = 0), O.forEach(([e, t]) => { d & e && t(n); -@@ -190,6 +202,7 @@ const u = null, {min: d, max: a, abs: h, floor: g} = Math, p = (e, t, o) => d(o, +@@ -190,6 +202,8 @@ const u = null, {min: d, max: a, abs: h, floor: g} = Math, p = (e, t, o) => d(o, l = i(), d && (a = !0), s && e.B(6, s()), e.B(1, n()), h(); }, p = t => { - if (c || !e.M() || t.ctrlKey) return; +- if (c || !e.M() || t.ctrlKey) return; ++ if (!e.M() || t.ctrlKey) return; + e.B(9); ++ if (c) return; const n = i() - l; 150 > n && 50 < n && (o ? t.deltaX : t.deltaY) && (c = !0); }, v = () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4cedd9bbe..e3516e853 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -9,7 +9,7 @@ overrides: patchedDependencies: isomorphic-git: e9b414a60d4cf1d8aa18f7a779483984e821989967c235662566e94ef0238d3f - virtua@0.49.3: 63923c1f0c73f6fd487c788159fe2f1bd6930e5aeebe815cc511c1c4747294bc + virtua@0.49.3: 30a7a92b94800ec37be8d36546ea98c04a05dcd0637f9fac27a06f016ba2eff4 importers: @@ -232,7 +232,7 @@ importers: version: 2.1.0 virtua: specifier: 0.49.3 - version: 0.49.3(patch_hash=63923c1f0c73f6fd487c788159fe2f1bd6930e5aeebe815cc511c1c4747294bc)(react-dom@19.2.7(react@19.2.7))(react@19.2.7) + version: 0.49.3(patch_hash=30a7a92b94800ec37be8d36546ea98c04a05dcd0637f9fac27a06f016ba2eff4)(react-dom@19.2.7(react@19.2.7))(react@19.2.7) yaml: specifier: ^2.8.3 version: 2.9.0 @@ -6926,7 +6926,7 @@ snapshots: '@types/unist': 3.0.3 vfile-message: 4.0.3 - virtua@0.49.3(patch_hash=63923c1f0c73f6fd487c788159fe2f1bd6930e5aeebe815cc511c1c4747294bc)(react-dom@19.2.7(react@19.2.7))(react@19.2.7): + virtua@0.49.3(patch_hash=30a7a92b94800ec37be8d36546ea98c04a05dcd0637f9fac27a06f016ba2eff4)(react-dom@19.2.7(react@19.2.7))(react@19.2.7): optionalDependencies: react: 19.2.7 react-dom: 19.2.7(react@19.2.7)