fix: some provider proxy impl - #51
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Summary by CodeRabbit
WalkthroughThis PR adds provider-specific proxy executors for Builder.io, Google Docs, and Prerender, with path-based routing and request normalization. It extends Hugging Face to support both OAuth2 and API key authentication across provider definition, runtime context, executor wiring, and validation. It also updates the proxy registry for forem, semantic_scholar, huggingface, posthog, and tiktok_business, and adds proxy-loader tests for the new and updated routing and auth flows. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/providers/huggingface/runtime.shared.ts (1)
352-371: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract hardcoded profile URL into a named constant.
The rest of the file defines endpoint URLs as top-level constants (
huggingfaceUserinfoUrl,huggingfaceWhoamiUrl, etc.), but this function inlineshttps://huggingface.co/${preferredUsername}directly.♻️ Proposed refactor
+export const huggingfaceProfileBaseUrl = "https://huggingface.co"; + function normalizeTokenCurrentUserPayload(payload: Record<string, unknown>): HuggingfaceCurrentUser { const user: HuggingfaceCurrentUser = { id: optionalString(payload.id) ?? requireProviderString(payload.name, "huggingface user name"), }; const preferredUsername = optionalString(payload.name); if (preferredUsername) { user.preferredUsername = preferredUsername; - user.profileUrl = `https://huggingface.co/${preferredUsername}`; + user.profileUrl = `${huggingfaceProfileBaseUrl}/${preferredUsername}`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/huggingface/runtime.shared.ts` around lines 352 - 371, The profile URL in normalizeTokenCurrentUserPayload is hardcoded inline, unlike the other endpoint URLs in this module. Add a top-level named constant for the Hugging Face profile base URL and use it when setting user.profileUrl, keeping the URL construction in normalizeTokenCurrentUserPayload consistent and easier to maintain.src/providers/huggingface/executors.test.ts (1)
1-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd oauth2 coverage alongside the new api_key test.
This new file only tests the
api_keypath forhuggingface.get_current_user. Theoauth2branch ofcreateContext(and its distincthuggingfaceUserinfoUrlrequest/normalization path) has no coverage here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/huggingface/executors.test.ts` around lines 1 - 52, Add test coverage for the oauth2 branch in Hugging Face executors. Extend the `executors["huggingface.get_current_user"]` tests to cover `createContext` when `getCredential` returns an oauth2 credential, and assert it calls the `huggingfaceUserinfoUrl` path instead of `whoami-v2`. Verify the response normalization matches the oauth2 userinfo shape and that the resulting `output` fields are mapped correctly, alongside the existing `apiKeyCredential` test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/providers/prerender/executors.ts`:
- Around line 184-196: Normalize the method in buildPrerenderProxyRequest before
the GET/POST branching, because ProxyRequestInput.method is a plain string and
exact-case checks can miss lowercase or mixed-case values. Update the logic
around buildPrerenderProxyRequest to compare against a normalized method value
(for example, a consistent uppercase form) so the cache-clear-status token
rewrite and POST body rewrite still happen regardless of input casing.
---
Nitpick comments:
In `@src/providers/huggingface/executors.test.ts`:
- Around line 1-52: Add test coverage for the oauth2 branch in Hugging Face
executors. Extend the `executors["huggingface.get_current_user"]` tests to cover
`createContext` when `getCredential` returns an oauth2 credential, and assert it
calls the `huggingfaceUserinfoUrl` path instead of `whoami-v2`. Verify the
response normalization matches the oauth2 userinfo shape and that the resulting
`output` fields are mapped correctly, alongside the existing `apiKeyCredential`
test.
In `@src/providers/huggingface/runtime.shared.ts`:
- Around line 352-371: The profile URL in normalizeTokenCurrentUserPayload is
hardcoded inline, unlike the other endpoint URLs in this module. Add a top-level
named constant for the Hugging Face profile base URL and use it when setting
user.profileUrl, keeping the URL construction in
normalizeTokenCurrentUserPayload consistent and easier to maintain.
🪄 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: b4be59f9-c68a-46d3-84b8-eeb89bd16ce0
📒 Files selected for processing (13)
src/providers/builder_io/executors.tssrc/providers/googledocs/executors.tssrc/providers/huggingface/definition.tssrc/providers/huggingface/executors.test.tssrc/providers/huggingface/executors.tssrc/providers/huggingface/runtime.shared.tssrc/providers/prerender/executors.tssrc/providers/provider-proxy-loader-a-c.test.tssrc/providers/provider-proxy-loader-d-g.test.tssrc/providers/provider-proxy-loader-h-m.test.tssrc/providers/provider-proxy-loader-n-s.test.tssrc/providers/provider-proxy-loader-t-z.test.tssrc/providers/proxy.registry.ts
No description provided.