# Channel-Visibility Unification (backlog item 3) — Design

**Status:** implemented 2026-07-20 (D9) — re-verified 2026-08-04
(`permissions/checker.go:110` `VisibleChannelIDs`; four delegating call sites;
REST/WS agreement test `Server/ws/channel_visibility_agreement_test.go`)
**Decision:** D9 in [audit-2026-07-19-decisions.md](audit-2026-07-19-decisions.md)
**Closes:** audit finding A-2026-07-07 (backlog §6 item 3)

## Problem

Per the audit, the "does this role see this channel" rule is implemented ~4× with
comments telling each copy it "must mirror" the others:

- `service.ChannelService.ListVisibleChannels` (REST `GET /channels`),
- `ws.buildReady` (the ready payload's channel list),
- `ws.computeAllowedChannels` (replay-buffer filtering on reconnect),
- `ws.Hub.RefreshChannelVisibility` (targeted channel_create/delete after an
  override change).

All four inline the same predicate — `HasAdmin(perms) || EffectivePerms(perms,
allow, deny)&ReadMessages == ReadMessages` — plus the same "skip `dm` channels,
fail closed on nil role" scaffolding. The recent private-channel fixes show this
churns; a drift between copies is an information-disclosure bug.

## Approach — funnel all four through the checker that already exists

The unified predicate is **already written**: `permissions.Checker.HasChannelPermBatch`
(pre-fetched overrides map) and `.HasChannelPerm` (single-channel). Three of the
four sites bypass it by re-inlining the bit math; `RefreshChannelVisibility` does
its own `GetChannelPermissions` + `EffectivePerms`. The fix is to route every
site through the checker, not to invent a new abstraction:

1. Add one thin helper to `permissions.Checker`:
   `VisibleChannelIDs(rolePerms int64, channels []ChannelRef, overrides map[int64]ChannelOverride) map[int64]bool`
   — iterates, skips `dm`, and calls the existing `HasChannelPermBatch` per
   channel. `ChannelRef{ID int64; Type string}` is declared in `permissions`
   (the package deliberately avoids importing `db`, so callers map their
   `[]db.Channel` down to `[]ChannelRef`).
2. `ListVisibleChannels`, `buildReady`, and `computeAllowedChannels` call it.
   `buildReady` still fetches the overrides map once (it reuses it for `can_send`
   and unread), then hands the same map to `VisibleChannelIDs`.
   `computeAllowedChannels` unions the result with the user's open DM channels
   (unchanged). Nil role → empty set, which the helper yields naturally.
3. `RefreshChannelVisibility`'s per-role check calls the existing
   `HasChannelPerm(rolePerms, roleID, ch.ID, ReadMessages)` instead of its inline
   copy. Its targeted-send + `visibilityChangeSeq` watermark mechanics are
   untouched.

## Files touched

- `Server/permissions/checker.go` — add `ChannelRef` + `VisibleChannelIDs`.
- `Server/permissions/checker_test.go` — cover the helper (admin bypass, deny
  override hides, no-override inherits base, dm skipped).
- `Server/service/channel.go` — `ListVisibleChannels` delegates.
- `Server/ws/serve.go` — `buildReady`, `computeAllowedChannels` delegate; drop
  the "mirrors …" comments.
- `Server/ws/hub.go` — `RefreshChannelVisibility` uses `HasChannelPerm`.
- Docs: `docs/audit-2026-07-19.md` (A-2026-07-07 → RESOLVED),
  `docs/architecture/server.md`/`websocket.md` if a visibility callout names the
  duplication.

## Test plan

- Unit tests for `VisibleChannelIDs` in `permissions`.
- **REST/WS agreement test** (the audit's explicit ask): seed a real in-memory
  `db` with roles, per-channel overrides, and text/announcement/voice/dm
  channels; assert `ListVisibleChannels`, the channel set inside `buildReady`,
  and the non-DM subset of `computeAllowedChannels` are byte-for-byte the same
  set for the same user across admin / member-with-deny / nil-role cases.
- Existing `serve_test.go`, `hub_test.go`, `channel_handler` tests stay green
  unchanged (behavior is identical; only the code path is shared).

## Non-goals

- No change to permission bits, override storage, or the `EffectivePerms`
  algebra — only where the existing predicate is called from.
- DM visibility stays membership-based (added on top of the server-channel set,
  not routed through the checker).
- `can_send`, unread counts, and voice-state filtering in `buildReady` are
  unchanged.
- Not touching `RefreshChannelVisibility`'s targeted-send / watermark design
  (backlog 11/12 territory) — only its read predicate.