Skip to content

refactor: use the SDK's id validator instead of a local base58 check - #67

Draft
lmjabreu wants to merge 1 commit into
mainfrom
lmjabreu/use-sdk-id-validator
Draft

lmjabreu wants to merge 1 commit into
mainfrom
lmjabreu/use-sdk-id-validator

Conversation

@lmjabreu

@lmjabreu lmjabreu commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Overview

The id check added in #66 was a duplicate of the SDK's isValidUuidV7Base58, which I only spotted after it merged. This removes ours in favour of the SDK one.

Reference

Follows @scottlovegrove's review on #66. The error helpers from that PR (isNotFound, isConflict, isMalformedId) have no SDK equivalent yet and are untouched here; they need their own SDK PR.

Changelog

A bare channel id resolves without needing a workspace again, and ids are recognised by the SDK's rule rather than a local copy of it.

Test plan

  1. tdc channel threads EngineeringDiscussion --limit 1
    • Error: CHANNEL_NOT_FOUND, so a name that looks like an id is still treated as a name
  2. tdc search x --channel CbjxNkWHJBwcaVkoTCRgM --limit 1 (any bare channel id)
    • Reaches the API, no INVALID_REF
  3. tdc conversation done <a conversation id with no digit in it> --dry-run
  4. tdc thread view id:nope
    • Error: INVALID_REF with the server's message, no stack trace
  5. tdc groups view id:<a well-formed id that does not exist>
    • Error: GROUP_NOT_FOUND with the tdc groups hint

🤖 Generated with Claude Code

`@doist/comms-sdk` has exported `isValidUuidV7Base58` since 0.11.1 (July),
and it checks the v7 version nibble and variant bits on top of the base58
decode. The local `looksLikeOpaqueCommsId` checked neither, so a 21-char
name like `EngineeringDiscussion` decoded to 16 bytes and read as an id.

That collision is the only reason `resolveChannelRef` grew a
name-first-then-getChannel fallback in #66, so both go: `getDirectChannelId`
recognises an opaque id again, which also restores the workspace-agnostic
behaviour a bare digit-free channel id had before #66.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant