Skip to content

feat(channel): add create, delete, archive, unarchive subcommands - #246

Merged
lmjabreu merged 2 commits into
mainfrom
lmjabreu/determined-poincare-43dd3d
May 27, 2026
Merged

feat(channel): add create, delete, archive, unarchive subcommands#246
lmjabreu merged 2 commits into
mainfrom
lmjabreu/determined-poincare-43dd3d

Conversation

@lmjabreu

Copy link
Copy Markdown
Contributor

Summary

  • Adds tw channel create, tw channel delete, tw channel archive, tw channel unarchive — wraps the four SDK endpoints that were already exposed but not surfaced in the CLI. Discovered as a gap while landing channel-membership commands in #244; end-to-end channel testing previously required the web UI.
  • create supports --workspace, --description, --private (default public), --dry-run, --json, --full.
  • delete requires --yes to mutate, with structured MISSING_YES_FLAG CliError in --json mode, and a clear FORBIDDEN CliError when the Twist API returns 403 (channel deletion is typically restricted to workspace admins).
  • archive / unarchive mirror the same shape with --dry-run and --json.
  • Skill quick-reference + SKILL.md regenerated.

Test plan

  • npm run type-check
  • npm run lint
  • npm test — 667/667 passing (17 new tests across the four commands)
  • Live smoke against the Doist workspace: create → verify in list → archive → list --state archived → unarchive → re-archive
  • Live-verified the FORBIDDEN error message on delete --yes (Twist returns 403 for non-admins, as expected)
  • Live-verified successful delete — blocked by workspace admin policy; unit-tested only

🤖 Generated with Claude Code

@doistbot
doistbot requested a review from nvignola May 22, 2026 11:09

@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.

This PR successfully bridges a crucial functionality gap by introducing subcommands for creating, deleting, archiving, and unarchiving channels. These additions will significantly improve the CLI's utility for workspace administration and end-to-end testing. There are a few opportunities for refinement, specifically around extracting shared logic for delete confirmations and archive toggling, broadening the channel resolution bounds to include unjoined or archived channels, and addressing minor gaps in test coverage, error handling, and agent documentation.

Share FeedbackReview Logs

