Skip to content

feat(channel): channel membership control (members list/add/remove/set)#7

Merged
scottlovegrove merged 2 commits into
mainfrom
feat/channel-membership
May 27, 2026
Merged

feat(channel): channel membership control (members list/add/remove/set)#7
scottlovegrove merged 2 commits into
mainfrom
feat/channel-membership

Conversation

@scottlovegrove
Copy link
Copy Markdown
Collaborator

Summary

Ports Doist/twist-cli#244 to comms-cli. Adds per-user and per-group membership control under tdc channel:

tdc channel members <ch-ref>                       # list (default)
tdc channel members add    <ch-ref> <ref...>       # users and/or group:<ref>
tdc channel members remove <ch-ref> <ref...>
tdc channel members set    <ch-ref> <ref...> [--apply] [--include-self]
  • Refs mix freely: tdc channel members add general luis@doist.com group:Design id:789.
  • group:<ref> is one-shot expansion — adds the group's current members at call time; the group is not persistently linked, and users added to the group later will not auto-join. Surfaced in --help for add/set.
  • set replaces membership with the resolved set. Dry-run by default; --apply to mutate. Refuses to remove the acting user unless --include-self is also passed.

Files

Layer File Notes
Ref parsing src/lib/refs.ts resolveChannelMemberRefsgroup: expansion, dedup, input-order preservation; resolves users + groups concurrently
API wrappers src/lib/api.ts addUsersToChannel / removeUsersFromChannel + spinner messages
Scopes src/lib/auth-provider.ts Added channels:write + channels:remove to READ_WRITE_SCOPES
Shared helpers src/commands/channel/membership-helpers.ts mutateChannelMembership, fetchUsersByIds, logExpansion, groupsFullyInChannel
List / Add / Remove / Set src/commands/channel/{members,add,remove,set}.ts one file per command
Registration src/commands/channel/index.ts members command group; list is { isDefault: true }
Skill docs src/lib/skills/content.ts + regenerated skills/comms-cli/SKILL.md
Tests src/commands/channel/members.test.ts (12), src/lib/refs.test.ts (+4)

Adaptations from the twist-cli source

comms-cli diverges from twist-cli, so this is not a 1:1 copy:

  • String channel IDs — comms channel id is a base58 string, not numeric; addUsersToChannel/removeUsersFromChannel take id: string.
  • Group.id is a stringexpandedFrom.groupId: string.
  • WorkspaceUser.fullName (comms) instead of .name (twist).
  • No batch APIfetchUsersByIds resolves members via getWorkspaceUsers(workspaceId) instead of twist's client.batch(...).
  • getCommsClient / tdc throughout.

Caveat to verify

Scope names (channels:write / channels:remove) are assumed to mirror the existing groups scopes. The upstream PR found them missing in twist's scope list; please confirm the comms backend accepts them with a live mutation before merge. Existing logged-in users must re-run tdc auth login to pick up the new scopes, otherwise channel mutations fail with INSUFFICIENT_SCOPE.

Test plan

  • npm run type-check
  • npm run lint
  • New tests pass (members.test.ts 12, refs.test.ts +4)
  • npm run sync:skill (SKILL.md regenerated)
  • --help smoke test of the members command tree
  • Live end-to-end mutation against a real workspace (verifies the scope assumption)

Note: one pre-existing markdown-render test fails on main as well — unrelated to this change.

🤖 Generated with Claude Code

Port Doist/twist-cli#244 to comms-cli. Adds `tdc channel members`:
- list: members + groups fully present in the channel
- add/remove: users and/or `group:<ref>` (one-shot expansion at call time)
- set: replace membership with the resolved set, dry-run by default
  (--apply to mutate), refuses to remove the acting user unless --include-self

Also adds resolveChannelMemberRefs (mixed user/group: ref parsing with
dedup + input-order preservation), addUsersToChannel/removeUsersFromChannel
API wrappers, and channels:write/channels:remove to READ_WRITE_SCOPES.

