fix(meshcore): channel view opens at the top and Delete appears to do nothing

[Yeraze]

Two independent, pre-existing bugs on the MeshCore channel view, both reported from the running app. Both date to the per-channel history introduced in #4461 — neither file was touched by the #4473 nav work (verified against both merge commits).

1. Delete appeared to do nothing

The stream renders filtered = merge(history, messages), but actions.deleteMessage only prunes the shared messages pool. Anything served from the per-channel backlog (#4460) — which is nearly everything on this view — was deleted server-side and then immediately re-rendered from history.

Confirmed against the live API while reproducing:

serverStillHasIt: false     ← the row was already gone from the database
uiRows:          [...still showing it...]

handleDeleteMessage and handleClearChannel now prune history (and the count badge) on success, and only on success — a failed delete leaves the row rather than vanishing optimistically.

DM view needs no change: it builds its list purely from the shared pool, with no separate history.

2. Channel opened at the top instead of the newest message

Mobile only, which is why desktop testing missed it — desktop lands at the bottom correctly every time.

The mobile two-pane layout mounts the conversation but hides it until the operator drills in from the list. Measured in that state:

clientHeight: 0   scrollHeight: 0   offsetParent: null

The entry scroll fired there, scrollTop = scrollHeight was 0 = 0 — a silent no-op — but the one shot was already spent. Drilling in then left the view pinned at the very top of a 17,000px backlog with nothing left to re-trigger it.

The scroll no longer spends the shot until the list has real layout, and a ResizeObserver completes it the moment the pane is revealed. This fixes the DM view too, which shares the component.

One implementation note worth flagging: the guard keys off scrollHeight alone, not clientHeight. A hidden subtree zeroes both, but jsdom reports clientHeight: 0 for everything — a clientHeight guard silently disabled the entry scroll in every test, including two existing ones that had been passing.

Verified on the running container

390×780, both MeshCore sources:

	before	after
entry scroll distance from bottom	16774 px	0
delete: rows in UI	202 → 202	202 → 201
delete: row still in DB	already gone	gone

Also removed the bogus 2082-dated message from both sources while confirming the fix (futureLeftOnServer: 0 on each).

Test plan

• 4 new tests in MeshCoreChannelsView.test.tsx — delete prunes the backlog; failed delete keeps the row; cancelled confirm is a no-op; clear empties the backlog.
• 2 new tests in MeshCoreMessageStream.test.tsx — entry scroll defers while hidden then runs on reveal; does not re-scroll on later resizes (so reading history isn't interrupted by a keyboard/rotate).
• Confirmed real regressions: reverting each fix fails exactly the corresponding tests (2 and 2).
• All 23 pre-existing stream tests still pass.
• Full Vitest suite — success: true, 12954 passed, 0 failed.
• npx tsc --noEmit clean; npm run lint:ci no FAIL lines.

---

Generated by Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: fix(meshcore): channel view opens at the top and Delete appears to do nothing

Tasks:

• Read changed files
• Review implementation files
• Review test coverage
• Post final feedback

---

This is a well-diagnosed and well-executed fix for two pre-existing bugs. The root causes are clearly documented, the implementation is correct, and the tests are thorough. Here's my detailed review:

Bug 1 — Delete/Clear didn't prune history (MeshCoreChannelsView.tsx)

Diagnosis: Correct. The filtered useMemo merges history and messages, but actions.deleteMessage only prunes messages. Anything sourced from the per-channel backlog lived in history and survived deletion.

Fix: handleDeleteMessage and handleClearChannel now prune history and counts on success, and do nothing on failure. This is the right behavior — no optimistic UI.

One minor note on handleDeleteMessage (lines 545–556):

const handleDeleteMessage = useCallback(async (m: MeshCoreMessage) => {
  ...
  setCounts(prev => {
    const current = prev[activeChannelIdxRef.current];
    ...
  });
}, [actions, t]);

Using activeChannelIdxRef.current inside the setCounts updater is safe because the ref is synchronously updated and this runs immediately after a successful delete. This is correctly using the ref to avoid a stale closure — no issue here.

handleClearChannel (lines 557–568): Uses active.id directly in the callback. Since active is derived from displayChannels in the same render and handleClearChannel depends on active, this is fine.

Bug 2 — Entry scroll fired while pane was hidden (MeshCoreMessageStream.tsx)

Diagnosis: Correct and well-explained. The mobile two-pane layout mounts the conversation hidden (scrollHeight: 0), so the entry scroll was a no-op that still consumed the "done" guard. The ResizeObserver approach is the right solution.

runEntryScroll (lines 184–188):

const runEntryScroll = useCallback((container: HTMLDivElement): boolean => {
  if (container.scrollHeight === 0) return false;
  container.scrollTop = container.scrollHeight;
  return true;
}, []);

The scrollHeight === 0 guard is load-bearing and the comment explains the jsdom rationale clearly. Using scrollHeight instead of clientHeight for the guard is a subtle but correct choice — documented well.

ResizeObserver effect (lines 213–223):

useEffect(() => {
  const container = listRef.current;
  if (!container || typeof ResizeObserver === 'undefined') return;
  const observer = new ResizeObserver(() => {
    const state = entryScrollRef.current;
    if (state.done) return;
    if (runEntryScroll(container)) state.done = true;
  });
  observer.observe(container);
  return () => observer.disconnect();
}, [runEntryScroll]);

The typeof ResizeObserver === 'undefined' guard is appropriate for environments that lack it (though jsdom ships it). The state.done check prevents re-scrolling after the entry scroll has fired — the test for this (line 532) confirms it works correctly.

Interaction with the existing auto-scroll effect (lines 226–234):

The auto-scroll on new messages uses isNearBottom to decide whether to scroll. After the entry scroll fires (scrollTop = scrollHeight), the user is at the bottom, so new messages will still auto-scroll correctly. These two effects compose correctly.

Test Coverage

The 6 new tests are well-targeted:

MeshCoreChannelsView (delete suite, lines 840–943):

• ✅ Delete prunes a backlog-sourced message on success
• ✅ Failed delete keeps the row (no optimistic removal)
• ✅ Cancelled confirm is a no-op (deleteMessage never called)
• ✅ Clear empties the entire backlog

MeshCoreMessageStream (hidden-pane scroll suite, lines 483–553):

• ✅ Defers while hidden (scrollHeight: 0), fires on reveal
• ✅ Does not re-scroll on later resizes once the entry scroll has run

The hidden-pane test setup is clever: it stubs ResizeObserver to capture callbacks, then drives them with act(). This directly exercises the production code path without any jsdom layout workarounds beyond the scrollHeight stub (which is already needed by the other scroll tests).

Minor observations (no action required)

1. clearChannelMessages is not in makeActions() mock — the test at line 919 overrides it explicitly with vi.fn().mockResolvedValue(true), which works. But since MeshCoreActions presumably includes clearChannelMessages, it might be worth adding a default stub to makeActions() for consistency. Not a bug.

2. confirmSpy is declared at describe scope (line 838) — this means it persists across all four tests in that suite. Each test calls confirmSpy.mockReturnValue(...) before use, so there's no leakage. Fine as-is.

3. The activeChannelIdxRef.current snapshot in the setCounts updater — this is actually using the ref (not active.id) intentionally to capture the channel at delete-time. It's correct but slightly subtle. Given the ref is assigned synchronously in the same component render cycle, this works correctly.

Summary

Both fixes are correct, minimal, and well-tested. The scrollHeight-based guard is particularly clean — it solves the mobile bug without breaking the test environment, and the comment explaining why clientHeight would fail in jsdom is exactly the kind of gotcha documentation this codebase benefits from.

✅ No blocking issues. Ready to merge.