Comment thread src/commands/channel/delete.ts
Comment thread src/commands/channel/index.ts
options: DeleteChannelOptions,
): Promise<void> {
const workspaceId = await getCurrentWorkspaceId()
const channel = await resolveChannelRef(ref, workspaceId)

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.

[P2] resolveChannelRef() only does name matching against getChannels({ workspaceId }), which this codebase treats as the joined-channel set. That means tw channel delete "Foo" can't resolve a public channel the admin hasn't joined, and archived channels can't be deleted by name at all. The same lookup pattern is used in archive/unarchive. For these management commands, use a broader lookup (public + archived) or tighten the accepted-ref docs/examples.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed, but this is the existing behaviour of resolveChannelRef from before this PR — it bites every command that uses it, not just these. I've drafted a focused follow-up that widens the name-resolution to also cover public and archived channels (and removes the temporary pass id: for archived channels signpost from the unarchive help). Keeping this PR scoped to the create/delete/archive surface so the fix can land as one self-contained change. Leaving this thread open until that PR lands.

Comment thread src/lib/skills/content.ts Outdated
Comment thread src/commands/channel/archive.ts Outdated
Comment thread src/commands/channel/archive.ts Outdated
Comment thread src/commands/channel/delete.ts Outdated
Comment thread src/commands/channel/channel.test.ts Outdated
Comment thread src/commands/channel/channel.test.ts
Comment thread src/commands/channel/channel.test.ts
Comment thread src/lib/api.ts Outdated
lmjabreu and others added 2 commits May 27, 2026 15:59
Wraps the SDK's createChannel, deleteChannel, archiveChannel and
unarchiveChannel endpoints so channel lifecycle no longer requires
the Twist web UI. Includes a clear FORBIDDEN error when the API
rejects deletion (typically restricted to workspace admins), plus
unit tests and skill-content updates.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add FORBIDDEN to the ErrorCode union.
- Use SDK CreateChannelArgs type for the createChannel wrapper.
- Use TwistRequestError instanceof for the 403 check in delete.
- Reorder delete guards: short-circuit --json without --yes before
  hitting workspace/channel lookups, and gate --dry-run before --yes.
- Add --workspace flag to delete/archive/unarchive for parity with
  other channel subcommands.
- Extract setArchiveState helper to share archive/unarchive flow,
  and skip the API call when the channel is already in the target
  state.
- Update skill content to mention --workspace and the no-op archive
  behaviour, regenerate SKILL.md.
- Tests: fix unrestored console.log spies in create tests, add
  --json --full create test, add --yes --dry-run delete test, add
  already-archived no-op test, and rework the 403 test to throw a
  real TwistRequestError.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lmjabreu
lmjabreu force-pushed the lmjabreu/determined-poincare-43dd3d branch from 95817ad to e8d108d Compare May 27, 2026 15:01
@lmjabreu
lmjabreu merged commit 62dbda9 into main May 27, 2026
5 checks passed
@lmjabreu
lmjabreu deleted the lmjabreu/determined-poincare-43dd3d branch May 27, 2026 15:01
doist-release-bot Bot added a commit that referenced this pull request May 27, 2026
## [2.44.0](v2.43.1...v2.44.0) (2026-05-27)

### Features

* **channel:** add create, delete, archive, unarchive subcommands ([#246](#246)) ([62dbda9](62dbda9))
@doist-release-bot

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.44.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

lmjabreu added a commit that referenced this pull request May 28, 2026
## 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: `tw 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 as `src/commands/channel/list.ts:177-180`
already uses for the `--scope public` path.

## Why this exists

The bug was first suspected to be "active vs archived" — that turned out
to be wrong: `getChannels({ workspaceId })` already returns both states
(Twist's API is tri-state — see `list.ts:163-169`). The real axis is
**joined vs unjoined**, which I confirmed live and which also holds for
non-admin users (`getPublicChannels` is not admin-gated). Scale check in
Doist workspace 1585: 998 public channels, 829 of which I haven't joined
— so the bug bites in production, not just edge cases.

## Test plan

- [x] `npm run lint` — clean
- [x] `npm run type-check` — no errors in `refs.ts` / `refs.test.ts`
(pre-existing `@doist/cli-core` export errors elsewhere on `main` are
unrelated)
- [x] `npx vitest run src/lib/refs.test.ts` — **71/71 pass** (4 new
tests: unjoined-public exact match, unjoined-public substring match,
dedup of joined-and-public, ambiguity across joined+public)
- [x] **Live smoke** in workspace 1767 (Testing Environment), against a
fresh test channel — no existing channels touched:
1. `tw channel create tw-cli-resolver-smoke-<ts> --workspace 1767` → id
837258
  2. Left the channel via SDK `removeUser`
3. `tw channel archive "tw-cli-resolver-smoke-<ts>" --workspace 1767` →
`{ id: 837258, archived: true }` ✅ (previously: `CHANNEL_NOT_FOUND`)
  4. `tw channel delete id:837258 --workspace 1767 --yes` → cleaned up

Smoke required #246's `channel create/archive/delete` commands to
exercise an action on the resolved channel; they were checked out
temporarily and reverted after.

## Out of scope

- Help-text cleanup on `tw channel unarchive` in #246's
`src/commands/channel/index.ts` — separate trivial follow-up once both
this and #246 land.
- `id:` and URL branches — already work for archived and unjoined
channels via `getChannel(id)`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doist-release-bot Bot added a commit that referenced this pull request May 28, 2026
## [2.44.2](v2.44.1...v2.44.2) (2026-05-28)

### Bug Fixes

* **refs:** resolve unjoined-but-public channels by name ([#249](#249)) ([74ad85a](74ad85a)), closes [#246](#246)
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.

3 participants