Skip to content

fix: generate canonical Comms search URLs#26

Merged
gnapse merged 1 commit into
mainfrom
ernesto/fix-comms-search-urls
Jun 15, 2026
Merged

fix: generate canonical Comms search URLs#26
gnapse merged 1 commit into
mainfrom
ernesto/fix-comms-search-urls

Conversation

@gnapse

@gnapse gnapse commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Am I correct in assuming it is wrong? My tdc CLI was giving me links with the /a section in front of the workspace ID, and the links did not work.

So this PR changes the CLI so that…

  • It generates canonical Comms search fallback URLs without the legacy /a path segment.
  • It links conversation search hits to conversations and message search hits to specific messages.

@gnapse gnapse requested a review from amix June 15, 2026 19:48
@gnapse gnapse self-assigned this Jun 15, 2026
@gnapse gnapse added the 🙋 Ask PR PR must be reviewed before merging label Jun 15, 2026
@gnapse gnapse marked this pull request as ready for review June 15, 2026 19:49
@doistbot doistbot requested a review from Bloomca June 15, 2026 19:50

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR updates fix: generate canonical Comms search URLs. No inline issues were flagged.

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

Optional follow-up note (1)
  • [P3] src/lib/search-helpers.ts:214: buildSearchResultUrl is only called with SearchResult items (response.items.map(...) below), but this adds another field to a local shadow shape instead of reusing that SDK type. Keeping type as plain string and manually mirroring fields here makes this helper easy to drift out of sync as the search result contract evolves. Prefer SearchResult itself, or at least Pick<SearchResult, 'id' | 'type' | 'threadId' | 'channelId' | 'conversationId' | 'commentId'>.

Share FeedbackReview Logs

@amix amix left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👌

@gnapse gnapse merged commit b4bcc77 into main Jun 15, 2026
8 checks passed
@gnapse gnapse deleted the ernesto/fix-comms-search-urls branch June 15, 2026 21:27
doist-release-bot Bot added a commit that referenced this pull request Jun 15, 2026
## [1.6.3](v1.6.2...v1.6.3) (2026-06-15)

### Bug Fixes

* generate canonical Comms search URLs ([#26](#26)) ([b4bcc77](b4bcc77))
@doist-release-bot

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.6.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🙋 Ask PR PR must be reviewed before merging released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants