Skip to content

[codex] Add Electron GUI#50

Open
Robertg761 wants to merge 2 commits intoLoongphy:mainfrom
Robertg761:codex/add-electron-gui
Open

[codex] Add Electron GUI#50
Robertg761 wants to merge 2 commits intoLoongphy:mainfrom
Robertg761:codex/add-electron-gui

Conversation

@Robertg761
Copy link
Copy Markdown

@Robertg761 Robertg761 commented Apr 7, 2026

Summary

Adds a GUI layer for codex-auth while keeping the existing CLI behavior as the source of truth.

What Changed

  • adds a local HTTP backend that reads the existing registry and invokes the current codex-auth commands
  • adds a compact browser UI for status, account switching, imports, cleanup, and config updates
  • adds an Electron shell so the GUI can run as a native desktop app
  • adds Electron Builder packaging plus macOS asset generation for local DMG builds
  • documents the browser and desktop GUI workflows in the README

Why

codex-auth currently requires terminal-driven account management. This change adds a visual workflow for the same operations without replacing the CLI or changing the underlying account storage model.

Notes

  • the GUI intentionally routes account actions through the existing CLI surface instead of introducing a separate account-management codepath
  • the macOS packaging flow currently produces ad-hoc signed local builds; notarization is still a follow-up step for fully trusted distribution

Validation

  • node --check gui/server.mjs
  • node --check gui/public/app.js
  • node --check gui/electron/main.cjs
  • npm run desktop:smoke
  • npm run dist

Note

Add Electron desktop GUI and local browser server for Codex Auth account management

  • Adds gui/electron/main.cjs which spawns a local HTTP server as a child process, waits for it to report its URL, and loads it in a BrowserWindow; supports a --smoke-test mode and graceful shutdown via SIGTERM/SIGKILL.
  • Adds gui/server.mjs which serves static assets from gui/public and exposes a JSON API (/api/overview, /api/refresh, /api/login, /api/switch, /api/import-*, /api/thresholds, etc.) by invoking the codex-auth binary and returning updated registry state.
  • Adds a browser-side dashboard (gui/public/app.js + gui/public/index.html) that renders account summaries, usage metrics, and action controls wired to the HTTP API.
  • Updates package.json with npm scripts to launch the server (gui:serve), start the desktop app (electron:start), and build a macOS DMG; adds electron and electron-builder as dev dependencies.
  • Bumps version to 0.2.3-alpha.3 in both package.json and src/version.zig.

Macroscope summarized 88305d8.

