fix(rail): make the workspace unread dot agree with the sidebar by landonforshage · Pull Request #207 · gammons/slk

landonforshage · 2026-09-11T18:10:35Z

Problem

The green dot next to a workspace in the left rail lit up for any channel with has_unread=1 in the cache. Every other unread indicator in slk (the sidebar dot, the (N) in the window title, $SLK_UNREAD) ignores muted channels. The rail didn't. So a single muted firehose channel kept a workspace's dot on permanently, with nothing visibly unread inside it.

Worse, one workspace was lit by a channel that wasn't in the sidebar or Ctrl+t at all, so there was no way to clear it from inside slk. That one turned out to be an archived Slack Connect channel.

I hit both within an hour of first use, on 4 of 5 workspaces. Once the dot is always on, you stop looking at it.

Cause

Two things, verified against live client.userBoot and client.counts responses on 2026-09-11:

Mutes aren't in the database. WorkspacesWithUnreads was a plain SQL query. Mute state lives only in service.MuteStore, in memory, so no query can filter it.
The cache knows about channels the sidebar hides. client.userBoot lists archived channels the user belongs to with is_archived: true. bootConversations correctly drops them from the sidebar, but hydrateFirstSight caches every channel userBoot returns, archived or not. client.counts still reported the archived channel as unread (its last_read was Slack's never-opened sentinel), so the row got has_unread=1 and nothing could ever show or clear it. Channel rows are never deleted, so the dot was permanent.

Fix

The rail now asks the same question the sidebar asks: does this workspace have a channel in its list that IsVisiblyUnread?

cache.UnreadChannels() returns unread rows with their channel IDs (replaces WorkspacesWithUnreads).
railUnreadWorkspaces (cmd/slk/rail_unread.go) is a pure function: for each unread row, find the channel in the workspace's wctx.Channels and apply IsVisiblyUnread. Muted → no dot. Not in the list → no dot. Workspace still connecting → keep the dot (the conservative default MuteStore.Ready documents, and what keeps cached dots visible during boot).
The title's +N and $SLK_OTHER_UNREAD read through the same function, so they stay in sync with the dots.

Review of the diff turned up three more gaps, each its own commit:

workspaceRouter gets a mutex. Its map was written by connect goroutines and read by UI callbacks with no lock; the comment saying it was populated before p.Run was wrong, since connects run alongside it. That's a runtime fatal waiting to happen, and the rail reader made it more likely. The duplicate workspaces map in run is removed too.
Inactive-workspace mute changes refresh the rail. Muting a channel from another client in a workspace you're not looking at updates the dot immediately instead of waiting for the next unrelated event.
Channel-list changes fan out to the rail and title. SectionsRefreshedMsg for the active workspace and WorkspaceReadyMsg both changed what the unread surfaces derive from without calling notifyReadStateChanged. Muting an unread channel in the active workspace left (N) stale, and a workspace finishing its connect kept a cached dot its now-known channel list no longer justified.

Six commits, one per change (the fifth only trims a comment), each with its own tests.

Why not fix it elsewhere

Filter archived channels out of hydrateFirstSight? The cache is allowed to know more than the sidebar shows (search results resolve <#C…> mentions through those rows), and it wouldn't fix existing caches since rows are never deleted. Happy to add it as a follow-up if you'd like the rows gone too.
Add an is_archived column? A migration for a display bug. Not needed once the rail checks the live list.
Filter at the counts write path? Three writers to keep in sync instead of one reader.

Tests

All new tests are plain testing.T, white-box. The rail tests add two unexported helpers local to their file (unreadRow, railLookup); nothing shared.

Cache: TestUnreadChannels.
Rail predicate: muted → dark, muted + unmuted → lit, absent from list → dark, unknown workspace → lit, and MuteStore not ready → lit (through the real buildChannelItem).
TestRailUnreadWorkspaces_ArchivedChannelInCache: the archived-channel repro built the way production builds it (hydrateFirstSight → bootConversations → ReplaceWorkspaceReadState).
TestRailUnreadWorkspaces_RailAndTitleAgree: dots and OtherUnreadCount are the same set.
TestWorkspaceRouter_ConcurrentAddAndByID: fails under -race with the mutex removed.
TestMuteRefreshMsg: active vs inactive message choice.
TestWorkspaceReady_RefreshesRailAndTitle and TestSectionsRefreshed_ActiveWorkspace_RefreshesTitle: through App.Update; the title and rail recompute when a workspace connects and when an active-workspace channel is muted.

Each commit's tests were confirmed red with its change reverted. go build, go vet, go test ./... -race, gofmt -l, and golangci-lint run (v2.13.1) are all clean.

Field check (macOS, five live workspaces): with the patched build sitting on one workspace, marking a message unread in a different workspace from the official client lit that workspace's dot; muting the channel there cleared the dot immediately, without touching slk; unmuting lit it again; opening the channel cleared it. That is the mute rule and the inactive-workspace refresh end to end. The archived-channel case was consumed before the fix (opening it in the official client is the only way out today), so it is covered by the production-shaped test above rather than a live re-run.

Noted but not changed

wctx.Channels is read on the UI goroutine while the WebSocket handler mutates it. That predates this PR (the Lookup callback does the same) and needs a lock or snapshot on WorkspaceContext, which is bigger than this change.
OtherUnreadCount will count a workspace that's in the cache but no longer configured. Pre-existing.

AI assistance

Written with Claude (Fable 5.1, extra-high thinking effort) driving the investigation, code, and tests from a brief I wrote after reproducing both bugs. The cause was confirmed against live API responses, not inferred from the code, and Codex reviewed the diff; its findings are commits 3, 4 and 6. I've read the diff and will defend it in review.

🤖 Generated with Claude Code

The workspace rail lit a workspace for any channels row with has_unread=1, while every other unread surface (the sidebar dot, UnreadChannelCount, $SLK_UNREAD, the title's "(N)") goes through ChannelItem.IsVisiblyUnread and so ignores muted channels. One muted firehose channel therefore kept a workspace's dot on permanently with nothing visibly unread inside it. Four of five workspaces were stuck that way in the field, each held by exactly one muted, zero-mention channel. The query cannot apply the filter itself: mute state lives only in service.MuteStore, in memory, and is never written to the channels table. So the cache now returns the unread rows with their channel IDs (UnreadChannels, replacing WorkspacesWithUnreads), and a pure predicate in cmd/slk, railUnreadWorkspaces, checks each row against the owning workspace's wctx.Channels with the same IsVisiblyUnread the sidebar uses. OtherUnreadCount reads through the same installed reader, so the title's "+N" and $SLK_OTHER_UNREAD move with the dots instead of diverging from $SLK_UNREAD. A workspace the router cannot resolve, or a row whose channel is not in wctx.Channels, is still lit: with no channel list to check against, showing a dot we might have suppressed beats hiding one the user wanted to see, the default MuteStore.Ready documents. That is also what keeps last session's cached dots visible during boot. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

A channels row can carry has_unread=1 for a channel that is not in the workspace's sidebar at all. The field case was an archived Slack Connect channel: client.userBoot lists archived conversations the user belongs to, with is_archived=true (verified against a live response); bootConversations and users.conversations (ExcludeArchived) keep them out of wctx.Channels, but hydrateFirstSight caches every conversation userBoot names, and client.counts still reported the channel unread because its last_read was Slack's never-opened sentinel. The rail counted the row, the sidebar and Ctrl+t could not show it, and no keystroke could clear it. The only way out was opening the channel in the official client, whose channel_marked write-through finally set has_unread=0. railUnreadWorkspaces now treats a row whose channel is absent from wctx.Channels as "cannot be shown, so never lit" instead of "mute unknown, so lit". That is the property the rail should have had from the start -- it agrees with what the sidebar renders -- and it holds regardless of how such a row got there, which also covers every existing cache: nothing deletes channel rows, so the archived rows are already present and would otherwise stay lit until each one is opened elsewhere. Filtering archived conversations out of hydrateFirstSight instead was considered and rejected. The cache is allowed to know more conversations than the sidebar shows (search results resolve <#C...> mentions through those rows), it would leave every existing cache lit, and it closes one way a row can be unshowable rather than the property itself. The nil-router case is unchanged: a workspace still connecting keeps its cached dots, the conservative default MuteStore.Ready documents. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

workspaceRouter.all was a plain map written by each workspace's connect goroutine and read from UI-goroutine callbacks. Its comment said the map was populated before p.Run and so needed no mutex; that was wrong. run launches the connect goroutines and then calls p.Run immediately, so each Add lands while the program is already handling messages -- including the WorkspaceReadyMsg of whichever workspace finished first, whose callbacks (EnsureSubscriptions, and since the previous commits the rail's unread reader on every read-state event) call ByID in that window. A collision is a runtime fatal, "concurrent map read and map write", not a report from the race detector. The map is now behind an RWMutex with Add, ByID and All. The `workspaces` map in run -- a byte-identical twin with the same writers and the same UI-goroutine readers -- is removed; its reads go through router.ByID. A race-detector test reproduces the boot pattern and fails without the lock. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

refreshMutedForActive posted nothing for an inactive workspace, which was fine while nothing outside the active sidebar depended on IsMuted. The rail dot, the title's "+N" and $SLK_OTHER_UNREAD now do (railUnreadWorkspaces), so a channel muted or unmuted from another Slack client left them stale -- an unmute left a workspace with unreads dark, a mute kept one lit -- until an unrelated read-state event happened to arrive. The inactive branch now posts ReadStateChangedMsg, the message notifyReadStateChanged already answers to; the App ignores its fields, so no new message type is needed. The choice of message is a pure function, muteRefreshMsg, so it is tested without a *tea.Program. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The doc had grown to twice the length of anything else in the repo: it carried the archived-channel investigation and the rejected hydrateFirstSight alternative, both of which the commit that added the rule already records. It also said router.ByID is read unsynchronized by precedent, which stopped being true one commit later when workspaceRouter gained its mutex. Keep the rule, the two edge cases, and the wctx.Channels caveat that still holds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Two entry points swapped what the unread surfaces derive from without fanning out to them. notifyReadStateChanged is the one place the sidebar cache, the rail dots, the window title and the status hook are recomputed together, and neither path called it. SectionsRefreshedMsg for the active workspace replaced the sidebar's items -- carrying new IsMuted flags -- and stopped there, so muting an unread channel dropped its sidebar dot while "(N)" and $SLK_UNREAD kept counting it until the next read-state event. Pre-existing; the active twin of the inactive case the previous fix covered. WorkspaceReadyMsg never refreshed the rail. That was harmless when the rail's answer came straight from the cache, but railUnreadWorkspaces keeps a workspace's cached dot until the router knows the workspace, and once connected the answer can change: a dot held up only by muted or unlisted channels should go dark, and nothing recomputed it. Both paths now call notifyReadStateChanged, tested through App.Update. The title-wiring test's doc still described the rail count as not mute-filtered; corrected alongside. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

gammons · 2026-09-13T10:43:22Z

This is a real bug, correctly diagnosed, and the fix is the right shape. Merging.

I verified the two load-bearing claims rather than trusting them:

Mutes live only in internal/service/mutestore.go (an in-memory map[string]bool), so a plain SQL query structurally cannot filter them. Correct.
IsArchived is dropped from the sidebar at bootstrap_adapters.go:408 but hydrateFirstSight still caches it — so the dot stays lit with no way to clear it. Correct. Your field-note about the archived Slack Connect channel explains how you hit it.

The fix routes the rail through IsVisiblyUnread, the single predicate the sidebar dot, window-title count, and $SLK_UNREAD already share. That is the right call. The worst version of this fix would have been a third parallel unread rule that drifts from the other two; you did the opposite.

I checked the things that usually bite:

The "channel not in wctx.Channels → never light" branch is safe for DMs — GetChannels requests im/mpim and connectWorkspace appends them, so DM unreads still light the rail. The unlisted case is only channels the sidebar can't render (archived, left-since), where a lit dot is unexplainable.
The wctx == nil → light-it default preserves boot dots and matches MuteStore.Ready()'s conservative assumption. Sensible.
I reverted the production change and confirmed the tests fail — TestRailUnreadWorkspaces_RailAndTitleAgree and the archived-channel repro both go red as you claimed, and the archived repro is built through the real production pipeline (hydrateFirstSight → bootConversations → buildChannelItem), not hand-rolled fixtures. That's the part that makes it trustworthy.
You also fixed a genuine concurrent-map fatal in the workspace router that an old comment declared impossible.

On the wctx.Channels race you flagged: your framing is right. It's a pre-existing non-fatal tear, not the fatal map race, and this PR adds a reader rather than creating the convention. I've filed #208 to track locking WorkspaceContext.Channels — worth doing, not worth blocking this.

Your AI-assistance note is exactly the standard I want to see: reproduced on live workspaces, confirmed the cause against real API responses rather than inferring it, and you're prepared to defend the diff. More of that, please.

Build, vet, gofmt, golangci-lint, and go test ./... -race all clean. Merging.

The rail dot only ever looked at channel rows, but a thread reply never sets the channel's has_unread (OnMessage's channelEligible rule), so a workspace could show a "•N" Threads badge in its own sidebar and a dark dot in the rail. Before #207 a muted firehose channel was lighting the dot by mistake and masking this; once the mute rule landed the gap showed: two unread DM thread replies in a workspace, no dot. The rail now asks the same question the Threads badge answers: does cache.ListSubscribedThreads return any Unread summary for this workspace? Reusing that query, rather than an EXISTS twin of it, is the point: a second predicate is a second place for the rail and the badge to drift apart. Its cost was measured on the field cache first (five workspaces, at most ten active subscriptions each, well under a millisecond in total), so no cheaper form is needed. Rejected: the boot-time counts.threads.has_unreads flag, which is stale from the first thread_marked onward and which per-thread last_read already replaced; a derived column on workspaces, a schema change for a display bug; and the in-memory badge, which exists only for the active workspace. $SLK_UNREAD and the title's "(N)" still count channels only; the dot, "+N" and $SLK_OTHER_UNREAD are "workspaces with something unread", and threads fit that. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The rail's thread half reads thread_subscriptions and the message cache, but none of the paths that change those rows in the active workspace called notifyReadStateChanged: a thread marked read from another client, slk's own mark completing, a threads-list reload, and a live reply all settled the sidebar badge and left the dot where it was until an unrelated channel event. That is the same lag #207 closed for inactive-workspace mute changes, one signal over. Three sites fan out now, chosen by one rule: the DB has already changed when the message arrives. applyThreadMarkListState (both mark messages: the cursor is written before either is sent); the ThreadsListLoadedMsg arm, for the active workspace so the rail moves with the badge, and for an inactive one because that message is the tail of a change dispatched before the user switched away and the switch itself recomputes nothing; and reduceNewMessage's thread-reply arm, mirroring its channel arms, so a reply reaches the dot at once rather than through a threads-list fetch that a workspace switch inside the debounce window would drop. The optimistic badge sites (MarkSelectedRead on open, the ThreadRepliesLoadedMsg recompute, applyThreadMarkUnread) do not fan out: nothing in the DB has changed at those moments, so the rail would only re-read the answer it already shows; the mark they issue, or its thread_marked echo, refreshes it when it lands. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…changes OnThreadMarked and OnThreadSubscriptionChanged already persist their rows regardless of which workspace is active, but both returned before dispatching anything for an inactive one, on the grounds that the list and badge live on the active workspace. The rail does not: its thread half reads the very rows these handlers write, so a thread read in the official client while the user was on another workspace kept its dot lit until an unrelated event, and a new auto-subscription never lit one. Each inactive branch now sends ReadStateChangedMsg{WorkspaceID}, the message notifyReadStateChanged answers to and the same choice #207 made for an inactive mute change (muteRefreshMsg). The active-only messages stay active-only. rtmEventHandler.program is narrowed from *tea.Program to the teaSender interface reconnect_sync already uses, and bootstrapPresenceAndDND's parameter with it; only Send was ever called through either, and the narrowing is what lets the tests assert on the dispatched message instead of on a pure helper standing in for it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The first draft carried the field narrative and the rejected alternatives, which the commit message already holds; #207's comment had to be cut from 60 lines after review for the same reason. What stays is the two signals, what each does and does not filter, the two edge cases, and the #208 caveat. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

#207's membership rule exists because a channel the sidebar cannot show has no dot to explain. It must not swallow thread unreads: the Threads badge does not filter on wctx.Channels, and the thread is on screen in the Threads view whether or not its channel is listed. The other thread cases already leave the channel unlisted, but none said so; this one does, so a future "apply the membership rule to threads too" change has a test to argue with. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

landonforshage and others added 6 commits September 11, 2026 12:48

gammons added the ready to merge Reviewed, approved, no blockers label Sep 13, 2026

gammons mentioned this pull request Sep 13, 2026

data race: WorkspaceContext.Channels read on UI goroutine, written by WS handler #208

Open

gammons merged commit cc3226f into gammons:main Sep 13, 2026
3 checks passed

landonforshage mentioned this pull request Sep 14, 2026

fix(rail): light the workspace dot for unread thread replies #215

Merged

Navigation Menu

fix(rail): make the workspace unread dot agree with the sidebar - #207

fix(rail): make the workspace unread dot agree with the sidebar#207
gammons merged 6 commits into
gammons:mainfrom
landonforshage:fix/rail-unread-agrees-with-sidebar

landonforshage commented Sep 11, 2026 •

edited

Loading

Uh oh!

gammons commented Sep 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

landonforshage commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Cause

Fix

Why not fix it elsewhere

Tests

Noted but not changed

AI assistance

Uh oh!

gammons commented Sep 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

landonforshage commented Sep 11, 2026 •

edited

Loading