Skip to content

fix(artifactory): preserve base URL path component when building request URLs - #94

Merged
kolatts merged 1 commit into
mainfrom
claude/issue-90-20260418-2207
Apr 18, 2026
Merged

fix(artifactory): preserve base URL path component when building request URLs#94
kolatts merged 1 commit into
mainfrom
claude/issue-90-20260418-2207

Conversation

@claude

@claude claude Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Changes const API = '/api' to const API = 'api' in src/services/artifactory/client.ts
  • Per the URL spec, new URL('/api/...', base) resolves relative to the origin, not the base path — so a configured baseUrl of https://host/artifactory had its /artifactory segment silently dropped on every request
  • With a relative path ('api/...'), new URL resolves against the full base URL path, producing the correct https://host/artifactory/api/...
  • Adds a regression test in src/lib/http.test.ts that stubs fetch to verify the URL includes the base path component

Closes #90

Generated with Claude Code

… URL path

`new URL('/api/...', 'https://host/artifactory/')` resolves relative to the
origin, silently stripping the `/artifactory` segment. Changing `const API`
from `'/api'` to `'api'` makes the path relative to the base URL so the full
configured path is preserved.

Closes #90

Co-authored-by: Sunny Kolattukudy <kolatts@users.noreply.github.com>
@claude

claude Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor Author

Claude finished @claude[bot]'s task in 1m 14s —— View job


PR Review Complete

  • Clean up stale Claude feedback
  • Review PR changes
  • Submit formal review (blocked — cannot approve own PR; see review summary below)

Verdict: Approve — no substantive issues found. GitHub prevents the bot from approving its own PR, so posting the review here instead.

What the fix does

buildUrl in src/lib/http.ts:26 constructs URLs via new URL(path, base). Per the URL spec, a path starting with / resolves relative to the origin of the base URL — silently dropping any path segment like /artifactory. Changing const API = '/api'const API = 'api' makes the path relative to the full base URL, so https://host/artifactory/ + api/system/ping correctly produces https://host/artifactory/api/system/ping.

Regression test (src/lib/http.test.ts:210–228)

Clean. vi.stubGlobal('fetch', ...) is restored in a finally block, the expected URL is an exact string assertion, and the assert sits outside the try so a premature throw would surface as a test failure rather than silently pass. fetchWithTimeout captures its fetcher default at call time (= fetch), so the global stub is picked up correctly.

No concerns

The change is minimal and correct. All other callers in client.ts pass paths through the API constant, so they all benefit from the fix. No other services use path-component base URLs in practice, and buildUrl would handle them correctly anyway as long as callers pass relative paths.

@kolatts
kolatts merged commit 26ab1ef into main Apr 18, 2026
3 checks passed
@kolatts
kolatts deleted the claude/issue-90-20260418-2207 branch April 18, 2026 22:15
kolatts added a commit that referenced this pull request Jul 29, 2026
…ace and similar hosts (#284)

Strip the leading '/' from path before new URL(path, base) so that an absolute-path
reference does not resolve against the origin and silently discard path components
already present in base (e.g. /e/<environment-id> for Dynatrace Managed clusters).

Fixes #283 — recurrence of BUG-4 root cause (same WHATWG URL spec behavior that
dropped /artifactory from Artifactory base URLs in PR #94).

Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Sunny Kolattukudy <kolatts@users.noreply.github.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.

artifactory commands fail when baseUrl includes a path component

1 participant