src/main/browser/utility/menu/items/app.ts (1)

16-24: Good implementation of the menu checkbox with proper state handling.

The checkbox menu item is well implemented with:

• Appropriate label "Set as Default Browser"
• Click handler that calls setDefaultBrowser()
• Dynamic checked state based on isDefaultBrowser()
• Disabling the checkbox when the app is already the default browser

Consider adding error handling or user feedback in case setting the default browser fails:

  click: () => {
-   setDefaultBrowser();
+   const success = setDefaultBrowser();
+   if (!success) {
+     // Show notification or log error
+   }
  },

src/renderer/src/components/settings/sections/general/set-as-default-browser-setting.tsx (1)

9-18: Polling interval might be too aggressive.

While the implementation works correctly, polling every 2 seconds (2000ms) could potentially cause unnecessary IPC traffic.

Consider using a slightly longer interval (e.g., 5000ms) to reduce overhead while still providing timely updates:

-   const interval = setInterval(refetchDefaultBrowser, 2000);
+   const interval = setInterval(refetchDefaultBrowser, 5000);

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/main/modules/default-browser.ts (1)

16-19: Potential logical inconsistency in return value.

The function returns a Promise that resolves to true if either HTTP or HTTPS protocol was successfully set, but isDefaultBrowser() returns true only if both are set. This could lead to confusing behavior where setDefaultBrowser() returns true but immediately calling isDefaultBrowser() returns false.

Consider changing the return value to ensure consistency:

export function setDefaultBrowser() {
-  app.setAsDefaultProtocolClient("http");
-  app.setAsDefaultProtocolClient("https");
+  const httpSucceed = app.setAsDefaultProtocolClient("http");
+  const httpsSucceed = app.setAsDefaultProtocolClient("https");
+  const bothSucceed = httpSucceed && httpsSucceed;

  return new Promise((resolve) => {

Then use bothSucceed in your platform-specific logic.

assets/default-app/windows.bat (1)

87-88: Consider security implications of self-deletion.

While the self-deletion feature is commented out, implementing it could lead to confusion if users need to run the script again. Additionally, it removes evidence of what changed the system configuration, which could complicate troubleshooting.

Either remove this commented code or add more context about when self-deletion would be appropriate.

src/main/modules/default-browser-windows.ts (1)

10-13: Consider fetching app description from package.json.

The app description is hardcoded in this file. For better maintainability, consider extracting it from your package.json file.

const APP_NAME_SHORT = "flow"; 
const APP_NAME = app.getName(); 
- const APP_DESCRIPTION = "An experimental browser built on Electron."; 
+ // Get description from package.json
+ const APP_DESCRIPTION = require("../../../package.json").description || "An experimental browser built on Electron.";

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

assets/default-app/register_app_user.bat (2)

18-25: Consider enhancing executable validation with additional checks.

The executable existence check is good, but could be enhanced for robustness.

rem Check if executable exists
if not exist "%APP_PATH%" (
    echo ERROR: Executable not found at "%APP_PATH%"
    pause
    goto :eof
)
+rem Ensure the path has .exe extension for an application
+if /i not "%APP_PATH:~-4%" == ".exe" (
+    echo WARNING: The specified path does not end with .exe: "%APP_PATH%"
+    echo This might not be a valid application.
+    pause
+)

---

84-88: Consider fallback for older Windows versions.

The script tries to open Settings using ms-settings:defaultapps, which works on Windows 10/11, but might fail on older Windows versions.

rem Try opening the modern Default Apps settings page (Win 10/11)
start "" ms-settings:defaultapps

+rem Fallback for older Windows versions if the above fails
+if %ERRORLEVEL% NEQ 0 (
+    echo Falling back to classic Control Panel for Default Programs...
+    control /name Microsoft.DefaultPrograms /page pageDefaultProgram
+)

rem Optional: Self-deletion - remove the 'rem' below if you want the script to delete itself
rem (goto) 2>nul & del "%~f0"

src/main/modules/default-browser.ts (1)

29-32: Document macOS behavior for better maintainability.

On macOS, the function assumes Electron's API shows the default app request dialog, but this should be better documented.

    } else if (process.platform === "darwin") {
-      // Electron API should be enough to show a popup for default app request
+      // On macOS, calling setAsDefaultProtocolClient shows a system dialog
+      // asking the user for permission. We've already called it above.
+      // Reference: https://www.electronjs.org/docs/latest/api/app#appsetasdefaultprotocolclientprotocol-path-args
      resolve(true);
      return;
    }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/renderer/src/components/onboarding/screen.tsx (1)

180-188: Per-shape filter: blur() is a GPU hotspot

Applying ctx.filter = 'blur(...)' inside the inner drawing loop triggers
a state change for every shape, which is one of the slowest canvas
operations – especially noticeable when targetShapeCount creeps towards
maxShapes.

Consider:

1. Drawing each shape to an off-screen canvas once and blitting it.
2. Moving the blur to CSS by layering an extra <canvas> with
   filter: blur(...) applied globally.
3. Dialling down the blur or only applying it to circles, which benefit
   visually the most.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/main/modules/default-browser-windows.ts (2)

17-17: ⚠️ Potential issue

File path mismatch for batch script.

The code is looking for register_app_user.bat but according to a previous review comment, the actual file is named windows.bat. This path mismatch will cause the registration process to fail.

- const scriptPath = path.join(PATHS.ASSETS, "default-app", "register_app_user.bat");
+ const scriptPath = path.join(PATHS.ASSETS, "default-app", "windows.bat");

---

30-31: ⚠️ Potential issue

Avoid using async Promise executor.

Using an async function as a Promise executor is problematic because errors thrown from the async function can't be caught by the Promise's catch handler. This pattern is discouraged and flagged by static analysis tools.

- // eslint-disable-next-line no-async-promise-executor
- return new Promise(async (resolve) => {
+ return new Promise((resolve) => {
+   (async () => {
      let scriptContents: Buffer | string;
      try {
        // Implementation...
+   })().catch(err => {
+     console.error("Unexpected error in registerAppForCurrentUserOnWindows:", err);
+     resolve(false);
+   });

src/main/modules/default-browser-windows.ts (1)

76-77: Add timeout handling for the script execution.

Currently, there's no timeout for the script execution, which could potentially hang indefinitely. Consider adding a timeout mechanism to ensure the operation completes within a reasonable timeframe.

exec(command, async (err) => {
  if (err) {
    // Log the specific error from the exec call
    console.error("Error executing registration script:", err.message);
    resolve(false);
  } else {
    console.log(`Registration script executed successfully for "${APP_NAME}". Check Default Apps settings.`);
+   // Consider observing if the default browser setting has actually changed
    resolve(true);
  }

You could also implement a timeout with something like:

const execWithTimeout = (command: string, timeoutMs = 30000): Promise<boolean> => {
  return new Promise((resolve) => {
    const process = exec(command);
    
    const timeoutId = setTimeout(() => {
      if (process.pid) {
        // Kill the process if it's still running
        exec(`taskkill /pid ${process.pid} /f /t`);
      }
      console.error("Registration script execution timed out");
      resolve(false);
    }, timeoutMs);
    
    process.on('exit', (code) => {
      clearTimeout(timeoutId);
      resolve(code === 0);
    });
    
    process.on('error', (err) => {
      clearTimeout(timeoutId);
      console.error("Error executing registration script:", err.message);
      resolve(false);
    });
  });
};

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/main/modules/default-browser-windows.ts (2)

53-54: ⚠️ Potential issue

Avoid using async Promise executor.

Using an async function as a Promise executor is problematic because errors thrown from the async function can't be caught by the Promise's catch handler. This was previously flagged in a review.

- // eslint-disable-next-line no-async-promise-executor
- return new Promise(async (resolve) => {
+ return new Promise((resolve) => {
+   (async () => {
      let scriptTemplate: string;
      try {
        // Implementation...
+   })().catch(err => {
+     console.error("Unexpected error in registerAppForCurrentUserOnWindows:", err);
+     resolve(false);
+   });

---

113-117: ⚠️ Potential issue

Input validation needed for command execution.

The application name, description, and paths are directly incorporated into a command string without validation or sanitization. This could lead to command injection issues if any of these values contain special characters.

+ // Sanitize inputs to prevent command injection
+ const sanitizeForCommand = (input: string): string => {
+   // Remove characters that could break command execution
+   return input.replace(/[&<>^|()%]/g, '');
+ };
+
- const arg1 = `"""${appExecutablePath}"""`;
- const arg2 = `"${APP_NAME_SHORT}"`;
- const arg3 = `"""${APP_NAME}"""`;
- const arg4 = `"""${APP_DESCRIPTION}"""`;
+ const arg1 = `"""${sanitizeForCommand(appExecutablePath)}"""`;
+ const arg2 = `"${sanitizeForCommand(APP_NAME_SHORT)}"`;
+ const arg3 = `"""${sanitizeForCommand(APP_NAME)}"""`;
+ const arg4 = `"""${sanitizeForCommand(APP_DESCRIPTION)}"""`;

src/main/modules/default-browser-windows.ts (2)

119-120: Consider removing PowerShell window after execution.

The current command doesn't hide the PowerShell window. Adding the -WindowStyle Hidden parameter would make the registration process more seamless for users.

- const command = `Powershell -NoProfile -ExecutionPolicy Bypass -Command "Start-Process -FilePath '${tempFile}' -ArgumentList ${arg1}, ${arg2}, ${arg3}, ${arg4}"`;
+ const command = `Powershell -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -Command "Start-Process -FilePath '${tempFile}' -ArgumentList ${arg1}, ${arg2}, ${arg3}, ${arg4}"`;

---

145-154: Remove commented example code in production.

The commented example code at the end of the file should be removed before production deployment as it serves no functional purpose and may confuse future developers.

- // --- Example Usage ---
- /*
- registerAppForCurrentUser().then(success => {
-   if (success) {
-     console.log("Application registration process completed successfully.");
-   } else {
-     console.error("Application registration process failed.");
-   }
- });
- */

assets/default-app/register_app_user_template.bat (2)

84-85: Consider fallback for older Windows versions.

The script only attempts to open the modern Settings app with ms-settings:defaultapps, which may not work on older Windows versions (pre-Windows 10). Consider adding a fallback for compatibility.

rem Try opening the modern Default Apps settings page (Win 10/11)
start "" ms-settings:defaultapps

+ rem Fallback for older Windows versions (Win 7/8)
+ if %ERRORLEVEL% neq 0 (
+     start control /name Microsoft.DefaultPrograms /page pageDefaultProgram
+ )

---

87-88: Self-deletion mechanism may be unnecessary.

Since the script is being called from Node.js which already handles the temporary file deletion (in the TypeScript code), this self-deletion mechanism is redundant and can be removed.

- rem Optional: Self-deletion - remove the 'rem' below if you want the script to delete itself
- rem (goto) 2>nul & del "%~f0"

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/main/modules/default-browser-windows.ts (3)

124-134: Improve error handling in exec callback.

The current error handling in the exec callback only logs the error message. Providing more detailed error information would aid in troubleshooting.

- exec(command, async (err, stdout) => {
+ exec(command, async (err, stdout, stderr) => {
  if (err) {
-   console.error("Error executing registration script:", err.message);
+   console.error("Error executing registration script:", {
+     message: err.message,
+     code: err.code,
+     stderr: stderr.trim()
+   });
    resolve(false);
  } else {
    console.log(`Registration script executed successfully for "${APP_NAME}". Check Default Apps settings.`);
    console.log("Output:", stdout);
    resolve(true);
  }

---

53-54: ⚠️ Potential issue

Fix the async Promise executor pattern.

Using an async function as a Promise executor is problematic because errors thrown from the async function can't be caught by the Promise's catch handler.

Apply this fix:

- // eslint-disable-next-line no-async-promise-executor
- return new Promise(async (resolve) => {
+ return new Promise((resolve) => {
+   (async () => {
      let scriptTemplate: string;
      try {
        // ...implementation...
+   })().catch(err => {
+     console.error("Unexpected error in registerAppForCurrentUserOnWindows:", err);
+     resolve(false);
+   });

---

113-118: ⚠️ Potential issue

Add input validation for command execution.

The application name, description, and paths are directly incorporated into a command string without validation. This could potentially lead to command injection issues if these values contain special characters.

Add a sanitization function:

+ // Sanitize inputs to prevent command injection
+ const sanitizeForCommand = (input: string): string => {
+   // Remove characters that could break command execution
+   return input.replace(/[&<>^|()%]/g, '');
+ };
+
- const arg1 = `"""${appExecutablePath}"""`;
- const arg2 = `"${APP_NAME_SHORT}"`;
- const arg3 = `"""${APP_NAME}"""`;
- const arg4 = `"""${APP_DESCRIPTION}"""`;
+ const arg1 = `"""${sanitizeForCommand(appExecutablePath)}"""`;
+ const arg2 = `"${sanitizeForCommand(APP_NAME_SHORT)}"`;
+ const arg3 = `"""${sanitizeForCommand(APP_NAME)}"""`;
+ const arg4 = `"""${sanitizeForCommand(APP_DESCRIPTION)}"""`;

src/main/index.ts (2)

152-153: Fix typo in comment.

There's a typo in the comment "Hnadle" instead of "Handle".

- // Hnadle command line arguments
+ // Handle command line arguments

---

94-130: Consider adding a timeout parameter to the handleOpenUrl function.

The current implementation tries to find a window 5 times with a fixed 50ms delay. This approach might not be flexible enough for different system conditions.

Consider refactoring the retry logic to be more configurable:

- const handleOpenUrl = async (url: string) => {
+ const handleOpenUrl = async (url: string, maxRetries = 5, retryDelay = 50) => {
  if (!browser) return;

  if (!app.isReady) {
    await app.whenReady();
  }

  let window: TabbedBrowserWindow | null = null;

- for (let i = 0; i < 5; i++) {
+ for (let i = 0; i < maxRetries; i++) {
    // Check if there is a focused window
    const focusedWindow = browser.getFocusedWindow();
    if (focusedWindow) {
      window = focusedWindow;
      break;
    }

    // Check for any window
    const firstWindow = browser.getWindows()[0];
    if (firstWindow) {
      window = firstWindow;
      break;
    }

-   await sleep(50);
+   await sleep(retryDelay);
  }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro