feat(api): 🚀 add WebSocket support and integrate new SSO adapters#688
feat(api): 🚀 add WebSocket support and integrate new SSO adapters#688aXenDeveloper wants to merge 1 commit intocanaryfrom
Conversation
- Removed old Copilot instructions and added new guidelines in `AGENTS.md`. - Introduced WebSocket functionality using `@hono/node-ws` for real-time communication. - Updated SSO adapters to use `SSOApiAdapter` instead of `SSOApiPlugin` for better type safety. - Enhanced documentation for WebSocket and SSO integration in the API. - Updated various components and configurations to support the new WebSocket and SSO structures.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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 enhances the VitNode API by introducing native WebSocket support for real-time interactions and refining the SSO integration mechanism for better developer experience and type safety. It also includes comprehensive updates to the project's documentation, providing clearer guidelines and expanding on core features, ensuring that developers have up-to-date resources for building and extending applications within the framework. 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
Activity
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 WebSocket support to the VitNode API, including new dependencies (@hono/node-ws, @vitnode/blog), WebSocket server implementation in apps/api/src/index.ts, and corresponding documentation. It also renames SSOApiPlugin to SSOApiAdapter across core files and documentation for consistency. Review feedback highlights several areas for improvement: removing a trailing comma in the AGENTS.md guidelines, replacing console.log with structured logging in WebSocket event handlers for better observability, correcting the instantiation of SSO adapters in documentation examples by removing the new keyword to avoid TypeErrors, completing the WebsocketAdapter type definition to include all expected Hono WebSocket handlers, and removing a debugging test method from WebsocketModel for code cleanliness.
|
|
||
| ## Main Rules | ||
|
|
||
| - Use React 19.2, Next.js 16, TypeScript 5.9, Hono.js 4, |
There was a problem hiding this comment.
This list item has a trailing comma. For clarity and grammatical correctness in documentation, it should be removed.
| - Use React 19.2, Next.js 16, TypeScript 5.9, Hono.js 4, | |
| - Use React 19.2, Next.js 16, TypeScript 5.9, Hono.js 4 |
| upgradeWebSocket(c => ({ | ||
| onOpen(event, ws) { | ||
| const user = c.get("user"); | ||
| console.log("Connection opened", event, ws, user); | ||
| }, | ||
| onMessage(event, ws) { | ||
| console.log(`Message from client`, event.data); | ||
| ws.send("Hello from server!"); | ||
| }, | ||
| onClose: () => { | ||
| console.log("Connection closed"); | ||
| }, | ||
| })), |
There was a problem hiding this comment.
The WebSocket event handlers use console.log for logging. For better observability and to align with the rest of the application, please use the structured logger available via c.get('log'). This will also prevent logging potentially large or sensitive objects like event and ws directly to the console.
| upgradeWebSocket(c => ({ | |
| onOpen(event, ws) { | |
| const user = c.get("user"); | |
| console.log("Connection opened", event, ws, user); | |
| }, | |
| onMessage(event, ws) { | |
| console.log(`Message from client`, event.data); | |
| ws.send("Hello from server!"); | |
| }, | |
| onClose: () => { | |
| console.log("Connection closed"); | |
| }, | |
| })), | |
| upgradeWebSocket(c => ({ | |
| onOpen(event, ws) { | |
| const user = c.get("user"); | |
| c.get("log").info({ userId: user?.id }, "WebSocket connection opened"); | |
| }, | |
| onMessage(event, ws) { | |
| c.get("log").info({ data: event.data }, "Message from client"); | |
| ws.send("Hello from server!"); | |
| }, | |
| onClose: () => { | |
| c.get("log").info("WebSocket connection closed"); | |
| }, | |
| })) |
| ssoAdapters: [ | ||
| // [!code ++] | ||
| new DiscordSSOApiPlugin({ | ||
| new DiscordSSOApiAdapter({ |
There was a problem hiding this comment.
The DiscordSSOApiAdapter is a factory function, not a class constructor. Using the new keyword here is incorrect and will result in a TypeError. Please remove new to correctly call the function.
DiscordSSOApiAdapter({
| ssoAdapters: [ | ||
| // [!code ++] | ||
| new FacebookSSOApiPlugin({ | ||
| new FacebookSSOApiAdapter({ |
There was a problem hiding this comment.
The FacebookSSOApiAdapter is a factory function, not a class constructor. Using the new keyword here is incorrect and will result in a TypeError. Please remove new to correctly call the function.
FacebookSSOApiAdapter({
| ssoAdapters: [ | ||
| // [!code ++] | ||
| new GoogleSSOApiPlugin({ | ||
| new GoogleSSOApiAdapter({ |
There was a problem hiding this comment.
The GoogleSSOApiAdapter is a factory function, not a class constructor. Using the new keyword here is incorrect and will result in a TypeError. Please remove new to correctly call the function.
GoogleSSOApiAdapter({
| ssoAdapters: [ | ||
| // [!code ++] | ||
| new DiscordSSOApiPlugin({ | ||
| new DiscordSSOApiAdapter({ |
There was a problem hiding this comment.
The DiscordSSOApiAdapter is a factory function, not a class constructor. Using the new keyword here is incorrect and will result in a TypeError. Please remove new to correctly call the function.
DiscordSSOApiAdapter({
| ssoAdapters: [ | ||
| // [!code ++] | ||
| new FacebookSSOApiPlugin({ | ||
| new FacebookSSOApiAdapter({ |
There was a problem hiding this comment.
The FacebookSSOApiAdapter is a factory function, not a class constructor. Using the new keyword here is incorrect and will result in a TypeError. Please remove new to correctly call the function.
FacebookSSOApiAdapter({
| ssoAdapters: [ | ||
| // [!code ++] | ||
| new GoogleSSOApiPlugin({ | ||
| new GoogleSSOApiAdapter({ |
There was a problem hiding this comment.
The GoogleSSOApiAdapter is a factory function, not a class constructor. Using the new keyword here is incorrect and will result in a TypeError. Please remove new to correctly call the function.
GoogleSSOApiAdapter({
| export type WebsocketAdapter = (c: Context) => { | ||
| onOpen: () => void; | ||
| }; |
There was a problem hiding this comment.
The WebsocketAdapter type is incomplete. The onOpen handler should accept arguments, and other handlers like onMessage, onClose, and onError are missing. Please update the type to accurately represent the WebSocket handlers provided by Hono, as shown in the example implementation in this file.
export type WebsocketAdapter = (c: Context) => {
onOpen?: (evt: Event, ws: WSContext) => void;
onMessage?: (event: MessageEvent, ws: WSContext) => void;
onClose?: (event: CloseEvent, ws: WSContext) => void;
onError?: (error: Error, ws: WSContext) => void;
};| test() { | ||
| console.log("WebsocketModel test method called"); | ||
|
|
||
| const user = this.c.get("user"); | ||
| } |
There was a problem hiding this comment.
This test method appears to be for debugging purposes. It contains a console.log statement and an unused user variable. This should be removed before merging to keep the model clean.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5d73304ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cronSecret: CONFIG.cronJobSecret, | ||
| plugins: pluginsMetadata, | ||
| cron: cronMetadata, | ||
| websocketManager: new WebsocketManager(c), |
There was a problem hiding this comment.
globalMiddleware creates new WebsocketManager(c) inside the per-request handler, so each HTTP/WebSocket upgrade gets its own fresh connections map and previously tracked sockets are never visible to later requests. Any feature that relies on getConnection/getAllConnections for cross-client messaging (broadcast, targeted push, cleanup) will silently fail because the registry is request-scoped instead of process-scoped.
Useful? React with 👍 / 👎.
| type: "cloudflare_turnstile" | "recaptcha_v3"; | ||
| }; | ||
| cronAdapter?: CronAdapter; | ||
| websocketAdapter?: WebsocketAdapter; |
There was a problem hiding this comment.
This commit adds websocketAdapter to the public VitNodeApiConfig, but the API bootstrap path does not consume it, so setting this option has no runtime effect. That creates a misleading public contract where users can configure an adapter that is never invoked, and WebSocket behavior remains hardcoded elsewhere.
Useful? React with 👍 / 👎.
1 participant
AGENTS.md.@hono/node-wsfor real-time communication.SSOApiAdapterinstead ofSSOApiPluginfor better type safety.Improving Documentation
pnpm lint:fixto fix formatting issues before opening the PR.Description
What?
Why?