Skip to content

fix(refs): resolve unjoined-but-public channels by name#11

Merged
scottlovegrove merged 2 commits into
mainfrom
fix/refs-resolve-unjoined-public-channels
May 28, 2026
Merged

fix(refs): resolve unjoined-but-public channels by name#11
scottlovegrove merged 2 commits into
mainfrom
fix/refs-resolve-unjoined-public-channels

Conversation

@scottlovegrove
Copy link
Copy Markdown
Collaborator

Summary

  • resolveChannelRef's name branch only called client.channels.getChannels({ workspaceId }), which is membership-scoped and excludes public channels the current user hasn't joined.
  • Effect: tdc channel <action> "Public Channel I Haven't Joined" fails with CHANNEL_NOT_FOUND, while id:N and URL refs work because getChannel(id) isn't membership-filtered.
  • Fix merges getChannels with workspaces.getPublicChannels and dedupes by id. Same shape src/commands/channel/list.ts:170-174 already uses for the --scope public path.

Ports Doist/twist-cli#249.

Test plan

  • npm run lint — clean
  • npm run type-check — clean
  • npx vitest run src/lib/refs.test.ts84/84 pass (4 new tests: unjoined-public exact match, unjoined-public substring match, dedup of joined-and-public, ambiguity across joined+public)

Out of scope

  • id: and URL branches — already work for unjoined channels via getChannel(id).

🤖 Generated with Claude Code

resolveChannelRef name-branch only called getChannels (membership-scoped),
so `tdc channel <action> "Public Channel I Haven't Joined"` failed with
CHANNEL_NOT_FOUND. Merge with workspaces.getPublicChannels and dedupe by
id — same shape as src/commands/channel/list.ts.

Ports Doist/twist-cli#249.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove self-assigned this May 28, 2026
@doistbot doistbot requested a review from nats12 May 28, 2026 11:29
Copy link
Copy Markdown
Member

@doistbot doistbot left a comment

Choose a reason for hiding this comment

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

Thanks scottlovegrove for your contribution 😊. Porting this fix from the twist-cli is a great catch, and the comprehensive test additions cover the new resolution paths well.

Few things worth tightening:

  • Defer the workspaces.getPublicChannels() fetch until after checking for an exact match in the joined channels list to save an unnecessary workspace-wide API call.
  • Use distinct object instances with the same ID in the deduplication test (rather than the same reference) to properly simulate the real API and ensure dedupe-by-id works as expected.

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

Optional follow-up note (1)
  • [P3] src/lib/refs.ts:279: This re-implements the same joined/public merge+d edupe rules that src/commands/channel/list.ts already has. Now that channel discoverability semantics live in two places, the next visibility/API change can fix one path and miss the other again. Please extract a small shared helper for this merged channel set and reuse it here and in channel/list.

Share FeedbackReview Logs

Comment thread src/lib/refs.ts Outdated
Comment thread src/lib/refs.test.ts Outdated
- Skip workspaces.getPublicChannels when an exact match already exists in
  the joined list. Common case (user types channel they're in) avoids the
  workspace-wide call.
- Dedupe test now uses distinct object instances with the same id, so it
  proves dedupe-by-id rather than reference equality.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label May 28, 2026
@scottlovegrove scottlovegrove merged commit 0e2aefb into main May 28, 2026
5 checks passed
@scottlovegrove scottlovegrove deleted the fix/refs-resolve-unjoined-public-channels branch May 28, 2026 11:38
doist-release-bot Bot added a commit that referenced this pull request May 28, 2026
## [1.3.2](v1.3.1...v1.3.2) (2026-05-28)

### Bug Fixes

* **refs:** resolve unjoined-but-public channels by name ([#11](#11)) ([0e2aefb](0e2aefb))
@doist-release-bot
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.3.2 🎉

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

released 👀 Show PR PR must be reviewed before or after merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants