Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the local development experience by introducing full HTTPS support for the frontend proxy. This change ensures that developers can work in an environment that more closely resembles production, addressing potential issues related to secure cookie handling and modern browser security policies early in the development cycle. It streamlines the process of testing features that require secure contexts, such as certain APIs or cookie attributes, without complex manual configurations. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for running the local development environment with HTTPS/HTTP2. The changes are comprehensive, touching the Vite configuration, the development proxy server, cookie handling logic, and environment variable examples. A new dependency @vitejs/plugin-basic-ssl is added to generate self-signed certificates. The logic to conditionally enable HTTPS based on environment variables is well-implemented, and the cookie rewriting logic is correctly updated to handle secure attributes when the proxy is running over HTTPS. I've added a couple of minor suggestions to improve code style and maintainability.
| const toLocalCookieName = (cookieName: string, options: LocalCookieRewriteOptions) => { | ||
| if (options.localSecure) | ||
| return cookieName | ||
|
|
||
| return cookieName.replace(SECURE_COOKIE_PREFIX_PATTERN, '') | ||
| } | ||
|
|
||
| type LocalCookieRewriteOptions = { | ||
| localSecure: boolean | ||
| } |
There was a problem hiding this comment.
For better readability and to follow a common convention in TypeScript, it's best to define types before they are used. The LocalCookieRewriteOptions type should be defined before the toLocalCookieName function that uses it.
| const toLocalCookieName = (cookieName: string, options: LocalCookieRewriteOptions) => { | |
| if (options.localSecure) | |
| return cookieName | |
| return cookieName.replace(SECURE_COOKIE_PREFIX_PATTERN, '') | |
| } | |
| type LocalCookieRewriteOptions = { | |
| localSecure: boolean | |
| } | |
| type LocalCookieRewriteOptions = { | |
| localSecure: boolean | |
| } | |
| const toLocalCookieName = (cookieName: string, options: LocalCookieRewriteOptions) => { | |
| if (options.localSecure) | |
| return cookieName | |
| return cookieName.replace(SECURE_COOKIE_PREFIX_PATTERN, '') | |
| } |
| type DevProxyEnv = Partial<Record< | ||
| | 'HONO_CONSOLE_API_PROXY_TARGET' | ||
| | 'HONO_PUBLIC_API_PROXY_TARGET', | ||
| string | ||
| > & Record< | ||
| | 'NEXT_PUBLIC_API_PREFIX' | ||
| | 'NEXT_PUBLIC_PUBLIC_API_PREFIX', | ||
| string | undefined | ||
| >> |
There was a problem hiding this comment.
The DevProxyEnv type can be simplified. Instead of using a complex intersection of Partial<Record<...>> and Record<...>, you can combine all optional properties into a single Partial<Record<...>>. This makes the type easier to read and understand.
type DevProxyEnv = Partial<Record<
| 'HONO_CONSOLE_API_PROXY_TARGET'
| 'HONO_PUBLIC_API_PROXY_TARGET'
| 'NEXT_PUBLIC_API_PREFIX'
| 'NEXT_PUBLIC_PUBLIC_API_PREFIX',
string
>>There was a problem hiding this comment.
Pull request overview
This PR adds conditional HTTPS support for the web dev environment (Vite dev server + local Hono dev proxy) when the configured API prefixes use https://..., and updates cookie rewriting so Secure/Partitioned cookies can be preserved when the local proxy is running over HTTPS.
Changes:
- Enable Vite dev-server HTTPS via
@vitejs/plugin-basic-sslwhen API prefixes are HTTPS. - Add HTTPS mode to
dev-hono-proxyand make cookie rewriting aware of whether the local proxy is secure. - Add protocol detection helper + tests for HTTPS switching and secure-cookie preservation.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/vite.config.ts | Conditionally enables basicSsl() based on env-driven HTTPS detection. |
| web/scripts/dev-hono-proxy.ts | Starts the Hono proxy as HTTPS when API prefixes indicate HTTPS, otherwise uses HTTP. |
| web/plugins/dev-proxy/protocol.ts | Adds env parsing helper to decide whether the dev proxy should use HTTPS. |
| web/plugins/dev-proxy/server.ts | Plumbs localSecure into Set-Cookie rewriting and re-exports HTTPS helper. |
| web/plugins/dev-proxy/cookies.ts | Preserves Secure/SameSite=None/Partitioned + __Host- naming when local proxy is HTTPS. |
| web/plugins/dev-proxy/server.spec.ts | Adds tests for HTTPS switching, HTTPS origin allowance, and secure cookie preservation. |
| web/package.json | Adds @vitejs/plugin-basic-ssl dependency. |
| web/pnpm-lock.yaml | Locks @vitejs/plugin-basic-ssl@2.2.0. |
| web/.env.example | Documents how to enable HTTPS mode for the dev proxy via API prefixes. |
Files not reviewed (1)
- web/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| > & Record< | ||
| | 'NEXT_PUBLIC_API_PREFIX' | ||
| | 'NEXT_PUBLIC_PUBLIC_API_PREFIX', | ||
| string | undefined | ||
| >> | ||
|
|
Important
Fixes #<issue number>.Summary
Screenshots
Checklist
make lintandmake type-check(backend) andcd web && npx lint-staged(frontend) to appease the lint gods