feat(channel): channel membership control (members/add/remove/sync)

[lmjabreu]

Summary

Adds per-user and per-group membership control to tw channel. Follows the plan in docs/CHANNEL-MEMBERSHIP-PLAN.md, with the command surface revised in review.

tw channel members <ch-ref>                          # list (default)
tw channel members add    <ch-ref> <ref...>          # users and/or group:<ref>
tw channel members remove <ch-ref> <ref...>
tw channel members set    <ch-ref> <ref...> [--apply] [--include-self]

• Refs can be mixed: tw 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 to the channel, and users added later to the group will not auto-join. Surfaced in --help for add and set.
• set replaces the channel's membership with the resolved set. Dry-run by default; pass --apply to mutate.
• set refuses to remove the acting user unless --include-self is also passed.

What's in the PR

One file per command, mirroring the rest of src/commands/channel/ (list.ts, threads.ts, etc.):

Layer	File	Notes
Ref parsing	src/lib/refs.ts	resolveChannelMemberRefs handles group: prefix; reuses resolveUserRefs + resolveGroupRef; preserves input order across mixed refs; resolves user and group lookups concurrently
API wrappers	src/lib/api.ts	addUsersToChannel / removeUsersFromChannel + spinner messages
Shared helpers	src/commands/channel/membership-helpers.ts	mutateChannelMembership, fetchUsersByIds (via getOptionalBatchData), logExpansion, groupsFullyInChannel
List	src/commands/channel/members.ts	listChannelMembers
Add	src/commands/channel/add.ts	Thin wrapper around mutateChannelMembership
Remove	src/commands/channel/remove.ts	Same shape as add
Set	src/commands/channel/set.ts	Dry-run-by-default, --apply, --include-self guard
Registration	src/commands/channel/index.ts	members is a command group; list is its { isDefault: true } subcommand
Tests	src/commands/channel/{members,add,remove,set}.test.ts + src/lib/refs.test.ts	One test file per command; new helper-level coverage for resolveChannelMemberRefs
Skill docs	src/lib/skills/content.ts	Quick Reference rows + group-expansion caveat; regenerated skills/twist-cli/SKILL.md
Scope fix	src/lib/auth-provider.ts	Added channels:write and channels:remove to READ_WRITE_SCOPES (see side discoveries)

CHANGELOG is not edited — semantic-release will generate it from the conventional-commit messages on the next release.

End-to-end test

Ran against the real Doist workspace (ws:1585) on a throwaway channel "Testing Group Setting Via CLI" (id:837185), with group id:32017 "25Q3|Engineering: Resharding" (3 users, none of them me (Luis)) and user id:252356 Ben (not in the group, not me (Luis)):

Step	Command	Result
Baseline	tw channel members <ch>	1 member (just me (Luis)) ✓
Dry-run add	tw channel members add <ch> group:32017 id:252356 --dry-run	Preview: 4 users to add (3 from group + Ben), dedupe correct ✓
Apply add	tw channel members add <ch> group:32017 id:252356	"Added 4 users … (now 5 members)" ✓
Verify	tw channel members <ch>	All 5 listed; "Groups fully in channel" correctly identifies Resharding ✓
Remove user	tw channel members remove <ch> id:252356	"Removed 1 user … (now 4 members)" ✓
Set self-guard	tw channel members set <ch> id:252356	Refused with INVALID_VALUE (would remove me (Luis)) ✓
Set dry-run	tw channel members set <ch> id:252356 id:447642	Preview: +1 / -3, dry-run by default ✓
Set apply	tw channel members set <ch> … --apply	Channel reset to {me (Luis), Ben}; "Set … +1 / -3 (now 2 members)" ✓
Restore	tw channel members remove <ch> id:252356	Back to baseline ✓

Channel was deleted afterwards via the Twist UI.

Note: the live commands above were run with the earlier tw channel add/remove/sync surface. The behaviour and refs are identical under the renamed tw channel members add/remove/set nesting; the post-rename build was smoke-tested via --help output and the full unit-test suite.

Side discoveries

Found while developing/testing — flagged so reviewers know they exist:

1. Fixed in this PR — channels:write and channels:remove were missing from READ_WRITE_SCOPES in src/lib/auth-provider.ts. Without them every channel mutation returned INSUFFICIENT_SCOPE. Existing logged-in users will need to re-run tw auth login to pick up the new scopes.
2. Out of scope, follow-up needed — tw channel create and tw channel delete don't exist in the CLI despite the SDK exposing both. End-to-end testing required creating/deleting the channel in the Twist UI. Worth adding to mirror tw groups create/delete.
3. Out of scope, follow-up needed — tw users appears to return users outside the current workspace. Test User1 (id:2342) showed up in tw users output but the channel API rejected the add with User 2342 is not part of workspace 1585. The list scope of tw users likely needs a workspace filter.

Review history

