Adaption to the new proxy format#6
Open
axlistas wants to merge 6 commits into
Open
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
Proxy integration
WalkthroughThis PR restructures the proxy wire payload shape from a nested Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 4/5 reviews remaining, refill in 12 minutes. 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/proxy.test.ts`:
- Line 50: The tests have inconsistent expectations for the proxy base path:
update the assertions that verify the generated proxy URL (the
expect(url).toBe(...) checks in tests that exercise the proxy URL generation) so
both cases match the configured base which includes the "/v1/proxy" prefix;
adjust the two failing expectations (the one currently expecting
"https://proxy.test.com/v1/proxy/ws-1/prj-1/env-1" and the other default
expectation that omits "/v1/proxy") so they consistently assert the same full
base (include "/v1/proxy" if the configured base includes it).
🪄 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: 4dcb1e64-75e0-43cb-8d28-b051cce87780
⛔ 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
SiebeBaree
requested changes
Apr 30, 2026
| /** Override the client's environment for this request. */ | ||
| environment?: string; | ||
| /** Override the client's `usePersonalValues` setting for this request. */ | ||
| usePersonal?: boolean; |
Member
There was a problem hiding this comment.
Why remove personal values? What will your proxy take? I think this option is needed
Author
There was a problem hiding this comment.
I'll add support for this to the proxy.
| import { HttpInterceptor } from "@/interceptor"; | ||
|
|
||
| const DEFAULT_PROXY_URL = "https://proxy.enkryptify.com"; | ||
| const DEFAULT_PROXY_URL = "https://proxy.enkryptify.com/v1/proxy"; |
Author
There was a problem hiding this comment.
Currently the proxy runs on /v1/proxy but I can change that real quick.
Bring back usePersonal support for proxy requests and interceptor rules by restoring is-personal on the proxy wire payload. Set the default proxy base URL back to the root host path instead of /v1/proxy and align tests accordingly. Made-with: Cursor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/proxy.test.ts (1)
229-257: ⚡ Quick winAdd one URL-encoding regression test for scoped path segments.
Current URL assertions only use safe slugs. Add a case with spaces/slashes to lock in
encodeURIComponentbehavior fromsrc/proxy.ts(Line 312-315).Suggested test addition
describe("client.proxy.request — low-level API", () => { + it("encodes workspace/project/environment when building proxy URL path", async () => { + fetchMock.mockResolvedValue(new Response("{}", { status: 200 })); + const client = new Enkryptify(makeConfig()); + + await client.proxy.request({ + url: "https://upstream/x", + method: "GET", + workspace: "team a", + project: "proj/blue", + environment: "env#1", + }); + + expect(fetchMock.mock.calls[0]?.[0]).toBe( + "https://proxy.test.com/team%20a/proj%2Fblue/env%231", + ); + }); + it("applies per-call environment override", async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/proxy.test.ts` around lines 229 - 257, Add a regression test that verifies URL-encoding of scoped path segments by calling client.proxy.request with workspace, project, and environment values containing spaces and slashes (e.g., "my ws", "prj/with/slash", "env with/slash") and assert the proxied URL used in fetchMock is the encoded form produced by encodeURIComponent for each segment; reuse the existing test pattern (fetchMock.mockResolvedValue, new Enkryptify(makeConfig()), await client.proxy.request(...), getCallBody(fetchMock.mock.calls[0]) and expect(fetchMock.mock.calls[0]?.[0]) to equal the proxy base plus the three segments encoded) to lock in the encodeURIComponent behavior referenced in src/proxy.ts (client.proxy.request).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/proxy.test.ts`:
- Around line 229-257: Add a regression test that verifies URL-encoding of
scoped path segments by calling client.proxy.request with workspace, project,
and environment values containing spaces and slashes (e.g., "my ws",
"prj/with/slash", "env with/slash") and assert the proxied URL used in fetchMock
is the encoded form produced by encodeURIComponent for each segment; reuse the
existing test pattern (fetchMock.mockResolvedValue, new
Enkryptify(makeConfig()), await client.proxy.request(...),
getCallBody(fetchMock.mock.calls[0]) and expect(fetchMock.mock.calls[0]?.[0]) to
equal the proxy base plus the three segments encoded) to lock in the
encodeURIComponent behavior referenced in src/proxy.ts (client.proxy.request).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b23df8c5-b400-479f-9922-026eaf9c39cb
📒 Files selected for processing (4)
src/interceptor.tssrc/proxy.tstests/interceptor.test.tstests/proxy.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change from Config{} to dynamic url support.
Summary by CodeRabbit
Documentation
Refactor