Skip to content

fix(tools): handle trailing slashes and whitespace in normalizeBaseUrl and addConversation - #1678

Closed
Adityakk9031 wants to merge 1 commit into
supermemoryai:mainfrom
Adityakk9031:fix/normalize-base-url-multiple-slashes
Closed

Adityakk9031 wants to merge 1 commit into
supermemoryai:mainfrom
Adityakk9031:fix/normalize-base-url-multiple-slashes

Conversation

@Adityakk9031

Copy link
Copy Markdown
Contributor

Summary

Fixes base URL normalization and image URL handling in @supermemory/tools:
1.
ormalizeBaseUrl in packages/tools/src/shared/context.ts used url.endsWith('/') ? url.slice(0, -1) : url. If a configured custom base URL had multiple trailing slashes (e.g. http://localhost:6768//) or surrounding whitespace (common in .env configurations), it left trailing slashes intact or failed to trim whitespace, causing duplicate path slashes like http://localhost:6768//v4/....
2. �ddConversation in packages/tools/src/conversations-client.ts bypassed
ormalizeBaseUrl and directly concatenated params.baseUrl || "https://api.supermemory.ai" with /v4/conversations.
3. oConversationImageUrl in packages/tools/src/conversations-client.ts did not support image objects containing a url property ({ url: string }), returning
ull for structured image objects.

Changes

  • Updated
    ormalizeBaseUrl in packages/tools/src/shared/context.ts to trim whitespace and strip all trailing slashes: url?.trim().replace(//+$/, "") || defaultUrl.
  • Applied
    ormalizeBaseUrl in �ddConversation.
  • Added support for { url: string } objects in oConversationImageUrl.
  • Added unit tests in packages/tools/src/tools-shared.test.ts covering multiple trailing slashes, whitespace trimming, fallback default URL, and image URL resolution.

@MaheshtheDev

Copy link
Copy Markdown
Member

🤖 AI-assisted triage, reviewed by @MaheshtheDev.

Thank you for this fix! It's now included in #1798, which bundles the community tools fixes for @supermemory/tools 3.1.2. Your commit is kept with you as the author, and you're credited as a co-author on the merge. We adapted it to the v5 SDK on top. Closing in favor of #1798.

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.

2 participants