fix: move proxy context to URL path#5
Conversation
Align proxy requests with backend routing by putting workspace/project/environment in the proxy URL path. Remove config and is-personal from the wire payload to match the proxy request schema. Made-with: Cursor
|
Caution Review failedPull request was closed or merged during review WalkthroughThis PR updates the proxy and interceptor layers: the default proxy endpoint now includes the /v1/proxy path; proxy request context (workspace, project, environment-id) is moved from a nested Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 3/5 reviews remaining, refill in 16 minutes and 56 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/proxy.test.ts (1)
446-453:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the default fallback expectation.
When
proxyis omitted, the client now defaults tohttps://proxy.enkryptify.com/v1/proxy, so this assertion should include/v1/proxyor it will fail againstsrc/enkryptify.tsLine 21.Suggested fix
- expect(fetchMock.mock.calls[0]?.[0]).toBe("https://proxy.enkryptify.com/ws-1/prj-1/env-1"); + expect(fetchMock.mock.calls[0]?.[0]).toBe("https://proxy.enkryptify.com/v1/proxy/ws-1/prj-1/env-1");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/proxy.test.ts` around lines 446 - 453, Update the test expectation to match the new default proxy path: in the "falls back to default POC URL when nothing else is set" test, change the asserted URL used by Enkryptify when proxy is omitted (created via makeConfig and exercised by client.proxy.fetch) to include the new default suffix "/v1/proxy" (i.e. expect "https://proxy.enkryptify.com/v1/proxy" instead of the old "/ws-1/prj-1/env-1").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/proxy.ts`:
- Around line 304-306: The helper buildProxyRequestUrl currently appends
workspace/project/environment directly to the normalized base and can drop the
required /v1/proxy segment; update buildProxyRequestUrl to detect if
normalizedBaseUrl already contains the "/v1/proxy" path (allow optional trailing
slash) and, if it does not, insert "/v1/proxy" before appending the encoded
workspace/project/environment; keep using encodeURIComponent for
workspace/project/environment and ensure you trim duplicate slashes when
concatenating.
---
Outside diff comments:
In `@tests/proxy.test.ts`:
- Around line 446-453: Update the test expectation to match the new default
proxy path: in the "falls back to default POC URL when nothing else is set"
test, change the asserted URL used by Enkryptify when proxy is omitted (created
via makeConfig and exercised by client.proxy.fetch) to include the new default
suffix "/v1/proxy" (i.e. expect "https://proxy.enkryptify.com/v1/proxy" instead
of the old "/ws-1/prj-1/env-1").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6cce579-fb39-4780-9bb8-4b9c0b1214b0
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
README.mdsrc/enkryptify.tssrc/interceptor.tssrc/proxy.tssrc/types.tstests/interceptor.test.tstests/proxy.test.ts
💤 Files with no reviewable changes (2)
- README.md
- src/types.ts
| function buildProxyRequestUrl(baseUrl: string, workspace: string, project: string, environmentId: string): string { | ||
| const normalizedBaseUrl = baseUrl.replace(/\/+$/, ""); | ||
| return `${normalizedBaseUrl}/${encodeURIComponent(workspace)}/${encodeURIComponent(project)}/${encodeURIComponent(environmentId)}`; |
There was a problem hiding this comment.
Preserve /v1/proxy for user-supplied proxy bases.
DEFAULT_PROXY_URL was updated to include /v1/proxy, but this helper still appends scope directly onto whatever came from config.proxy.url / ENKRYPTIFY_PROXY_URL. A host-only override like "https://proxy.example.com" now becomes https://proxy.example.com/ws/prj/env, which misses the new backend route and breaks existing custom proxy configs.
Possible fix
function buildProxyRequestUrl(baseUrl: string, workspace: string, project: string, environmentId: string): string {
const normalizedBaseUrl = baseUrl.replace(/\/+$/, "");
- return `${normalizedBaseUrl}/${encodeURIComponent(workspace)}/${encodeURIComponent(project)}/${encodeURIComponent(environmentId)}`;
+ const routedBaseUrl = normalizedBaseUrl.endsWith("/v1/proxy")
+ ? normalizedBaseUrl
+ : `${normalizedBaseUrl}/v1/proxy`;
+ return `${routedBaseUrl}/${encodeURIComponent(workspace)}/${encodeURIComponent(project)}/${encodeURIComponent(environmentId)}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function buildProxyRequestUrl(baseUrl: string, workspace: string, project: string, environmentId: string): string { | |
| const normalizedBaseUrl = baseUrl.replace(/\/+$/, ""); | |
| return `${normalizedBaseUrl}/${encodeURIComponent(workspace)}/${encodeURIComponent(project)}/${encodeURIComponent(environmentId)}`; | |
| function buildProxyRequestUrl(baseUrl: string, workspace: string, project: string, environmentId: string): string { | |
| const normalizedBaseUrl = baseUrl.replace(/\/+$/, ""); | |
| const routedBaseUrl = normalizedBaseUrl.endsWith("/v1/proxy") | |
| ? normalizedBaseUrl | |
| : `${normalizedBaseUrl}/v1/proxy`; | |
| return `${routedBaseUrl}/${encodeURIComponent(workspace)}/${encodeURIComponent(project)}/${encodeURIComponent(environmentId)}`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/proxy.ts` around lines 304 - 306, The helper buildProxyRequestUrl
currently appends workspace/project/environment directly to the normalized base
and can drop the required /v1/proxy segment; update buildProxyRequestUrl to
detect if normalizedBaseUrl already contains the "/v1/proxy" path (allow
optional trailing slash) and, if it does not, insert "/v1/proxy" before
appending the encoded workspace/project/environment; keep using
encodeURIComponent for workspace/project/environment and ensure you trim
duplicate slashes when concatenating.
1 participant
Align proxy requests with backend routing by putting
workspace/project/environment in the proxy URL path.
Remove config and is-personal from the wire payload
to match the proxy request schema.
Made-with: Cursor
Summary by CodeRabbit
Breaking Changes
usePersonaloption for proxy requests.Updates
Documentation