fix: accept digit-free opaque ids and map 404/409 to CliError

src/lib/api.test.ts

@@ -260,7 +260,66 @@ describe('wrapResult — central 403 translation', () => {
         })
     })
 
-    it('passes non-403 errors through untranslated', async () => {
+    it('translates a 404 into NOT_FOUND', async () => {
+        sdkMocks.deleteChannel.mockRejectedValueOnce(
+            new CommsRequestError('Request failed with status 404', 404, {
+                error_string: 'Resource not found',
+                error_code: 110,
+            }),
+        )
+        const client = createWrappedCommsClient('test-token')
+
+        await expect(client.channels.deleteChannel('CH404')).rejects.toMatchObject({
+            code: 'NOT_FOUND',
+            message: 'Comms could not find that resource: 404.',
+            hints: ['Check the id, or pass the Comms URL instead'],
+        })
+    })
+
+    it('translates a malformed-id 409 into INVALID_REF', async () => {
+        sdkMocks.deleteChannel.mockRejectedValueOnce(
+            new CommsRequestError('Request failed with status 409', 409, {
+                error_string: 'id must decode to 16 bytes. Regenerate the ID and retry.',
+                error_code: 217,
+            }),
+        )
+        const client = createWrappedCommsClient('test-token')
+
+        await expect(client.channels.deleteChannel('nope')).rejects.toMatchObject({
+            code: 'INVALID_REF',
+            message:
+                'Comms rejected the id: id must decode to 16 bytes. Regenerate the ID and retry.',
+        })
+    })
+
+    it('gives a 217 without an error_string a readable message', async () => {
+        sdkMocks.deleteChannel.mockRejectedValueOnce(
+            new CommsRequestError('Request failed with status 409', 409, { error_code: 217 }),
+        )
+        const client = createWrappedCommsClient('test-token')
+
+        await expect(client.channels.deleteChannel('nope')).rejects.toMatchObject({
+            code: 'INVALID_REF',
+            message: 'Comms rejected the id: it does not decode to a Comms id (409)',
+        })
+    })
+
+    it('translates any other 409 into CONFLICT, keeping the server message', async () => {
+        sdkMocks.deleteChannel.mockRejectedValueOnce(
+            new CommsRequestError('Request failed with status 409', 409, {
+                error_string: 'Channel name already taken',
+                error_code: 300,
+            }),
+        )
+        const client = createWrappedCommsClient('test-token')
+
+        await expect(client.channels.deleteChannel('CH409')).rejects.toMatchObject({
+            code: 'CONFLICT',
+            message: 'Comms refused this request: Channel name already taken',
+        })
+    })
+
+    it('passes unmapped errors through untranslated', async () => {
         const originalError = new CommsRequestError('Request failed with status 500', 500, {})
         sdkMocks.deleteChannel.mockRejectedValueOnce(originalError)
         const client = createWrappedCommsClient('test-token')

src/lib/api.ts

@@ -7,7 +7,16 @@ import {
 } from '@doist/comms-sdk'
 import { getApiTokenSnapshot } from './auth.js'
 import { getConfig, updateConfig } from './config.js'
-import { CliError, isForbidden, isInsufficientScope, isInvalidToken } from './errors.js'
+import {
+    CliError,
+    getCommsErrorString,
+    isConflict,
+    isForbidden,
+    isInsufficientScope,
+    isInvalidToken,
+    isMalformedId,
+    isNotFound,
+} from './errors.js'
 import { ensureMutationAllowed, isMutatingMethod } from './permissions.js'
 import { getProgressTracker } from './progress.js'
 import { withSpinner } from './spinner.js'
@@ -217,6 +226,23 @@ function wrapResult(
                     'Re-authenticate with `tdc auth login`, then check `tdc auth status`',
                 ])
             }
+            if (isNotFound(error)) {
+                throw new CliError('NOT_FOUND', 'Comms could not find that resource: 404.', [
+                    'Check the id, or pass the Comms URL instead',
+                ])
+            }
+            if (isMalformedId(error)) {
+                throw new CliError(
+                    'INVALID_REF',
+                    `Comms rejected the id: ${getCommsErrorString(error) ?? 'it does not decode to a Comms id (409)'}`,
+                )
+            }
+            if (isConflict(error)) {
+                throw new CliError(
+                    'CONFLICT',
+                    `Comms refused this request: ${getCommsErrorString(error) ?? '409 Conflict'}`,
+                )
+            }
             throw error
         })
 

src/lib/errors.test.ts

@@ -1,7 +1,17 @@
 import { CommsRequestError } from '@doist/comms-sdk'
 import { describe, expect, it } from 'vitest'
 