@Robertg761 Robertg761 marked this pull request as ready for review April 7, 2026 01:02
Comment thread gui/electron/main.cjs
Comment on lines +98 to +101
async function startServer() {
if (serverProcess && appUrl) {
return appUrl;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low electron/main.cjs:98

The guard if (serverProcess && appUrl) on line 99 returns the stale appUrl even if the server process has exited. On macOS, after the user closes all windows and the server later crashes, clicking the dock icon triggers activateboot()startServer(), which returns the dead URL and causes loadURL() to fail. Consider checking serverProcess.killed (or .exitCode !== null) to verify the process is still alive before returning the cached URL.

 async function startServer() {
-  if (serverProcess && appUrl) {
+  if (serverProcess && !serverProcess.killed && appUrl) {
     return appUrl;
   }
🤖 Copy this AI Prompt to have your agent fix this:
In file gui/electron/main.cjs around lines 98-101:

The guard `if (serverProcess && appUrl)` on line 99 returns the stale `appUrl` even if the server process has exited. On macOS, after the user closes all windows and the server later crashes, clicking the dock icon triggers `activate` → `boot()` → `startServer()`, which returns the dead URL and causes `loadURL()` to fail. Consider checking `serverProcess.killed` (or `.exitCode !== null`) to verify the process is still alive before returning the cached URL.

Evidence trail:
gui/electron/main.cjs lines 98-99 (startServer function with guard), lines 105-119 (serverProcess and appUrl assignment), lines 122-136 (stopServer which is the only place variables are reset), lines 158-161 (activate event handler calling boot()), lines 168-170 (macOS doesn't quit on window-all-closed). Verified at commit REVIEWED_COMMIT.

Comment thread gui/public/app.js
Comment on lines +237 to +239
function renderAccounts() {
const registry = state.overview.registry;
const filterText = state.filter.trim().toLowerCase();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High public/app.js:237

When loadOverview() fails, state.overview remains null, but the search input stays enabled (explicitly skipped by setBusy() at line 131). Typing in the search field triggers renderAccounts(), which accesses state.overview.registry at line 238 and throws TypeError: Cannot read property 'registry' of null. Consider guarding renderAccounts() to return early when state.overview is null, or disable the search input while the initial load is pending.

 function renderAccounts() {
+  if (!state.overview) return;
   const registry = state.overview.registry;
Also found in 1 other location(s)

gui/server.mjs:314

If the registry.json file exists but is valid JSON lacking an accounts property, registry.accounts will be undefined at line 314, causing registry.accounts.find(...) to throw a TypeError. The readRegistry() function only supplies a default accounts: [] on ENOENT, not when the JSON structure is incomplete.

🤖 Copy this AI Prompt to have your agent fix this:
In file gui/public/app.js around lines 237-239:

When `loadOverview()` fails, `state.overview` remains `null`, but the search input stays enabled (explicitly skipped by `setBusy()` at line 131). Typing in the search field triggers `renderAccounts()`, which accesses `state.overview.registry` at line 238 and throws `TypeError: Cannot read property 'registry' of null`. Consider guarding `renderAccounts()` to return early when `state.overview` is null, or disable the search input while the initial load is pending.

Evidence trail:
gui/public/app.js line 4: `state.overview: null` initialization.
gui/public/app.js lines 303-308: `loadOverview()` sets `state.overview = payload.overview` only on success; throws on failure.
gui/public/app.js lines 555-558: Initial call catches error but doesn't set `state.overview`.
gui/public/app.js line 131: `setBusy()` skips search-input: `if (node.id === "search-input") continue;`
gui/public/app.js lines 397-400: Search input listener calls `renderAccounts()`.
gui/public/app.js line 238: `const registry = state.overview.registry;` - no null check.

Also found in 1 other location(s):
- gui/server.mjs:314 -- If the `registry.json` file exists but is valid JSON lacking an `accounts` property, `registry.accounts` will be `undefined` at line 314, causing `registry.accounts.find(...)` to throw a `TypeError`. The `readRegistry()` function only supplies a default `accounts: []` on `ENOENT`, not when the JSON structure is incomplete.

Comment thread gui/public/app.js
Comment on lines +127 to +134
function setBusy(isBusy) {
state.busy = isBusy;
document.body.classList.toggle("is-busy", isBusy);
for (const node of document.querySelectorAll("button, input")) {
if (node.id === "search-input") continue;
node.disabled = isBusy;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium public/app.js:127

After setBusy(false) completes an action, it forcibly enables all buttons and inputs by setting node.disabled = false, overwriting business-logic disabled states like the active account's Switch button (intentionally disabled in renderAccounts) and removeAllButton when the registry is empty (disabled at line 243). When an action fails, renderAccounts() is not called because no payload.overview exists, so these buttons remain incorrectly enabled. Track the original disabled state before setting isBusy, or scope the reset to only nodes that were actually disabled by setBusy.

function setBusy(isBusy) {
   state.busy = isBusy;
   document.body.classList.toggle("is-busy", isBusy);
-  for (const node of document.querySelectorAll("button, input")) {
-    if (node.id === "search-input") continue;
-    node.disabled = isBusy;
-  }
+  for (const node of document.querySelectorAll("button, input")) {
+    if (node.id === "search-input") continue;
+    if (isBusy) {
+      node.dataset.wasDisabled = node.disabled;
+      node.disabled = true;
+    } else {
+      node.disabled = node.dataset.wasDisabled === "true";
+      delete node.dataset.wasDisabled;
+    }
+  }
 }
🤖 Copy this AI Prompt to have your agent fix this:
In file gui/public/app.js around lines 127-134:

After `setBusy(false)` completes an action, it forcibly enables all buttons and inputs by setting `node.disabled = false`, overwriting business-logic disabled states like the active account's Switch button (intentionally disabled in `renderAccounts`) and `removeAllButton` when the registry is empty (disabled at line 243). When an action fails, `renderAccounts()` is not called because no `payload.overview` exists, so these buttons remain incorrectly enabled. Track the original disabled state before setting `isBusy`, or scope the reset to only nodes that were actually disabled by `setBusy`.

Evidence trail:
gui/public/app.js lines 127-134 (setBusy function sets node.disabled = isBusy for all buttons/inputs); gui/public/app.js line 243 (removeAllButton.disabled business logic); gui/public/app.js line 297 (active account Switch button disabled via `${isActive ? "disabled" : ""}`); gui/public/app.js lines 310-337 (performAction function - catch block at lines 331-333 does not call renderAccounts, finally block at line 335 calls setBusy(false))

@MarlonPassos-git
Copy link
Copy Markdown

I think adding this to the main project is completely unnecessary; perhaps a plugin or another package would suffice. In day-to-day use, the CLI solves all problems 99% of the time. Adding this would only add more weight to the CLI and make maintaining the project more complex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants