fix(users): filter removed members from tdc users by default#10
Merged
Conversation
`tdc users` was listing users who had been removed from the workspace.
Same root cause as twist-cli#245: the Comms API returns both active and
removed members with a `removed: boolean` flag. Comms SDK 0.3.0 adds an
`includeRemoved` option that filters `removed === true` by default; this
change bumps the SDK and threads the option through.
- Bump @doist/comms-sdk 0.2.0 → 0.3.0
- `getWorkspaceUsers` accepts `{ includeRemoved? }`; cache keyed by both
workspaceId and the flag
- `tdc users --include-removed` opts back into the historical roster and
renders a red `[removed]` chip
- `removed` added to curated user JSON fields
- `resolveUserRefs` / `resolveNotifyIds` now default to active-only,
fixing the silent "User N is not part of workspace" 403s on
channel/group writes that resolve a tombstoned name
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
reviewed
May 28, 2026
Member
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
Thanks scottlovegrove for your contribution 🤗. The upgrade to Comms SDK 0.3.0 and the new --include-removed flag cleanly solve the 403 errors by properly handling tombstoned users during reference lookups.
Few things worth tightening:
- Update
buildUserNameMapto explicitly fetch removed members (includeRemoved: true), ensuring historical messages from departed users don't fall back to displaying raw IDs. - Add a test to verify that the active-only and include-removed cache variants don't collide, confirming two separate API calls are made when both configurations are requested.
Addresses PR #10 review feedback: - `buildUserNameMap` now fetches `includeRemoved: true`. Historical messages from members who have since been removed need to render the author's name, not fall back to `user:123`. The active-only cache entry is left alone for ref-resolution callers. - Add a cache-collision test in `api.test.ts` that calls `getWorkspaceUsers` with both default and `includeRemoved: true` to prove the cache keys don't collide and each variant is itself cached. 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
## [1.3.1](v1.3.0...v1.3.1) (2026-05-28) ### Bug Fixes * **users:** filter removed members from `tdc users` by default ([#10](#10)) ([31e185c](31e185c))
Contributor
|
🎉 This PR is included in version 1.3.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Port of Doist/twist-cli#245 to comms-cli.
tdc userswas listing users who had been removed from the workspace, pollutingtdc users [--json]output and silently breaking name/email resolution — any command that takes a user ref (tdc channel add,tdc group add-user,tdc thread …with notify) could resolve to a tombstoned user and then 403 from the backend withUser N is not part of workspace.Root cause: the Comms API's
workspace_users/getreturns both active and removed members with aremoved: booleanflag. The SDK already exposed the field but the CLI ignored it. Comms SDK 0.3.0 adds anincludeRemovedoption that filtersremoved === trueby default, mirroring@doist/twist-sdk@2.8.1.Changes
package.json,package-lock.json@doist/comms-sdk0.2.0 → 0.3.0src/lib/api.tsgetWorkspaceUsers(workspaceId, { includeRemoved? }); cache keyed by(workspaceId, includeRemoved)so the two variants don't collide. Fixestdc users,resolveUserRefs(src/lib/refs.ts), andresolveNotifyIds(src/commands/thread/helpers.ts) in one shot.src/commands/user.ts--include-removedflag. Human output appends a red[removed]chip.src/lib/output.tsremovedadded toUSER_ESSENTIAL_FIELDSso callers can distinguish the two states without--full.src/lib/skills/content.ts,skills/comms-cli/SKILL.mdtdc users --include-removedexample.src/commands/user.test.ts--jsonexposesremovedwithout leakingshortName.src/lib/api.test.tsincludeRemovedto the SDK unchanged.Why a flag, not always filter
Audit / recovery cases occasionally do want the historical roster (e.g. "who used to be in here?").
--include-removedcovers that without making the common case noisy.Test plan
npm run type-checknpm run lint:checknpm run check:skill-syncnpm test— 689/689 passing (5 new)tdc users --json | jq 'length'vstdc users --include-removed --json | jq 'length'→ counts differtdc users --include-removedshows red[removed]chip for removed memberstdc users --json | jq '.[0] | has("removed")'→true🤖 Generated with Claude Code