refactor(errors): centralize 403 FORBIDDEN translation in wrapResult

src/commands/channel/channel.test.ts

@@ -582,19 +582,6 @@ describe('tw channel delete', () => {
         expect(apiMocks.deleteChannel).not.toHaveBeenCalled()
     })
 
-    it('translates a 403 from the API into a FORBIDDEN CliError', async () => {
-        const apiError = Object.assign(new Error('Request failed with status 403'), {
-            httpStatusCode: 403,
-            responseData: {},
-        })
-        apiMocks.deleteChannel.mockRejectedValueOnce(apiError)
-        const program = createProgram()
-
-        await expect(
-            program.parseAsync(['node', 'tw', 'channel', 'delete', 'Engineering', '--yes']),
-        ).rejects.toMatchObject({ code: 'FORBIDDEN' })
-    })
-
     it('outputs JSON result with --yes --json', async () => {
         const program = createProgram()
         const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {})

src/commands/channel/delete.ts

@@ -6,15 +6,6 @@ import { resolveChannelRef } from '../../lib/refs.js'
 
 type DeleteChannelOptions = MutationOptions & { yes?: boolean }
 
-function isForbidden(error: unknown): boolean {
-    return (
-        typeof error === 'object' &&
-        error !== null &&
-        'httpStatusCode' in error &&
-        (error as { httpStatusCode: number }).httpStatusCode === 403
-    )
-}
-
 export async function deleteChannelCommand(
     ref: string,
     options: DeleteChannelOptions,
@@ -42,21 +33,7 @@ export async function deleteChannelCommand(
         return
     }
 
-    try {
-        await deleteChannel(channel.id)
-    } catch (error) {
-        if (isForbidden(error)) {
-            throw new CliError(
-                'FORBIDDEN',
-                `Twist refused to delete "${channel.name}" (id:${channel.id}): 403 Forbidden.`,
-                [
-                    'Channel deletion is typically restricted to workspace admins',
-                    'Ask a workspace admin to delete it, or use the Twist web UI',
-                ],
-            )
-        }
-        throw error
-    }
+    await deleteChannel(channel.id)
 
     if (options.json) {
         console.log(formatJson({ id: channel.id, deleted: true }))

src/lib/api.test.ts

@@ -0,0 +1,88 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const sdkMocks = vi.hoisted(() => ({
+    deleteChannel: vi.fn(),
+}))
+
+vi.mock('@doist/twist-sdk', () => {
+    class TwistApi {
+        channels = { deleteChannel: sdkMocks.deleteChannel }
+        constructor(_token: string) {}
+    }
+    return {
+        TwistApi,
+        TwistRequestError: class TwistRequestError extends Error {
+            constructor(
+                message: string,
+                public httpStatusCode: number,
+                public responseData?: unknown,
+            ) {
+                super(message)
+            }
+        },
+    }
+})
+
+vi.mock('./auth.js', () => ({
+    getApiToken: vi.fn().mockResolvedValue('test-token'),
+    getAuthMetadata: vi.fn().mockResolvedValue({ authMode: 'full' }),
+}))
+
+vi.mock('./spinner.js', () => ({
+    withSpinner: (_config: unknown, fn: () => Promise<unknown>) => fn(),
+}))
+
+describe('wrapResult — central 403 translation', () => {
+    beforeEach(() => {
+        vi.clearAllMocks()
+        vi.resetModules()
+    })
+
+    it('translates a plain 403 into a FORBIDDEN CliError', async () => {
+        sdkMocks.deleteChannel.mockRejectedValueOnce(
+            Object.assign(new Error('Request failed with status 403'), {
+                httpStatusCode: 403,
+                responseData: {},
+            }),
+        )
+
+        const { createWrappedTwistClient } = await import('./api.js')
+        const client = createWrappedTwistClient('test-token')
+
+        await expect(client.channels.deleteChannel(500)).rejects.toMatchObject({
+            code: 'FORBIDDEN',
+        })
+    })
+
+    it('prefers INSUFFICIENT_SCOPE over FORBIDDEN when error_string indicates scope', async () => {
+        sdkMocks.deleteChannel.mockRejectedValueOnce(
+            Object.assign(new Error('Request failed with status 403'), {
+                httpStatusCode: 403,
+                responseData: { error_string: 'Insufficient scope provided: channels:write' },
+            }),
+        )
+
+        const { createWrappedTwistClient } = await import('./api.js')
+        const client = createWrappedTwistClient('test-token')
+
+        await expect(client.channels.deleteChannel(500)).rejects.toMatchObject({
+            code: 'INSUFFICIENT_SCOPE',
+        })
+    })
+
+    it('passes non-403 errors through untranslated', async () => {
+        sdkMocks.deleteChannel.mockRejectedValueOnce(
+            Object.assign(new Error('Request failed with status 500'), {
+                httpStatusCode: 500,
+                responseData: {},
+            }),
+        )
+
+        const { createWrappedTwistClient } = await import('./api.js')
+        const client = createWrappedTwistClient('test-token')
+
+        await expect(client.channels.deleteChannel(500)).rejects.toThrow(
+            'Request failed with status 500',
+        )
+    })
+})

src/lib/api.ts

@@ -9,7 +9,7 @@ import {
 } from '@doist/twist-sdk'
 import { getApiToken } from './auth.js'
 import { getConfig, updateConfig } from './config.js'
-import { CliError, isInsufficientScope } from './errors.js'
+import { CliError, isForbidden, isInsufficientScope } from './errors.js'
 import { ensureWriteAllowed, isMutatingMethod } from './permissions.js'
 import { getProgressTracker } from './progress.js'
 import { withSpinner } from './spinner.js'
@@ -194,6 +194,12 @@ function wrapResult(
                     ['Run `tw auth login` to re-authenticate with the required scopes'],
                 )
             }
+            if (isForbidden(error)) {
+                throw new CliError('FORBIDDEN', 'Twist refused this action: 403 Forbidden.', [
+                    'You may not have permission for this action — workspace admins can perform it',
+                    'Or re-authenticate with `tw auth login` if your scopes look wrong',
+                ])
+            }
             throw error
         })
 

