-
Notifications
You must be signed in to change notification settings - Fork 523
Neon / portal template support #713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cubic analysis
10 issues found across 44 files • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
export const NeonConfigure = () => { | ||
const selectedAppId = useAtomValue(selectedAppIdAtom); | ||
const { app } = useLoadApp(selectedAppId); | ||
const [] = useState<string | null>(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useState result is destructured into an empty array, making the hook call pointless and creating unnecessary state.
Prompt for AI agents
Address the following comment on src/components/preview_panel/NeonConfigure.tsx at line 37:
<comment>useState result is destructured into an empty array, making the hook call pointless and creating unnecessary state.</comment>
<file context>
@@ -0,0 +1,183 @@
+import { useQuery } from "@tanstack/react-query";
+import { useAtomValue } from "jotai";
+import { useState } from "react";
+import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card";
+import { Badge } from "@/components/ui/badge";
+
+import {} from "@/components/ui/alert-dialog";
+import { Database, GitBranch } from "lucide-react";
+import { selectedAppIdAtom } from "@/atoms/appAtoms";
</file context>
@@ -54,6 +57,7 @@ export const messages = sqliteTable("messages", { | |||
// Define relations | |||
export const appsRelations = relations(apps, ({ many }) => ({ | |||
chats: many(chats), | |||
versions: many(versions), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
versions is referenced before it is declared, so evaluating relations() will throw a runtime ReferenceError in the module’s top-level code.
Prompt for AI agents
Address the following comment on src/db/schema.ts at line 60:
<comment>versions is referenced before it is declared, so evaluating relations() will throw a runtime ReferenceError in the module’s top-level code.</comment>
<file context>
@@ -54,6 +57,7 @@ export const messages = sqliteTable("messages", {
// Define relations
export const appsRelations = relations(apps, ({ many }) => ({
chats: many(chats),
+ versions: many(versions),
}));
</file context>
setLoading(false); | ||
} | ||
}, | ||
[processAppOutput], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
appUrlObj is referenced inside this callback but omitted from the dependency array, which can cause the function to use an outdated URL object
Prompt for AI agents
Address the following comment on src/hooks/useRunApp.ts at line 106:
<comment>appUrlObj is referenced inside this callback but omitted from the dependency array, which can cause the function to use an outdated URL object</comment>
<file context>
@@ -45,41 +46,65 @@ export function useRunApp() {
}
}
};
- const runApp = useCallback(async (appId: number) => {
- setLoading(true);
- try {
- const ipcClient = IpcClient.getInstance();
- console.debug("Running app", appId);
</file context>
[processAppOutput], | |
[processAppOutput, appUrlObj], |
// Process proxy server output | ||
processProxyServerOutput(output); | ||
}, | ||
[setAppOutput], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
processProxyServerOutput is used inside this callback but is not listed in the dependency array, so React may work with a stale reference and skip necessary updates
Prompt for AI agents
Address the following comment on src/hooks/useRunApp.ts at line 71:
<comment>processProxyServerOutput is used inside this callback but is not listed in the dependency array, so React may work with a stale reference and skip necessary updates</comment>
<file context>
@@ -45,41 +46,65 @@ export function useRunApp() {
}
}
};
- const runApp = useCallback(async (appId: number) => {
- setLoading(true);
- try {
- const ipcClient = IpcClient.getInstance();
- console.debug("Running app", appId);
</file context>
[setAppOutput], | |
[setAppOutput, processProxyServerOutput], |
</div> | ||
<div className="overflow-y-auto h-[calc(100%-60px)]"> | ||
{versions.length === 0 ? ( | ||
<div className="p-4 ">No versions available</div> | ||
) : ( | ||
<div className="divide-y divide-border"> | ||
{versions.map((version: Version, index) => ( | ||
{versions.map((version: Version) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rendering cost grows quadratically because each iteration performs versions.findIndex to recover the index that was already available from map’s callback.
Prompt for AI agents
Address the following comment on src/components/chat/VersionPane.tsx at line 121:
<comment>Rendering cost grows quadratically because each iteration performs versions.findIndex to recover the index that was already available from map’s callback.</comment>
<file context>
@@ -94,21 +102,23 @@ export function VersionPane({ isVisible, onClose }: VersionPaneProps) {
return (
<div className="h-full border-t border-2 border-border w-full">
<div className="p-2 border-b border-border flex items-center justify-between">
- <h2 className="text-base font-semibold pl-2">Version History</h2>
- <button
- onClick={onClose}
- className="p-1 hover:bg-(--background-lightest) rounded-md "
- aria-label="Close version pane"
- >
</file context>
{versions.map((version: Version) => ( | |
{versions.map((version: Version, index) => ( |
/> | ||
<defs> | ||
<linearGradient | ||
id="a" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coded gradient ids "a" and "b" are not unique; rendering multiple NeonSvg components will duplicate these ids in the DOM and can break SVG references.
Prompt for AI agents
Address the following comment on src/components/NeonConnector.tsx at line 133:
<comment>Hard-coded gradient ids "a" and "b" are not unique; rendering multiple NeonSvg components will duplicate these ids in the DOM and can break SVG references.</comment>
<file context>
@@ -0,0 +1,157 @@
+import { useEffect } from "react";
+import { Button } from "@/components/ui/button";
+import { IpcClient } from "@/ipc/ipc_client";
+import { toast } from "sonner";
+import { useSettings } from "@/hooks/useSettings";
+
+import { useDeepLink } from "@/contexts/DeepLinkContext";
+import { ExternalLink } from "lucide-react";
+import { useTheme } from "@/contexts/ThemeContext";
</file context>
} | ||
}; | ||
handleDeepLink(); | ||
}, [lastDeepLink]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refreshSettings is used inside the effect but is missing from the dependency array, which can cause stale state if the hook ever returns a new function.
Prompt for AI agents
Address the following comment on src/components/NeonConnector.tsx at line 25:
<comment>refreshSettings is used inside the effect but is missing from the dependency array, which can cause stale state if the hook ever returns a new function.</comment>
<file context>
@@ -0,0 +1,157 @@
+import { useEffect } from "react";
+import { Button } from "@/components/ui/button";
+import { IpcClient } from "@/ipc/ipc_client";
+import { toast } from "sonner";
+import { useSettings } from "@/hooks/useSettings";
+
+import { useDeepLink } from "@/contexts/DeepLinkContext";
+import { ExternalLink } from "lucide-react";
+import { useTheme } from "@/contexts/ThemeContext";
</file context>
}, [lastDeepLink]); | |
}, [lastDeepLink, refreshSettings]); |
// Send message to renderer to trigger re-render | ||
mainWindow?.webContents.send("deep-link-received", { | ||
type: parsed.hostname, | ||
url, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sensitive OAuth tokens are forwarded to the renderer via IPC, exposing credentials outside the trusted main process (This reflects feedback from previous reviews about avoiding accidental leakage of secrets to the renderer).
Prompt for AI agents
Address the following comment on src/main.ts at line 227:
<comment>Sensitive OAuth tokens are forwarded to the renderer via IPC, exposing credentials outside the trusted main process (This reflects feedback from previous reviews about avoiding accidental leakage of secrets to the renderer).</comment>
<file context>
@@ -208,6 +209,25 @@ function handleDeepLinkReturn(url: string) {
);
return;
}
+ if (parsed.hostname === "neon-oauth-return") {
+ const token = parsed.searchParams.get("token");
+ const refreshToken = parsed.searchParams.get("refreshToken");
+ const expiresIn = Number(parsed.searchParams.get("expiresIn"));
+ if (!token || !refreshToken || !expiresIn) {
+ dialog.showErrorBox(
</file context>
import { eq } from "drizzle-orm"; | ||
import { ipcMain } from "electron"; | ||
import { EndpointType } from "@neondatabase/api-client"; | ||
import { retryOnLocked } from "../utils/retryOnLocked"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mutual imports between neon_handlers.ts and retryOnLocked.ts create a circular dependency (neon_handlers -> retryOnLocked -> neon_handlers). Although ES modules allow cycles, this pattern risks runtime "undefined is not a function" errors if retryOnLocked is executed before neon_handlers finishes evaluating, and complicates maintainability.
Prompt for AI agents
Address the following comment on src/ipc/handlers/neon_handlers.ts at line 22:
<comment>The mutual imports between neon_handlers.ts and retryOnLocked.ts create a circular dependency (neon_handlers -> retryOnLocked -> neon_handlers). Although ES modules allow cycles, this pattern risks runtime "undefined is not a function" errors if retryOnLocked is executed before neon_handlers finishes evaluating, and complicates maintainability.</comment>
<file context>
@@ -0,0 +1,263 @@
+import log from "electron-log";
+
+import { createTestOnlyLoggedHandler } from "./safe_handle";
+import { handleNeonOAuthReturn } from "../../neon_admin/neon_return_handler";
+import {
+ getNeonClient,
+ getNeonErrorMessage,
+ getNeonOrganizationId,
+} from "../../neon_admin/neon_management_client";
</file context>
}) { | ||
// Given the connection uri, update the env var for POSTGRES_URL | ||
const envVars = parseEnvFile(await readEnvFile({ appPath })); | ||
for (const envVar of envVars) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updatePostgresUrlEnvVar rewrites the file without inserting POSTGRES_URL when it is absent, so the intended update may be silently skipped
Prompt for AI agents
Address the following comment on src/ipc/utils/app_env_var_utils.ts at line 26:
<comment>updatePostgresUrlEnvVar rewrites the file without inserting POSTGRES_URL when it is absent, so the intended update may be silently skipped</comment>
<file context>
@@ -3,7 +3,58 @@
* Environment variables are sensitive and should not be logged.
*/
+import { getDyadAppPath } from "@/paths/paths";
import { EnvVar } from "../ipc_types";
+import path from "path";
+import fs from "fs";
+
+export const ENV_FILE_NAME = ".env.local";
</file context>
Add mini-store template refactor ipc handlers / settings / basic UI create app button create app flow support y/n and basic neon setup flow child branches wip wip changes fix DB more progress restart app when db, and better loading screens gracefully handle checking out versions wihthout db polish ui better warning screen make user friendly clean up db straighten out clean up neon ui wip delete-able branches simple graph viz everything working clean DB migration more changes minor clean DYAD_DISABLE_DB_PUSH more neon Revert "more neon" This reverts commit 6e209f7. proper version semantics skip supabase available sys prompt
TODOs:
Phase 2?