• doistbot: 15 inline comments (2 P1, 12 P2, 1 P3) — all addressed except one deferred refactor (extracting a shared mutation flow with groups/members.ts); see response.
• @scottlovegrove (1st pass): split commands into separate files; split tests by command — addressed in 3f3652f.
• @scottlovegrove (2nd pass): nest commands under members; rename sync → set; switch interface → type on ChannelMemberRefs; fix <ref> → <channel-ref> in skill docs; reset stray package-lock.json diff — addressed in fc5dda2 (plus a follow-up lockfile fix after the initial reset used a stale local main).

Checks

• npm run type-check
• npm run lint
• npm test (677 passing, including new helper-level tests for resolveChannelMemberRefs)
• npm run build && npm run sync:skill
• Manual end-to-end (see above)
• CI green (lint, test, SKILL.md Sync, Validate Title)

[lmjabreu]

Closing while work is in progress; will reopen when ready for review.

[doistbot]

This PR introduces comprehensive channel membership control to the CLI, adding commands to list, add, remove, and sync members using both user and one-shot group references. The feature provides a highly flexible and powerful tool for managing channel participation across the workspace. There are a few areas that need refinement, including updating batch API access to adhere to repository guidelines, replacing sequential network network waterfalls with concurrent fetching, extracting shared membership mutation logic, and expanding test coverage for the new reference parsers and output flags.

_{Share Feedback • Review Logs}

[doistbot]

[P1] AGENTS.md mandates that you should never access .data directly on a batch result, as this bypasses API error handling. Replace this manual loop with the buildOptionalBatchNameMap helper from src/lib/api.ts (e.g., return buildOptionalBatchNameMap(userIds, responses, 'user')).

[doistbot]

[P2] --ndjson always emits the full channel object here, even without --full, while the --json path emits the slim { id, name, workspaceId, members, groupsFullyInChannel } payload by default. That makes the machine-readable contract depend on which flag the user picked and leaves no way to get non-full NDJSON output. Build the same default payload for NDJSON and only switch to the full object when options.full is set.

[doistbot]

[P2] Splitting refs into userRefs and groupRefs loses the original mixed order. For example, ['group:Backend', 'alice'] resolves as alice first and the group members second, so the resulting added/removed arrays and dry-run output no longer match the input order. If input-order dedupe is part of the contract, resolve the refs in a single pass (or preserve an ordered expansion list) instead of batching all users first.

[doistbot]

[P2] resolveChannelMemberRefs already deduplicates IDs internally via a Set before returning them. This dedupe call is redundant, and the local helper function can be removed entirely.

[doistbot]

[P3] This logic for logging group expansions is duplicated exactly in syncChannelMembers at line 322. Consider extracting it to a shared helper function.

[doistbot]

[P2] User and group references are resolved sequentially, causing a network waterfall (one API call per group). Execute resolveUserRefs and all resolveGroupRef calls concurrently with Promise.all.

[doistbot]

[P2] mockGetUserById is a bare vi.fn() without a return value, causing the implementation's getUserById calls to return undefined. Because mockBatch resolves blindly regardless of arguments, the construction of the batch requests is effectively untested. Consider having mockGetUserById return a dummy request object and asserting that mockBatch actually receives those objects.

[doistbot]

[P2] mockGetChannel is wired up here but never exercised because the --full flag is completely untested across add, remove, and sync. Adding a test for --full will ensure the secondary API call and JSON formatting are properly verified.

[doistbot]

[P1] This test doesn't actually verify group expansion or dedupe. Both behaviors are precomputed in the mocked resolveChannelMemberRefs result, and there isn't any companion src/lib/refs.test.ts coverage for the new helper. Please add a focused helper-level test for mixed user + group: refs, input-order dedupe, and invalid group: input; otherwise the core feature added in this PR is protected only by mocks.

[doistbot]

[P2] The new remove subcommand only gets side-effect assertions here. Its separate --dry-run, --json, and --full branches aren't exercised at all, so regressions in those user-facing modes will slip through. Add at least one dry-run case and one machine-output case for remove.

[lmjabreu]

Addressed doistbot's feedback in 02c9f04..3f3652f (squashed mentally: review fixes + chore to untrack a lock file).

Done:

• [P1] members.ts:42 — use getOptionalBatchData instead of accessing .data directly.
• [P1] members.test.ts:137 — added focused helper-level coverage for resolveChannelMemberRefs in src/lib/refs.test.ts: mixed user+group: refs, input-order dedupe, invalid group: rejection, case-insensitive prefix.
• [P2] refs.ts:427 — input order is now preserved across mixed user/group: refs (['id:30', 'group:100'] resolves to [30, 10, 20], not [10, 20, 30]).
• [P2] refs.ts:456 — resolveUserRefs + all resolveGroupRef calls now run concurrently in one Promise.all.
• [P2] members.ts:64, :189, :255 — independent fetches now run concurrently (resolveChannelRef + resolveChannelMemberRefs; resolveChannelRef + getWorkspaceGroups; channel + session-user + member-refs in sync).
• [P2] members.ts:69 — dropped redundant dedupe(); resolveChannelMemberRefs already dedupes via a Set.
• [P2] members.ts:223 — --ndjson now honours --full the same way --json does (slim default, raw on --full).
• [P2] members.ts:295 — add and remove run concurrently in sync when both are non-empty.
• [P2] members.test.ts:60 — mockGetUserById now returns a tagged sentinel and we assert batch() receives those exact requests (new test batches one getUserById request per channel member).
• [P2] members.test.ts:61 — --full is now exercised across add, remove, and the members view.
• [P2] members.test.ts:234 — added remove --dry-run and remove --json coverage.
• [P3] members.ts:137 — extracted a logExpansion helper used by both mutate and sync.

