feat: add network proxy settings and application menu by cyio · Pull Request #46 · bsv-blockchain/bsv-desktop

cyio · 2026-05-11T01:32:35Z

This PR introduces application-wide HTTP proxy settings and a system application menu for BSV Desktop.

Changes:

Proxy Settings:
- Added a new "Network" settings dialog to configure an HTTP proxy.
- Proxy settings are persisted to disk (network-settings.json) and applied on application startup.
- Provided a way to restart the app to apply changes.
Application Menu:
- Implemented a standard application menu for both macOS and Windows/Linux.
- Added a "Settings > Network" menu item (shortcut: CmdOrCtrl+,) to quickly access proxy settings.
Cleanup & Relaunch:
- Improved application cleanup logic before exit/restart.
- Added an IPC handler to relaunch the application.

Motivation:

Allows users in restricted network environments to use BSV Desktop by routing traffic through an HTTP proxy.

Copilot

Pull request overview

Adds an application menu entry to open a new Network settings dialog, enabling users to configure a persisted HTTP proxy that is applied on app startup, plus an IPC-based app relaunch path.

Changes:

Added a renderer “Network” dialog for configuring/storing proxy settings and triggering restart.
Added main-process persistence + startup application of proxy settings via session.defaultSession.setProxy.
Introduced a cross-platform application menu with “Settings > Network” and an app:restart IPC handler, plus refactored cleanup logic.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/lib/UserInterface.tsx	Mounts the new `NetworkSettingsDialog` so it can be opened via IPC/menu.
src/lib/components/NetworkSettingsDialog.tsx	New UI for proxy configuration, persistence via IPC, and restart flow.
src/global.d.ts	Extends `window.electronAPI` typing to include `network` and `app.restart`.
electron/preload.ts	Exposes new `network` and `app.restart` APIs to the renderer via `contextBridge`.
electron/networkSettings.ts	Implements proxy settings read/write/validate and IPC registration; applies proxy via `session.setProxy`.
electron/main.ts	Registers network IPC, applies persisted proxy at startup, adds restart IPC, and centralizes cleanup before exit.
electron/appMenu.ts	Implements an application menu with “Settings > Network” to open the dialog.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

sirdeggen · 2026-07-09T16:17:05Z

+    return;
+  }
+
+  await session.defaultSession.setProxy({


[bug] Proxy settings are applied only through session.defaultSession.setProxy. That covers renderer/fetch and other Chromium-session traffic, but not Node networking used elsewhere in the app. The dialog copy describes an “application-wide” proxy, yet after restart the monitor worker (Services from @bsv/wallet-toolbox), main-process node-fetch (manifest proxy), and electron-updater continue to connect directly. For the common case where a proxy is required to reach the internet, this can leave core wallet background work and updates non-functional while the UI appears configured.

Suggestion: Either (a) make proxy coverage truly app-wide—set/clear HTTP_PROXY/HTTPS_PROXY/NO_PROXY early at startup (so forked monitor workers inherit them), route main-process HTTP through a proxy-aware agent or Electron net/session fetch, and document updater behavior—or (b) narrow the UI copy to “Chromium/renderer traffic only” and explicitly list what is not proxied. Prefer (a) if the feature is meant for corporate/firewall users.

sirdeggen · 2026-07-09T16:17:05Z

+    try {
+      const validated = validateProxySettings(settings);
+      writeProxySettings(validated);
+      return { success: true, settings: validated, restartRequired: true };


[suggestion] network:set-proxy-settings only validates and writes to disk; it never calls applyProxySettings, and always returns restartRequired: true. Chromium session proxies can be applied immediately with setProxy, so forcing a full relaunch is heavier than necessary for renderer traffic. Restart also does not fix Node traffic (Issues 1–2), so the restart UX can give a false sense that everything will pick up the new proxy.

Suggestion: Apply settings immediately via applyProxySettings(validated) after a successful write. Only require restart if/when Node/worker processes need it, and set restartRequired based on that reality (or remove it if live apply is enough).

sirdeggen · 2026-07-09T16:17:05Z

+    const settings = await window.electronAPI.network.getProxySettings()
+    setProxyEnabled(settings.mode === 'fixed_servers' && Boolean(settings.proxyRules))
+    setProxyUrl(settings.proxyRules || settings.lastProxyRules || DEFAULT_PROXY_URL)
+    setRestartAvailable(false)


[suggestion] Opening the dialog always calls loadSettings(), which resets restartAvailable to false. If the user saves, closes the dialog, then reopens Network settings, the Restart button is gone even though persisted settings still differ from the running process (under the current restart-required design).

Suggestion: Track “dirty vs applied” on the main side (e.g. return whether applied settings match disk, or expose restartRequired from get) and restore the Restart affordance whenever saved settings are not yet applied. Alternatively apply live (Issue 3) and drop the Restart button.

sirdeggen · 2026-07-09T16:17:05Z

+  const loadSettings = useCallback(async () => {
+    if (!hasElectronNetworkApi) return
+
+    const settings = await window.electronAPI.network.getProxySettings()


[suggestion] loadSettings awaits getProxySettings() with no try/catch. An IPC failure becomes an unhandled rejection and leaves the dialog open with default/stale fields and no user-facing error.

Suggestion: Wrap the load in try/catch, toast an error, and optionally keep the dialog closed or show a disabled error state.

sirdeggen · 2026-07-09T16:17:05Z

+    }
+  ];
+
+  Menu.setApplicationMenu(Menu.buildFromTemplate(template));


[suggestion] Menu.setApplicationMenu replaces Electron’s default menu. On Windows/Linux the new template has no Quit/Exit item and no standard File menu, so users lose the usual menu path to quit (they must close the window). macOS still has Quit under the app menu.

Suggestion: Add a File (or app) menu on non-darwin with { role: 'quit' }, or append a Quit item under Settings/Window for Windows/Linux.

sirdeggen · 2026-07-09T16:17:05Z

+      ]
+    },
+    {
+      label: 'Edit',


[suggestion] The Network menu action only does getMainWindow()?.webContents.send('network-settings:open'). If the window is minimized/hidden, or React has not yet registered the listener, the event is easy to miss and no window is brought forward.

Suggestion: Restore/show/focus the main window before send; optionally buffer a “pending open network settings” flag in main until the renderer acknowledges readiness.

sirdeggen · 2026-07-09T16:17:05Z

+    app.exit(0);
+  }
+
+  return cleanupError


[nit] In app:restart, app.relaunch() and app.exit(0) run in a finally block, so the following return cleanupError ? … : { success: true } is unreachable. The renderer’s await window.electronAPI.app.restart() will not observe that result.

Suggestion: Drop the dead return (and the unused cleanupError plumbing), or only exit after the invoke response is sent if you truly need a result—usually exit-after-cleanup is enough.

sirdeggen · 2026-07-09T16:17:05Z

+    removeOpenSettingsListener: (callback: () => void) => void;
+  };
+  app: {
+    restart: () => Promise<void>;


[nit] app.restart is typed as Promise<void> (also in src/global.d.ts), while the main handler attempts to return { success, error }. Types and implementation disagree (and the return is currently unreachable anyway).

Suggestion: Align the type with the real contract (Promise<void> if the process always exits; otherwise a success/error object) across preload, global.d.ts, and main.

sirdeggen · 2026-07-09T16:17:05Z

+  lastProxyRules?: string;
+}
+
+const DEFAULT_PROXY_URL = 'http://127.0.0.1:7890'


[nit] DEFAULT_PROXY_URL is hard-coded to http://127.0.0.1:7890 (common Clash/V2Ray port). Users who enable the switch and save without editing may unintentionally point traffic at a local proxy that is not running, causing confusing connection failures after restart.

Suggestion: Use an empty placeholder (http://host:port) and require an explicit URL when enabling, or detect/display a clearer empty state instead of a tool-specific default.

sirdeggen · 2026-07-09T17:08:55Z

@cyio thanks for this, coming back to it two months later. Apologies for the wait, but I had to focus elsewhere for a while. I'm going to make some tweaks based on this review and get things merged.

Keep proxy settings silent for users who never open them: leave Chromium defaults alone when no network-settings.json exists, apply live when saved, and only offer restart when monitor workers need it. Restore a standard File/Edit/View/Window/Help menu with Quit on all platforms, focus the window before opening Network Settings, honor session proxies for manifest fetch, and set HTTP(S)_PROXY for forked workers so coverage is closer to application-wide.

Add network proxy settings

a169c2e

sirdeggen requested a review from Copilot May 12, 2026 03:57

Copilot started reviewing on behalf of sirdeggen May 12, 2026 03:57 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

Comment thread src/lib/components/NetworkSettingsDialog.tsx Outdated

Comment thread electron/networkSettings.ts Outdated

Comment thread electron/main.ts Outdated

Comment thread electron/main.ts Outdated

Comment thread electron/main.ts Outdated

Apply suggestions from code review

00cc529

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

sirdeggen reviewed Jul 9, 2026

View reviewed changes

sirdeggen added 3 commits July 9, 2026 12:10

Merge master into feature/network-proxy-settings

efadd9b

2.6.2

52ee0e7

sirdeggen merged commit da80a70 into bsv-blockchain:master Jul 9, 2026

Uh oh!

Conversation

cyio commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes:

Motivation:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sirdeggen commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cyio commented May 11, 2026 •

edited

Loading