-import { isForbidden, isInsufficientScope, isInvalidToken } from './errors.js'
+import {
+    CliError,
+    getCommsErrorString,
+    isCliErrorCode,
+    isConflict,
+    isForbidden,
+    isInsufficientScope,
+    isInvalidToken,
+    isMalformedId,
+    isNotFound,
+} from './errors.js'
 
 describe('isInsufficientScope', () => {
     it('returns true for a 403 with "Insufficient scope" error_string', () => {
@@ -118,3 +128,54 @@ describe('isInvalidToken', () => {
         expect(isInvalidToken('string')).toBe(false)
     })
 })
+
+describe('isNotFound / isConflict', () => {
+    it('match on status alone', () => {
+        expect(isNotFound(new CommsRequestError('Request failed with status 404', 404, {}))).toBe(
+            true,
+        )
+        expect(isConflict(new CommsRequestError('Request failed with status 409', 409, {}))).toBe(
+            true,
+        )
+        expect(isNotFound(new CommsRequestError('Request failed with status 409', 409, {}))).toBe(
+            false,
+        )
+        expect(isConflict(new CommsRequestError('Request failed with status 404', 404, {}))).toBe(
+            false,
+        )
+        expect(isNotFound(new Error('something'))).toBe(false)
+    })
+})
+
+describe('isMalformedId', () => {
+    it('is true only for the 409 the API sends for an id that does not decode', () => {
+        const malformed = new CommsRequestError('Request failed with status 409', 409, {
+            error_string: 'id must decode to 16 bytes. Regenerate the ID and retry.',
+            error_code: 217,
+        })
+        expect(isMalformedId(malformed)).toBe(true)
+        expect(getCommsErrorString(malformed)).toBe(
+            'id must decode to 16 bytes. Regenerate the ID and retry.',
+        )
+
+        const otherConflict = new CommsRequestError('Request failed with status 409', 409, {
+            error_string: 'Channel name already taken',
+        })
+        expect(isMalformedId(otherConflict)).toBe(false)
+        expect(
+            isMalformedId(new CommsRequestError('Request failed with status 409', 409, {})),
+        ).toBe(false)
+        expect(getCommsErrorString(new CommsRequestError('x', 409, undefined))).toBeNull()
+    })
+})
+
+describe('isCliErrorCode', () => {
+    it('matches a CliError by any of the given codes and nothing else', () => {
+        const notFound = new CliError('NOT_FOUND', 'x')
+        expect(isCliErrorCode(notFound, 'NOT_FOUND')).toBe(true)
+        expect(isCliErrorCode(notFound, 'INVALID_REF', 'NOT_FOUND')).toBe(true)
+        expect(isCliErrorCode(notFound, 'INVALID_REF')).toBe(false)
+        expect(isCliErrorCode(new Error('x'), 'NOT_FOUND')).toBe(false)
+        expect(isCliErrorCode(new CommsRequestError('x', 404, {}), 'NOT_FOUND')).toBe(false)
+    })
+})

src/lib/errors.ts

@@ -62,6 +62,7 @@ export type ErrorCode =
     | 'UNKNOWN_AGENT'
     // API & internal
     | 'API_ERROR'
+    | 'CONFLICT'
     | 'INTERNAL_ERROR'
     // Config file inspection
     | 'CONFIG_READ_FAILED'
@@ -85,15 +86,9 @@ function hasCommsStatusCode(error: unknown, status: number): error is { httpStat
  * Works with any error shaped like CommsRequestError (httpStatusCode + responseData).
  */
 export function isInsufficientScope(error: unknown): boolean {
-    if (!hasCommsStatusCode(error, 403)) return false
-    if (!('responseData' in error)) return false
-    const data = error.responseData
     return (
-        typeof data === 'object' &&
-        data !== null &&
-        'error_string' in data &&
-        typeof data.error_string === 'string' &&
-        data.error_string.includes('Insufficient scope')
+        hasCommsStatusCode(error, 403) &&
+        (getCommsErrorString(error)?.includes('Insufficient scope') ?? false)
     )
 }
 
@@ -118,6 +113,46 @@ export function isInvalidToken(error: unknown): boolean {
     return hasCommsStatusCode(error, 401)
 }
 
+/** True when `error` is a CliError carrying one of the given codes. */
+export function isCliErrorCode(error: unknown, ...codes: ErrorCode[]): boolean {
+    return error instanceof CliError && codes.includes(error.code)
+}
+
+export function isNotFound(error: unknown): boolean {
+    return hasCommsStatusCode(error, 404)
+}
+
+export function isConflict(error: unknown): boolean {
+    return hasCommsStatusCode(error, 409)
+}
+
+function getCommsResponseField(error: unknown, field: string): unknown {
+    if (typeof error !== 'object' || error === null || !('responseData' in error)) return undefined
+    const data = error.responseData
+    if (typeof data !== 'object' || data === null || !(field in data)) return undefined
+    return (data as Record<string, unknown>)[field]
+}
+
+/** The server's `error_string`, when the response body carried one. */
+export function getCommsErrorString(error: unknown): string | null {
+    const value = getCommsResponseField(error, 'error_string')
+    return typeof value === 'string' ? value : null
+}
+
+/** The server's numeric `error_code`, when the response body carried one. */
+export function getCommsErrorCode(error: unknown): number | null {
+    const value = getCommsResponseField(error, 'error_code')
+    return typeof value === 'number' ? value : null
+}
+
+/**
+ * Comms answers 409 with error_code 217 when an id does not base58-decode to
+ * 16 bytes. That is a bad reference, not a conflict.
+ */
+export function isMalformedId(error: unknown): boolean {
+    return isConflict(error) && getCommsErrorCode(error) === 217
+}
+
 /**
  * Comms-flavoured CliError that preserves the historical positional
  * `(code, message, hints?, type?)` signature used across hundreds of call