diff --git a/package-lock.json b/package-lock.json index 004f1cb..0aea2ce 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "license": "MIT", "dependencies": { "@doist/cli-core": "0.24.0", - "@doist/comms-sdk": "0.2.0", + "@doist/comms-sdk": "0.3.0", "@pnpm/tabtab": "0.5.4", "chalk": "5.6.2", "commander": "14.0.3", @@ -182,9 +182,9 @@ } }, "node_modules/@doist/comms-sdk": { - "version": "0.2.0", - "resolved": "https://registry.npmjs.org/@doist/comms-sdk/-/comms-sdk-0.2.0.tgz", - "integrity": "sha512-ywZEls1fYvZtKmfADvxXD0kAtw+u5P75+TraV9VkAjA509AG6Q69ldkaHo5s9SQX6YCyQAZkuv8KaGDDrRJTrw==", + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/@doist/comms-sdk/-/comms-sdk-0.3.0.tgz", + "integrity": "sha512-vfdNmTMmdKV4VTo8/uMqSY0j7e6puTjb0OWcTkC5nl11s2jGqRemrh1Jm0/SiYiAtJB7MDjvXYWk4bnsdNLSmg==", "license": "MIT", "dependencies": { "camelcase": "8.0.0", diff --git a/package.json b/package.json index 1076249..2e1af48 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ ], "dependencies": { "@doist/cli-core": "0.24.0", - "@doist/comms-sdk": "0.2.0", + "@doist/comms-sdk": "0.3.0", "@pnpm/tabtab": "0.5.4", "chalk": "5.6.2", "commander": "14.0.3", diff --git a/skills/comms-cli/SKILL.md b/skills/comms-cli/SKILL.md index 2ccabe3..52790ff 100644 --- a/skills/comms-cli/SKILL.md +++ b/skills/comms-cli/SKILL.md @@ -208,8 +208,9 @@ tdc search "query" --all # Fetch all result pages tdc user # Show current user info tdc user --json # JSON output tdc user --json --full # Include all fields in JSON output -tdc users # List workspace users +tdc users # List active workspace users tdc users --search # Filter by name/email +tdc users --include-removed # Include users removed from the workspace tdc channels # List active joined workspace channels (alias of: tdc channel list) tdc channels --state all # Include archived joined channels too tdc channels --scope discoverable # Active public channels you can see but have not joined diff --git a/src/commands/user.test.ts b/src/commands/user.test.ts index 382379f..ebde3ee 100644 --- a/src/commands/user.test.ts +++ b/src/commands/user.test.ts @@ -56,6 +56,67 @@ describeEmptyMachineOutput('tdc users empty output', { humanMessage: 'No users found.', }) +describe('tdc users --include-removed', () => { + const active = { + id: 1, + fullName: 'Active', + email: 'a@x', + userType: 'USER', + removed: false, + } + const removed = { + id: 2, + fullName: 'Ghost', + email: 'ghost@x', + userType: 'GUEST', + removed: true, + } + + beforeEach(() => { + vi.clearAllMocks() + apiMocks.getCurrentWorkspaceId.mockResolvedValue(1) + }) + + it('passes includeRemoved: undefined by default so the SDK applies its default filter', async () => { + apiMocks.getWorkspaceUsers.mockResolvedValueOnce([active]) + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'users']) + + expect(apiMocks.getWorkspaceUsers).toHaveBeenCalledWith(1, { includeRemoved: undefined }) + expect(consoleSpy.mock.calls.flat().join('\n')).not.toMatch(/\[removed\]/) + }) + + it('passes includeRemoved: true and annotates removed users in text output', async () => { + apiMocks.getWorkspaceUsers.mockResolvedValueOnce([active, removed]) + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'users', '--include-removed']) + + expect(apiMocks.getWorkspaceUsers).toHaveBeenCalledWith(1, { includeRemoved: true }) + const lines = consoleSpy.mock.calls.flat().join('\n') + expect(lines).toMatch(/id:2.*Ghost.*\[removed\]/) + expect(lines).not.toMatch(/id:1.*Active.*\[removed\]/) + }) + + it('surfaces removed in curated --json output without --full', async () => { + apiMocks.getWorkspaceUsers.mockResolvedValueOnce([active, removed]) + const program = createProgram() + const consoleSpy = captureConsole('log') + + await program.parseAsync(['node', 'tdc', 'users', '--include-removed', '--json']) + + const parsed = JSON.parse(consoleSpy.mock.calls[0][0]) + expect(parsed).toHaveLength(2) + expect(parsed[0]).toMatchObject({ id: 1, removed: false }) + expect(parsed[1]).toMatchObject({ id: 2, removed: true }) + // Curated, not --full: shortName must not leak in. + expect(parsed[0]).not.toHaveProperty('shortName') + }) +}) + describe('user --json', () => { const sampleUser = { id: 42, diff --git a/src/commands/user.ts b/src/commands/user.ts index 1a45a1c..703362b 100644 --- a/src/commands/user.ts +++ b/src/commands/user.ts @@ -11,7 +11,11 @@ import type { ViewOptions } from '../lib/options.js' import { colors, formatJson, formatNdjson, printEmpty } from '../lib/output.js' import { resolveWorkspaceRef } from '../lib/refs.js' -type UsersOptions = ViewOptions & { workspace?: string; search?: string } +type UsersOptions = ViewOptions & { + workspace?: string + search?: string + includeRemoved?: boolean +} async function showCurrentUser(options: ViewOptions): Promise { const user = await getSessionUser() @@ -54,7 +58,7 @@ async function listUsers(workspaceRef: string | undefined, options: UsersOptions workspaceId = await getCurrentWorkspaceId() } - let users = await getWorkspaceUsers(workspaceId) + let users = await getWorkspaceUsers(workspaceId, { includeRemoved: options.includeRemoved }) if (options.search) { const search = options.search.toLowerCase() @@ -85,7 +89,8 @@ async function listUsers(workspaceRef: string | undefined, options: UsersOptions const name = u.fullName const email = u.email ? colors.timestamp(`<${u.email}>`) : '' const type = colors.channel(`[${u.userType}]`) - console.log(`${id} ${name} ${email} ${type}`) + const removed = u.removed ? chalk.red(' [removed]') : '' + console.log(`${id} ${name} ${email} ${type}${removed}`) } } @@ -109,6 +114,7 @@ Examples: .description('List users in a workspace') .option('--workspace ', 'Workspace ID or name') .option('--search ', 'Filter by name/email') + .option('--include-removed', 'Include users who have been removed from the workspace') .option('--json', 'Output as JSON') .option('--ndjson', 'Output as newline-delimited JSON') .option('--full', 'Include all fields in JSON output') @@ -117,7 +123,8 @@ Examples: ` Examples: tdc users - tdc users --search "Jane" --json`, + tdc users --search "Jane" --json + tdc users --include-removed`, ) .action(listUsers) } diff --git a/src/lib/api.test.ts b/src/lib/api.test.ts new file mode 100644 index 0000000..6bd618f --- /dev/null +++ b/src/lib/api.test.ts @@ -0,0 +1,75 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +// Mock the SDK so we can observe how `getWorkspaceUsers` invokes +// `client.workspaceUsers.getWorkspaceUsers`. The real filtering of +// removed users lives in the SDK (≥0.3.0), so the contract this test +// guards is "we pass `includeRemoved` through unchanged." +const getWorkspaceUsersMock = vi.hoisted(() => vi.fn().mockResolvedValue([])) + +vi.mock('@doist/comms-sdk', () => ({ + CommsApi: class { + workspaceUsers = { getWorkspaceUsers: getWorkspaceUsersMock } + }, +})) + +vi.mock('./auth.js', () => ({ + getApiToken: vi.fn().mockResolvedValue('test-token'), +})) + +vi.mock('./permissions.js', () => ({ + ensureWriteAllowed: vi.fn(), + isMutatingMethod: vi.fn().mockReturnValue(false), +})) + +vi.mock('./spinner.js', () => ({ + withSpinner: (_label: unknown, fn: () => Promise) => fn(), +})) + +vi.mock('./progress.js', () => ({ + getProgressTracker: () => ({ isEnabled: () => false, emitApiCall: vi.fn() }), +})) + +const { clearWorkspaceUserCache, getWorkspaceUsers } = await import('./api.js') + +describe('getWorkspaceUsers', () => { + beforeEach(() => { + getWorkspaceUsersMock.mockClear() + clearWorkspaceUserCache() + }) + + it('passes includeRemoved: undefined by default so the SDK applies its default filter', async () => { + await getWorkspaceUsers(1585) + expect(getWorkspaceUsersMock).toHaveBeenCalledWith({ + workspaceId: 1585, + includeRemoved: undefined, + }) + }) + + it('forwards includeRemoved: true to the SDK', async () => { + await getWorkspaceUsers(1585, { includeRemoved: true }) + expect(getWorkspaceUsersMock).toHaveBeenCalledWith({ + workspaceId: 1585, + includeRemoved: true, + }) + }) + + it('caches active and include-removed variants separately', async () => { + // First call seeds the active-only cache entry. + await getWorkspaceUsers(1585) + // Second call (same workspace, default flag) must hit cache → no extra SDK call. + await getWorkspaceUsers(1585) + expect(getWorkspaceUsersMock).toHaveBeenCalledTimes(1) + + // Switching to include-removed must NOT collide with the active entry. + await getWorkspaceUsers(1585, { includeRemoved: true }) + expect(getWorkspaceUsersMock).toHaveBeenCalledTimes(2) + expect(getWorkspaceUsersMock).toHaveBeenLastCalledWith({ + workspaceId: 1585, + includeRemoved: true, + }) + + // And the include-removed variant is itself cached. + await getWorkspaceUsers(1585, { includeRemoved: true }) + expect(getWorkspaceUsersMock).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/lib/api.ts b/src/lib/api.ts index afe7e82..ee54e3c 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -305,19 +305,30 @@ export async function getSessionUser(): Promise { return sessionUserCache } -const workspaceUserCache = new Map() +const workspaceUserCache = new Map() -async function loadWorkspaceUsers(workspaceId: number): Promise { - const cached = workspaceUserCache.get(workspaceId) +function workspaceUserCacheKey(workspaceId: number, includeRemoved: boolean | undefined): string { + return `${workspaceId}:${includeRemoved ? 'all' : 'active'}` +} + +async function loadWorkspaceUsers( + workspaceId: number, + includeRemoved: boolean | undefined, +): Promise { + const key = workspaceUserCacheKey(workspaceId, includeRemoved) + const cached = workspaceUserCache.get(key) if (cached) return cached const client = await getCommsClient() - const users = await client.workspaceUsers.getWorkspaceUsers({ workspaceId }) - workspaceUserCache.set(workspaceId, users) + const users = await client.workspaceUsers.getWorkspaceUsers({ workspaceId, includeRemoved }) + workspaceUserCache.set(key, users) return users } -export async function getWorkspaceUsers(workspaceId: number): Promise { - return loadWorkspaceUsers(workspaceId) +export async function getWorkspaceUsers( + workspaceId: number, + options: { includeRemoved?: boolean } = {}, +): Promise { + return loadWorkspaceUsers(workspaceId, options.includeRemoved) } export function clearWorkspaceUserCache(): void { @@ -335,14 +346,21 @@ export async function buildUserNameMap( workspaceId: number, client?: CommsApi, ): Promise> { - const cached = workspaceUserCache.get(workspaceId) + // Historical messages can reference users who have since been removed; + // those messages should still render the author's name rather than fall + // back to `user:123`. So this map always pulls the full roster. + const key = workspaceUserCacheKey(workspaceId, true) + const cached = workspaceUserCache.get(key) let users: WorkspaceUser[] if (cached) { users = cached } else { const api = client ?? (await getCommsClient()) - users = await api.workspaceUsers.getWorkspaceUsers({ workspaceId }) - workspaceUserCache.set(workspaceId, users) + users = await api.workspaceUsers.getWorkspaceUsers({ + workspaceId, + includeRemoved: true, + }) + workspaceUserCache.set(key, users) } return new Map(users.map((u) => [u.id, u.fullName])) } diff --git a/src/lib/output.ts b/src/lib/output.ts index 61ef630..d0b3287 100644 --- a/src/lib/output.ts +++ b/src/lib/output.ts @@ -58,7 +58,14 @@ const MESSAGE_ESSENTIAL_FIELDS = [ const WORKSPACE_ESSENTIAL_FIELDS = ['id', 'name', 'creator', 'plan'] as const -const USER_ESSENTIAL_FIELDS = ['id', 'fullName', 'email', 'timezone', 'userType'] as const +const USER_ESSENTIAL_FIELDS = [ + 'id', + 'fullName', + 'email', + 'timezone', + 'userType', + 'removed', +] as const const CHANNEL_ESSENTIAL_FIELDS = [ 'id', diff --git a/src/lib/skills/content.ts b/src/lib/skills/content.ts index ee05343..d33d158 100644 --- a/src/lib/skills/content.ts +++ b/src/lib/skills/content.ts @@ -212,8 +212,9 @@ tdc search "query" --all # Fetch all result pages tdc user # Show current user info tdc user --json # JSON output tdc user --json --full # Include all fields in JSON output -tdc users # List workspace users +tdc users # List active workspace users tdc users --search # Filter by name/email +tdc users --include-removed # Include users removed from the workspace tdc channels # List active joined workspace channels (alias of: tdc channel list) tdc channels --state all # Include archived joined channels too tdc channels --scope discoverable # Active public channels you can see but have not joined