Deferred ([P2] members.ts:57): extracting a shared membership-mutation flow between src/commands/groups/members.ts and src/commands/channel/members.ts.

I see the duplication and agree it'll drift. I'm punting it to a follow-up PR for two reasons:

1. The shared helper would have to touch groups/members.ts, which is outside this PR's scope and out of the existing 700–1000 LOC-per-PR target (the @ScottL guideline from 2026-05-19). Bundling it would put the diff well over.
2. The two flows have small but real divergences (group expansion + the expandedFrom reporting only matter for channels) that the shared helper would need to abstract cleanly. I'd rather design that with the duplication visible than rush an abstraction.

Happy to open the follow-up immediately after this lands. Let me know if you'd rather I roll it in here.

[scottlovegrove]

A couple of big changes required, once they are done, I'll go through the PR again.

[lmjabreu]

Done in 0c40d20 (@scottlovegrove).

Code split — src/commands/channel/ now follows the same one-file-per-command shape as the rest of the folder:

• add.ts, remove.ts, sync.ts — one handler each.
• members.ts — now only the list handler.
• membership-helpers.ts — shared bits (mutateChannelMembership, fetchUsersByIds, logExpansion, describeExpansion, channelUserIds, groupsFullyInChannel).
• index.ts — imports updated.

I considered keeping the shared helpers inside the existing helpers.ts, but that file is narrow (just cursor encode/decode) and the membership bits are a meaningful chunk. Happy to merge if you'd prefer.

Tests split — one file per command:

• members.test.ts — only the list tests.
• add.test.ts, remove.test.ts, sync.test.ts — each with its own minimal mock setup (only the mocks each command actually exercises).

677 tests still passing (43 test files, +3 from the split). Type-check and lint clean. Ready for your second pass when you've got a moment.

[scottlovegrove]

I'm still not 100% sold on the API surface of a lot of the commands here.

• tw channel add - Sounds like a command you'd use to add a new channel, whereas we are using it for adding members
• tw channel sync - Sync what? It's not immediately apparent what is going on unless you do the help flag. This could even be tw channel members set.
• tw channel remove - Similar to add, sounds like the command you'd use to delete the channel.

I think a lot of the member-related ones should be sub-commands of the members command, ensures the context is right.

[scottlovegrove]

Suggested change

	tw channel sync <ref> group:Squad --apply # Mutate. Refuses to remove self unless --include-self
	tw channel sync <channel-ref> group:Squad --apply # Mutate. Refuses to remove self unless --include-self

Let's keep this consistent with the line above

[scottlovegrove]

What changed on this file? Should be nothing, right? Ask your agent to reset this file.

[scottlovegrove]

Prefer type instead of interface.

[lmjabreu]

Addressed in fc5dda2 (@scottlovegrove):

Naming + nesting. Commands now live under members so the noun is explicit:

tw channel members <ref>              # list (default)
tw channel members add    <ref> ...
tw channel members remove <ref> ...
tw channel members set    <ref> ...   # renamed from `sync`

members is now a pure command group; list is its { isDefault: true } subcommand. That keeps tw channel members <ref> working as the no-arg list while preventing the parent's --json from shadowing the mutating subcommands' --json (which surfaced as broken tests during the refactor — easy to miss without the existing coverage).

I went with set over sync because (a) it reads as the replace semantic and (b) it pairs naturally with add / remove. Happy to switch back if you'd rather.

Inline fixes.

• src/lib/refs.ts:404 — interface ChannelMemberRefs → type ChannelMemberRefs.
• src/lib/skills/content.ts:231 — <ref> → <channel-ref> for consistency; whole Quick Reference + notes paragraph updated to the new structure. SKILL.md regenerated.
• package-lock.json reset to main. (Your read was right: that diff was leftover from an npm i during the cli-core OAuth dust-up earlier and shouldn't have been committed.)

677 tests still passing, type-check + lint clean. Smoke-tested --help output for tw channel members and tw channel members set --help locally to confirm the new shape renders cleanly.

Re-requesting your review when you've got a moment.

[scottlovegrove]

Just one small nit that needs sorting, but other than that, approving.

[scottlovegrove]

The casting of Promise<User> is not required.

Suggested change

	getSessionUser() as Promise<User>,
	getSessionUser(),

[doist-release-bot]

🎉 This PR is included in version 2.43.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