refactor(errors): centralize 403 FORBIDDEN translation in wrapResult

[scottlovegrove]

Summary

Ports Doist/twist-cli#247 to comms-cli.

• Lift the local isForbidden check from src/commands/channel/delete.ts into the central wrapResult handler in src/lib/api.ts. Every SDK call now gets uniform 403 → CliError('FORBIDDEN') translation, not just channel deletion. Future commands (kick user, group delete, etc.) inherit the friendly message for free.
• New isForbidden(error) predicate in src/lib/errors.ts sits next to isInsufficientScope. Both delegate to a shared hasCommsStatusCode(error, status) type predicate so the shape narrowing lives in one place, with no as casts. JSDoc documents the precedence rule: callers must test isInsufficientScope first so OAuth-scope 403s keep their dedicated INSUFFICIENT_SCOPE code and hints; isForbidden is the catch-all.
• FORBIDDEN end-to-end translation tests in src/lib/api.test.ts exercise the real proxy + wrapResult flow by mocking @doist/comms-sdk. Covers FORBIDDEN translation, INSUFFICIENT_SCOPE precedence, and non-403 pass-through (asserted by object identity, not just message).

Hint copy is call-agnostic. The central message reads Comms refused this action: 403 Forbidden. with hints You may not have permission for this action and Contact your workspace admin, or re-authenticate with `tdc auth login` if your token looks wrong. The previous local message included the channel name; uniform coverage is the deliberate tradeoff. If per-command flavour matters later, we can plumb the spinner config's command name into the central message.

Follow-ups (not this PR): the same central-translation pattern should eventually cover 404 (NOT_FOUND-style with resource hint), 429 (rate-limit with retry-after), and 5xx (transient retry with backoff). Each warrants its own PR with its own behaviour decision.

Test plan

• `npm run type-check`
• `npm run lint`
• `npm test` (701 passed)

🤖 Generated with Claude Code

[doistbot]

Thanks scottlovegrove for your contribution 🤗. The refactoring cleanly centralizes 403 FORBIDDEN handling and safely removes manual type casts by extracting the hasCommsStatusCode helper.

Few things worth tightening:

• Top-level callable properties like client.batch(...) are not currently wrapped by the root proxy, meaning batch calls will still bypass the translation layer and leak raw CommsRequestErrors.
• The new tests mock isMutatingMethod to false, which bypasses the permission-checked branch for deleteChannel; consider adjusting the mock for at least one case or keeping a command-level regression test to ensure the actual mutating path remains covered.

I also included a few optional follow-up notes in the details below.

Optional follow-up notes (2)

• [P3] src/lib/errors.ts:99: isForbidden() overlaps with isInsufficientScope(), so callers have to remember the ordering rule from the docstring to avoid downgrading scope errors to FORBIDDEN. That’s a brittle shared API: the next call site that checks only isForbidden() will get the wrong CLI error. Consider making isForbidden() exclusive (403 && !isInsufficientScope(error)) or replacing the two booleans with a single classifier.
• [P3] src/lib/api.test.ts:98: vi.resetModules() and the repeated dynamic await import('./api.js') inside each test are unnecessary. createWrappedCommsClient is a pure factory that returns a fresh proxy instance every time it is called. You can safely add createWrappedCommsClient to the existing top-level dynamic import (line 48) and simplify the tests by calling it directly.

_{Share Feedback • Review Logs}

[rmartins90]

✅

[doist-release-bot]

🎉 This PR is included in version 1.4.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