fix: drop leading /api/ from CheckmarxClient paths (fixes doubled /api/api/ after buildUrl fix)

[kolatts]

Fixes the /api/api/ path doubling regression introduced by PR #284.

Closes #285

Generated with Claude Code

[claude]

Claude finished @kolatts's task in 4m 5s —— View job

---

PR Review Complete

• Clean up stale Claude comments
• Read changed files and diff
• Review changes for correctness
• Submit formal review — BLOCKING: requested changes

---

Summary

The path changes in CheckmarxClient are logically correct — dropping the leading /api/ removes the doubling that PR #284's buildUrl() fix introduced for users whose base URL already contained /api. The test updates match the new paths.

However, skills/pncli/checkmarx.md was not updated, and CLAUDE.md is explicit that this skills file is the onboarding contract and must be kept in sync on every credential or URL change.

Blocking

buildUrl('https://ast.checkmarx.net', 'projects') resolves to https://ast.checkmarx.net/projects — the /api/ segment is missing. Any user who followed the documented example before this PR (baseUrl = https://ast.checkmarx.net) will silently get 404s after upgrading.

skills/pncli/checkmarx.md needs to update its example base URL from https://ast.checkmarx.net → https://ast.checkmarx.net/api. See the inline comment on client.ts:14 for full detail. The buildCheckmarxFetcher IAM host derivation is unaffected (it uses new URL(baseUrl).hostname only).

What looks good

• All 5 path edits in client.ts are consistent with each other
• Both test assertions in client.test.ts were updated to match
• buildCheckmarxFetcher auth exchange is not affected by the base URL path change

[claude]

These paths now implicitly require that checkmarx.baseUrl ends with /api (e.g. https://ast.checkmarx.net/api), but skills/pncli/checkmarx.md still documents the example as https://ast.checkmarx.net (no /api).

With buildUrl('https://ast.checkmarx.net', 'projects') the result is https://ast.checkmarx.net/projects — the /api/ segment is missing, so any user who followed the documented example before this PR will silently get 404s.

CLAUDE.md calls the skills file the "onboarding contract" and says it must be updated on every credential/URL change. The fix needs to either:

a) Update the docs — change the example in skills/pncli/checkmarx.md from https://ast.checkmarx.net → https://ast.checkmarx.net/api

Suggested change

	const res = await this.http.checkmarx<CxOneProjectsResponse>('projects', { params: { limit: 100 } });
	const res = await this.http.checkmarx<CxOneProjectsResponse>('projects', { params: { limit: 100 } });

(No code change needed here — the paths are correct given option a. Just noting the inline location of the dependency.)

b) Keep /api/ in the paths as a relative prefix (e.g. api/projects) and keep the documented base URL as https://ast.checkmarx.net. But this still breaks users who already have /api in their saved config, so option (a) is cleaner.

Either way, skills/pncli/checkmarx.md must be updated before merging.

[claude]

BLOCKING — documentation not updated to match the new base URL contract.

Summary

The path changes in CheckmarxClient are logically correct: dropping the leading /api/ removes the doubling that PR #284's buildUrl() fix introduced for users whose base URL already contained /api. The test updates match the new paths.

However, skills/pncli/checkmarx.md was not updated, and CLAUDE.md is explicit that this file is the onboarding contract and must be kept in sync on every credential or URL change.

Blocking issue

buildUrl('https://ast.checkmarx.net', 'projects') resolves to https://ast.checkmarx.net/projects — the /api/ segment is gone. Any user who followed the documented example (baseUrl = https://ast.checkmarx.net) before this PR will silently get 404s after upgrading.

skills/pncli/checkmarx.md needs to update its example from https://ast.checkmarx.net to https://ast.checkmarx.net/api so the documented base URL matches what buildUrl() now expects. See inline comment on client.ts:14.

What looks good

• The 5-path edit in client.ts is consistent and the logic is correct given the contract change.
• Both unit-test assertions in client.test.ts were updated to match the new paths.
• buildCheckmarxFetcher derives the IAM host from new URL(baseUrl).hostname only, so adding /api to the base URL path does not affect token exchange.

[kolatts]

@claude[agent] Update accordingly based on PR feedback, note that there are no users that were using checkmarx before so breaking changes are ok.

[claude]

Claude finished @kolatts's task in 1m 47s —— View job

---

• Clean up stale Claude comments
• Read changed files and diff
• Review changes for correctness
• Submit formal review — APPROVED

---

Review Complete

The blocking issue from the previous review has been addressed. Both commits in this PR are correct.

docs: update Checkmarx API base URL (a96e5a1)

skills/pncli/checkmarx.md now documents https://ast.checkmarx.net/api throughout — table, config file example, env var example, and the regional deployment note. The onboarding contract is in sync with the code.

fix: drop leading /api/ from CheckmarxClient paths (91ce3eb)

All 5 paths in client.ts are bare relative paths (projects, projects/${id}, scans, scans/${id}, results/summary). With base URL https://ast.checkmarx.net/api, buildUrl() correctly resolves these to https://ast.checkmarx.net/api/projects, etc. buildCheckmarxFetcher derives the IAM host via new URL(baseUrl).hostname, which returns ast.checkmarx.net regardless of the path segment — so adding /api to the base URL has no effect on token exchange. Both unit test assertions updated to match the new paths.

No issues found.

[claude]

Approved

The blocking issue from the previous review has been addressed. Both commits in this PR are correct.

docs: update Checkmarx API base URL — skills/pncli/checkmarx.md now documents the base URL as https://ast.checkmarx.net/api throughout (table, config file, env var, and regional deployment note). Onboarding contract is in sync.

fix: drop leading /api/ from CheckmarxClient paths — All 5 paths in client.ts are bare relative paths (projects, projects/${id}, scans, scans/${id}, results/summary). buildUrl('https://ast.checkmarx.net/api', 'projects') correctly resolves to https://ast.checkmarx.net/api/projects. buildCheckmarxFetcher uses new URL(baseUrl).hostname for IAM host derivation so the /api path segment doesn't affect token exchange. Both unit test assertions updated to match.

No issues found.