Skip to content

fix(channels): Include unjoined public channels in public-channel allowlist - #266

Merged
engfragui merged 3 commits into
mainfrom
francesca/fix-issue-263
Jun 8, 2026
Merged

fix(channels): Include unjoined public channels in public-channel allowlist#266
engfragui merged 3 commits into
mainfrom
francesca/fix-issue-263

Conversation

@engfragui

@engfragui engfragui commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Overview

getPublicChannelIds built its allowlist from getChannels alone, which is membership-scoped and excludes public channels the user hasn't joined. This wrongly rejected threads in unjoined-but-public channels as private via assertChannelIsPublic, and left a latent masking bug in filterVisibleSearchResults.

Merge getChannels with workspaces.getPublicChannels and dedupe by id, mirroring the #249 fix in resolveChannelRef.

Reference

Test Plan

  • npm run build
  • Via the UI, visit an unjoined channel (i.e. I picked "z.Cooking" but you do you)
  • Copy the link of any thread inside that unjoined channel
  • Extract the threadId from the URL (it's the last id in the URL)
  • node dist/index.js thread view id:<threadId>
  • Verify that the thread renders normally (instead of NOT_FOUND: This thread belongs to a private channel)

…owlist

getPublicChannelIds built its allowlist from getChannels alone, which is
membership-scoped and excludes public channels the user hasn't joined. This
wrongly rejected threads in unjoined-but-public channels as private via
assertChannelIsPublic, and left a latent masking bug in
filterVisibleSearchResults.

Merge getChannels with workspaces.getPublicChannels and dedupe by id,
mirroring the #249 fix in resolveChannelRef.

Fixes #263

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@engfragui engfragui self-assigned this Jun 5, 2026
@doistbot
doistbot requested a review from rfgamaral June 5, 2026 11:56
@engfragui

Copy link
Copy Markdown
Contributor Author

@doistbot /review

@engfragui engfragui added the 🙋 Ask PR PR must be reviewed before merging label Jun 5, 2026

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for putting together this fix for unjoined public channels, Francesca 😎 👊.

Few things worth tightening:

  • The getChannels API call is likely redundant and can be removed, as getPublicChannels already returns all public channels regardless of membership (unless you specifically need to capture archived joined channels).

I also included a few optional follow-up notes in the details below.

Optional follow-up note (1)
  • [P3] src/lib/public-channels.ts:17: This is now the third place that reimplements the "joined channels + getPublicChannels + dedupe by id" rule (alongside resolveChannelRef and listChannels). Since this PR exists because those call sites drifted apart, it would be better to extract a shared helper for this workspace-channel visibility logic and build the ID set from that, so future fixes only need to land once.

Share FeedbackReview Logs

Comment thread src/lib/public-channels.ts Outdated
engfragui and others added 2 commits June 5, 2026 14:27
getPublicChannels is workspace-scoped and returns every public channel
(active and archived, joined and unjoined), making it a complete superset
of the public channels getChannels would surface. Drop the redundant
getChannels call and Promise.all in getPublicChannelIds — one API call
instead of two.

Addresses review feedback on #266.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@engfragui
engfragui merged commit 6de6209 into main Jun 8, 2026
5 checks passed
@engfragui
engfragui deleted the francesca/fix-issue-263 branch June 8, 2026 08:23
doist-release-bot Bot added a commit that referenced this pull request Jun 8, 2026
## [2.45.1](v2.45.0...v2.45.1) (2026-06-08)

### Bug Fixes

* **channels:** Include unjoined public channels in public-channel allowlist ([#266](#266)) ([6de6209](6de6209)), closes [#263](#263)
@doist-release-bot

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.45.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🙋 Ask PR PR must be reviewed before merging released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

getPublicChannelIds is membership-scoped — same bug #249 fixed in refs.ts

3 participants