src/lib/errors.test.ts

@@ -1,7 +1,7 @@
 import { TwistRequestError } from '@doist/twist-sdk'
 import { describe, expect, it } from 'vitest'
 
-import { isInsufficientScope } from './errors.js'
+import { isForbidden, isInsufficientScope } from './errors.js'
 
 describe('isInsufficientScope', () => {
     it('returns true for a 403 with "Insufficient scope" error_string', () => {
@@ -38,3 +38,48 @@ describe('isInsufficientScope', () => {
         expect(isInsufficientScope('string')).toBe(false)
     })
 })
+
+describe('isForbidden', () => {
+    it('returns true for any 403 — even without responseData', () => {
+        const error = new TwistRequestError('Request failed with status 403', 403, undefined)
+        expect(isForbidden(error)).toBe(true)
+    })
+
+    it('returns true for a 403 with an arbitrary error_string', () => {
+        const error = new TwistRequestError('Request failed with status 403', 403, {
+            error_code: 100,
+            error_string: 'Access denied',
+        })
+        expect(isForbidden(error)).toBe(true)
+    })
+
+    it('returns false for non-403 status codes', () => {
+        expect(isForbidden(new TwistRequestError('Request failed with status 401', 401, {}))).toBe(
+            false,
+        )
+        expect(isForbidden(new TwistRequestError('Request failed with status 404', 404, {}))).toBe(
+            false,
+        )
+        expect(isForbidden(new TwistRequestError('Request failed with status 500', 500, {}))).toBe(
+            false,
+        )
+    })
+
+    it('returns false for plain errors and non-object values', () => {
+        expect(isForbidden(new Error('something'))).toBe(false)
+        expect(isForbidden(null)).toBe(false)
+        expect(isForbidden(undefined)).toBe(false)
+        expect(isForbidden('string')).toBe(false)
+    })
+
+    // Precedence: both predicates fire on a scope 403, so callers must test
+    // isInsufficientScope first (the central handler in api.ts does).
+    it('fires alongside isInsufficientScope for an "Insufficient scope" 403', () => {
+        const error = new TwistRequestError('Request failed with status 403', 403, {
+            error_code: 109,
+            error_string: 'Insufficient scope provided: user:write',
+        })
+        expect(isInsufficientScope(error)).toBe(true)
+        expect(isForbidden(error)).toBe(true)
+    })
+})

src/lib/errors.ts

@@ -12,6 +12,7 @@ export type ErrorCode =
     // Auth & permissions
     | 'AUTH_FAILED'
     | 'AUTH_MIGRATION_PENDING'
+    | 'FORBIDDEN'
     | 'INSUFFICIENT_SCOPE'
     | 'INVALID_TOKEN'
     | 'NO_TOKEN'
@@ -86,6 +87,21 @@ export function isInsufficientScope(error: unknown): boolean {
     )
 }
 
+/**
+ * Check whether an error is any Twist API 403 response.
+ *
+ * Precedence: callers must test `isInsufficientScope` first so OAuth-scope
+ * 403s keep their dedicated `INSUFFICIENT_SCOPE` code and hints; `isForbidden`
+ * is the catch-all fallback for plain workspace-permission 403s.
+ */
+export function isForbidden(error: unknown): boolean {
+    if (typeof error !== 'object' || error === null || !('httpStatusCode' in error)) {
+        return false
+    }
+    const { httpStatusCode } = error as { httpStatusCode: number }
+    return httpStatusCode === 403
+}
+
 /**
  * Twist-flavoured CliError that preserves the historical positional
  * `(code, message, hints?, type?)` signature used across hundreds of call