Existing logged-in users must re-run `tdc auth login` to pick up the new
scopes, otherwise channel mutations fail with INSUFFICIENT_SCOPE.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove self-assigned this May 27, 2026
@doistbot doistbot requested a review from nvignola May 27, 2026 15: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 😎 👊. The new channel membership mutation commands are well-structured and nicely adapt the twist-cli logic for comms-cli.

Few things worth tightening:

  • Correctness: Watch out for a resolver mapping bug with comma-separated user refs that can silently drop assignments, and ensure dry-run mode outputs valid JSON when --json is passed.
  • Efficiency: Member listing and group expansions currently fetch the entire workspace user/group directories sequentially; we should batch group lookups and only fetch the specific users needed.
  • Testing & cleanup: Expand the test suite to cover interleaved user/group arrays and the --apply --json contract, and remove some unreachable "sparing self" dead code in set.ts.

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

Optional follow-up notes (4)
  • [P3] src/commands/channel/set.ts:57: printDryRun() always ends with Run without --dry-run to execute.. On the default set preview path that's incorrect: rerunning without --dry-run still won't mutate unless --apply is passed. Please use a custom preview message here or override the footer so the next step isn't misleading.
  • [P3] src/commands/channel/set.ts:85: When --full is used, set merges CLI result metadata (added, removed, etc.) into the API's updated channel object. This diverges from how add and remove handle --full in membership-helpers.ts (and the away.ts reference implementation), which both log the unmodified API entity. Consider standardizing the behavior by using console.log(formatJson(updated, 'channel', true)) here to match.
  • [P3] src/commands/channel/members.test.ts:8: The --full JSON output branch for mutative commands (add, remove, set) makes an API call (client.channels.getChannel) which is currently untested. If a test triggered this branch, it would crash because the getCommsClient mock here returns undefined. Consider updating this mock to return { channels: { getChannel: vi.fn() } } and adding a test case that verifies the --full expanded payload.
  • [P3] src/lib/skills/content.ts:236: This file is installed into AI agent skill directories, so the new alice / a@d.com examples push agents toward PII-based refs. The Internal AI Tools standard says AI data flows should prefer user IDs over emails/names where possible, so please switch these new examples (and the adjacent prose in this section) to id:-style refs by default and mention email/name only as optional human-facing inputs. https://handbook.doist.com/doc/standard-internal-ai-tools-zU0fqnhpmC

Share FeedbackReview Logs

Comment thread src/lib/refs.ts Outdated
Comment thread src/commands/channel/set.ts
Comment thread src/commands/channel/members.ts
Comment thread src/commands/channel/membership-helpers.ts Outdated
Comment thread src/lib/refs.ts Outdated
Comment thread src/commands/channel/members.test.ts Outdated
Comment thread src/lib/refs.test.ts
Comment thread src/commands/channel/members.test.ts
- refs.ts: resolve each user slot individually so a comma/multi-match ref
  can't shift index mapping and silently drop users (P1); fetch the
  workspace group list once for name refs instead of per-ref (P2).
- set.ts: emit a JSON object in dry-run when --json (was printing text,
  breaking parsers) (P1); drop the unreachable self-sparing ternary (the
  earlier guard already throws) (P2).
- membership-helpers.ts: fetchUsersByIds resolves members via per-id
  getUserById (mirrors groups/view) instead of downloading the whole
  workspace directory (P2).
- tests: rename the misleading "sparing self" case; add set --apply --json
  and dry-run --json coverage; add an interleaved multi-group refs test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove merged commit 2a1e473 into main May 27, 2026
5 checks passed
@scottlovegrove scottlovegrove deleted the feat/channel-membership branch May 27, 2026 15:49
doist-release-bot Bot added a commit that referenced this pull request May 27, 2026
## [1.1.0](v1.0.0...v1.1.0) (2026-05-27)

### Features

* **channel:** channel membership control (members list/add/remove/set) ([#7](#7)) ([2a1e473](2a1e473)), closes [Doist/twist-cli#244](Doist/twist-cli#244)
@doist-release-bot
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.1.0 🎉

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants