feat: add request code generator modal supporting cURL, fetch, axios,… - #888
feat: add request code generator modal supporting cURL, fetch, axios,…#888ayash911 wants to merge 1 commit into
Conversation
721c50e to
1d1c8ac
Compare
giemic8
left a comment
There was a problem hiding this comment.
Thanks @ayash911 — the modal UX and the breadth of generators are nice, and having unit tests per target is the right instinct. My main concerns are architectural and one is a security issue, detailed inline.
Architecture — the central issue: the renderer re-derives request semantics that already exist in the main process.
getAuthHeaderreimplements basic/bearer/oauth2/inherit header generation thatenvironmentService.getAuthorizationHeader(main) already does, including variable substitution. Two copies of security-sensitive logic will drift, and this copy already misses cases: OAuth2 only reads a cachedtokens.access_token(empty until a real send), client-credentials/PKCE flows aren't handled, and there's no mTLS/API-key awareness.resolveTemplateVariablesis a third template engine alongside the existing renderer template infra and main'ssetVariablesInString. It only sees the env vars passed in — not collection variables, secrets, or system/dynamic vars — so a generated snippet can differ from what the app actually sends. The whole point of a code generator is fidelity to the real request.
The robust design is to resolve the request once, through the existing pipeline, and have the generators consume an already-resolved (url, headers, body) tuple. The per-language functions then only do string formatting, not auth/variable logic. That also removes the any types (see below) because they'd operate on resolved primitives.
Type safety. auth: any, collectionAuth: any and const variables: any (test) violate the no-any rule in CLAUDE.md/AGENTS.md. The proper types exist: AuthorizationInformation / AuthorizationType enum, Collection['auth'], VariableObject. The string literals 'basic'|'bearer'|'oauth2'|'inherit' happen to match the enum values today, so it works by luck — use the enum.
Security. See the inline note — resolved secrets get baked into the snippet, and the modal advertises it "for integration or sharing."
Scope. MainTopBar.tsx contains save-shortcut changes unrelated to code generation that look like regressions — flagged inline. And there's the same unrelated formatting churn as #887 (auth-code-flow.ts, SidebarRequestList.tsx, template-variable-semantic-tokens-provider.ts) — please drop it.
Coverage gap. A FILE body type produces a snippet with no body in the non-form generators (cURL/fetch/axios/python only handle TEXT and FORM_DATA). Either handle it or note it as unsupported.
| /** | ||
| * Resolves template variables like {{baseUrl}} in a string against a key-value record. | ||
| */ | ||
| export function resolveTemplateVariables(text: string, variables: Record<string, string>): string { |
There was a problem hiding this comment.
This is a third template-variable engine in the codebase, parallel to the existing renderer template infrastructure and main's environmentService.setVariablesInString. It only resolves the env vars passed into the modal — not collection variables, secrets, or system/dynamic variables — so the generated snippet can diverge from what the app actually sends. A code generator's value is fidelity; please resolve through the existing pipeline rather than a fresh regex.
| * Generates authorization header for a request based on its auth settings and variables. | ||
| */ | ||
| export function getAuthHeader( | ||
| auth: any, |
There was a problem hiding this comment.
Architecture + types. This duplicates environmentService.getAuthorizationHeader (main), which already handles basic/bearer/oauth2/inherit with variable substitution. Two copies of auth logic will drift, and this one is already thinner: OAuth2 only reads a cached tokens.access_token (empty until a real request runs), and the client-credentials/auth-code/PKCE flows aren't covered.
Also: auth: any / collectionAuth?: any violate the no-any rule (CLAUDE.md). Use AuthorizationInformation and the AuthorizationType enum instead of the string literals 'basic'|'bearer'|'oauth2'|'inherit' — they match the enum values only by coincidence today.
| /** | ||
| * Generate code snippet based on selected language | ||
| */ | ||
| export function generateCodeSnippet( |
There was a problem hiding this comment.
Security. This resolves auth credentials and secret variables into the snippet as plaintext — Basic base64, bearer/OAuth tokens, and any secret env var. The modal subtitle says the snippet is "for integration or sharing," so a user copying it will leak live secrets into chats/docs/repos. Please either keep {{variable}} placeholders unresolved by default (with an opt-in to inline), mask secret-typed variables, or at minimum show a clear warning. Worth a /security-review before merge.
| await Promise.all(editor.getModels().map(saveModelContent)); | ||
|
|
||
| updateRequest(await eventService.saveChanges(request), true); | ||
| await eventService.saveChanges(request); |
There was a problem hiding this comment.
Out of scope for a code-generator PR, and looks like a regression: the previous code did updateRequest(await eventService.saveChanges(request), true) so the store received the saved request and cleared the draft flag. Now it just await eventService.saveChanges(request) and discards the result, so the UI state won't reflect the save. The if (request == null) return; guard was also dropped, so this can call saveChanges(null). Please revert these unrelated changes or split them into their own PR with justification.
| if (isSaveShortcut && request?.draft) { | ||
| event.preventDefault(); | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if ((e.metaKey || e.ctrlKey) && e.key === 's') { |
There was a problem hiding this comment.
Also unrelated to code generation and a behavior change: the old handler only saved when request?.draft was true and used event.key.toLowerCase() === 's'. Now it saves on every Cmd/Ctrl+S regardless of draft state, and e.key === 's' won't match when Shift is held (key becomes 'S'). Please keep these out of this PR.
|
|
||
| return ( | ||
| <Dialog open={isOpen} onOpenChange={onClose}> | ||
| <DialogContent className="text-text-primary border-divider flex h-[600px] max-w-4xl flex-col overflow-hidden rounded-xl border bg-[#141517] p-0 shadow-2xl"> |
There was a problem hiding this comment.
Hardcoded hex colors (bg-[#141517], #16181a, #1e1e1e, text-purple-400, #c084fc, #a855f7, bg-green-950/...) bypass the project's CSS-variable theme tokens used everywhere else. This won't adapt to the light theme and is inconsistent with the convention called out on the other UI work. Please use the existing theme tokens / Tailwind theme colors.
Changes
Adds a code generator panel to convert any request into ready-to-use snippets (#775):
CodeGeneratorModalUI featuring a clean sidebar selector for languages.code-generator-serviceto export requests as cURL commands, JS/TS fetch, Axios, and Python requests.{{variable}}and resolved auth credentials recursively.Closes #775
Testing
code-generator-service.test.tsfor all target generators.Checklist