From 025c8f0604d08751910d676e68728c20dacc2e9d Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Tue, 30 Jun 2026 01:42:33 -0500 Subject: [PATCH] refactor(desktop): extract git IPC handlers from main.cjs into git-ipc.cjs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit electron/main.cjs is the worst god file in the desktop app (~7.6k lines, 93 IPC handlers across unrelated domains). Begin peeling cohesive handler clusters into sibling modules — the established main.cjs pattern. First cluster: the 19 git/worktree/review IPC handlers (all thin delegators to the existing git-*-ops modules) move into a new electron/git-ipc.cjs exposing registerGitIpc({ ipcMain, resolveGitBinary, resolveGhBinary }). The git/gh binary resolvers stay in main.cjs (Windows PATH discovery) and are injected, so the new module is pure. Channel names are unchanged, so preload/renderer are unaffected. Adds electron/git-ipc.test.cjs (wired into test:desktop:platforms) asserting the full channel surface and resolver delegation. main.cjs: 7,617 -> 7,530. --- apps/desktop/electron/git-ipc.cjs | 120 +++++++++++++++++++++++++ apps/desktop/electron/git-ipc.test.cjs | 61 +++++++++++++ apps/desktop/electron/main.cjs | 99 ++------------------ apps/desktop/package.json | 2 +- 4 files changed, 188 insertions(+), 94 deletions(-) create mode 100644 apps/desktop/electron/git-ipc.cjs create mode 100644 apps/desktop/electron/git-ipc.test.cjs diff --git a/apps/desktop/electron/git-ipc.cjs b/apps/desktop/electron/git-ipc.cjs new file mode 100644 index 00000000000..3f765490cfc --- /dev/null +++ b/apps/desktop/electron/git-ipc.cjs @@ -0,0 +1,120 @@ +'use strict' + +const { scanGitRepos } = require('./git-repo-scan.cjs') +const { + fileDiffVsHead, + repoStatus, + reviewCommit, + reviewCommitContext, + reviewCreatePr, + reviewDiff, + reviewList, + reviewPush, + reviewRevParse, + reviewRevert, + reviewShipInfo, + reviewStage, + reviewUnstage +} = require('./git-review-ops.cjs') +const { addWorktree, listBranches, listWorktrees, removeWorktree, switchBranch } = require('./git-worktree-ops.cjs') + +// Every IPC channel this registrar owns. Exported so a test can assert the +// renderer-facing git surface stays complete after refactors. +const GIT_IPC_CHANNELS = [ + 'hermes:git:worktreeList', + 'hermes:git:worktreeAdd', + 'hermes:git:worktreeRemove', + 'hermes:git:branchSwitch', + 'hermes:git:branchList', + 'hermes:git:repoStatus', + 'hermes:git:review:list', + 'hermes:git:review:diff', + 'hermes:git:fileDiff', + 'hermes:git:review:stage', + 'hermes:git:review:unstage', + 'hermes:git:review:revert', + 'hermes:git:review:revParse', + 'hermes:git:review:commit', + 'hermes:git:review:commitContext', + 'hermes:git:review:push', + 'hermes:git:review:shipInfo', + 'hermes:git:review:createPr', + 'hermes:git:scanRepos' +] + +// Register the git/worktree/review IPC handlers. Thin delegators to the +// git-*-ops sibling modules; the git/gh binary resolution lives in the main +// process (Windows PATH discovery) and is injected so this module stays pure. +function registerGitIpc({ ipcMain, resolveGitBinary, resolveGhBinary }) { + // Git-driven worktree management ("Start work" flow). Errors surface to the + // renderer as rejected promises so it can toast a friendly message. + ipcMain.handle('hermes:git:worktreeList', async (_event, repoPath) => listWorktrees(repoPath, resolveGitBinary())) + + ipcMain.handle('hermes:git:worktreeAdd', async (_event, repoPath, options) => + addWorktree(repoPath, options || {}, resolveGitBinary()) + ) + + ipcMain.handle('hermes:git:worktreeRemove', async (_event, repoPath, worktreePath, options) => + removeWorktree(repoPath, worktreePath, options || {}, resolveGitBinary()) + ) + + ipcMain.handle('hermes:git:branchSwitch', async (_event, repoPath, branch) => + switchBranch(repoPath, branch, resolveGitBinary()) + ) + + ipcMain.handle('hermes:git:branchList', async (_event, repoPath) => listBranches(repoPath, resolveGitBinary())) + + // Compact repo status (branch, ahead/behind, change counts + files) for the + // composer coding rail. Returns null on a non-repo / remote backend so the rail + // hides cleanly rather than erroring. + ipcMain.handle('hermes:git:repoStatus', async (_event, repoPath) => repoStatus(repoPath, resolveGitBinary())) + + // Codex-style review pane: list changed files for a scope, fetch one file's + // unified diff, and stage / unstage / revert. Reads return empty on failure; + // mutations reject so the renderer can toast. + ipcMain.handle('hermes:git:review:list', async (_event, repoPath, scope, baseRef) => + reviewList(repoPath, scope, baseRef, resolveGitBinary()) + ) + ipcMain.handle('hermes:git:review:diff', async (_event, repoPath, filePath, scope, baseRef, staged) => + reviewDiff(repoPath, filePath, scope, baseRef, staged, resolveGitBinary()) + ) + // Working-tree-vs-HEAD diff for one file (the preview's "show the diff" view). + ipcMain.handle('hermes:git:fileDiff', async (_event, repoPath, filePath) => + fileDiffVsHead(repoPath, filePath, resolveGitBinary()) + ) + ipcMain.handle('hermes:git:review:stage', async (_event, repoPath, filePath) => + reviewStage(repoPath, filePath ?? null, resolveGitBinary()) + ) + ipcMain.handle('hermes:git:review:unstage', async (_event, repoPath, filePath) => + reviewUnstage(repoPath, filePath ?? null, resolveGitBinary()) + ) + ipcMain.handle('hermes:git:review:revert', async (_event, repoPath, filePath) => + reviewRevert(repoPath, filePath ?? null, resolveGitBinary()) + ) + ipcMain.handle('hermes:git:review:revParse', async (_event, repoPath, ref) => + reviewRevParse(repoPath, ref, resolveGitBinary()) + ) + ipcMain.handle('hermes:git:review:commit', async (_event, repoPath, message, push) => + reviewCommit(repoPath, message, Boolean(push), resolveGitBinary()) + ) + ipcMain.handle('hermes:git:review:commitContext', async (_event, repoPath) => + reviewCommitContext(repoPath, resolveGitBinary()) + ) + ipcMain.handle('hermes:git:review:push', async (_event, repoPath) => reviewPush(repoPath, resolveGitBinary())) + ipcMain.handle('hermes:git:review:shipInfo', async (_event, repoPath) => reviewShipInfo(repoPath, resolveGhBinary())) + ipcMain.handle('hermes:git:review:createPr', async (_event, repoPath) => + reviewCreatePr(repoPath, resolveGitBinary(), resolveGhBinary()) + ) + + // Repo-first project discovery: scan bounded roots for git repos (pure fs walk, + // no native addon). Never throws to the renderer — failures yield an empty list. + ipcMain.handle('hermes:git:scanRepos', async (_event, roots, options) => { + try { + return await scanGitRepos(roots || [], options || {}) + } catch { + return [] + } + }) +} + +module.exports = { GIT_IPC_CHANNELS, registerGitIpc } diff --git a/apps/desktop/electron/git-ipc.test.cjs b/apps/desktop/electron/git-ipc.test.cjs new file mode 100644 index 00000000000..8157698f881 --- /dev/null +++ b/apps/desktop/electron/git-ipc.test.cjs @@ -0,0 +1,61 @@ +'use strict' + +const assert = require('node:assert/strict') +const test = require('node:test') + +const { GIT_IPC_CHANNELS, registerGitIpc } = require('./git-ipc.cjs') + +function fakeIpcMain() { + const handlers = new Map() + + return { + handlers, + handle(channel, handler) { + handlers.set(channel, handler) + } + } +} + +test('registerGitIpc wires every advertised git channel exactly once', () => { + const ipcMain = fakeIpcMain() + + registerGitIpc({ + ipcMain, + resolveGitBinary: () => 'git', + resolveGhBinary: () => 'gh' + }) + + assert.deepEqual([...ipcMain.handlers.keys()].sort(), [...GIT_IPC_CHANNELS].sort()) + + for (const channel of GIT_IPC_CHANNELS) { + assert.equal(typeof ipcMain.handlers.get(channel), 'function', `${channel} should register a handler`) + } +}) + +test('registerGitIpc delegates worktreeList to the git-worktree-ops module', async () => { + const ipcMain = fakeIpcMain() + const calls = [] + + // Stub the git binary resolver so we can confirm the handler threads it into + // the ops layer without shelling out to a real git. + registerGitIpc({ + ipcMain, + resolveGitBinary: () => { + calls.push('git') + + return 'git' + }, + resolveGhBinary: () => 'gh' + }) + + const worktreeList = ipcMain.handlers.get('hermes:git:worktreeList') + // The resolver is consulted synchronously to build the ops call; whatever the + // ops layer then does with a non-repo path is irrelevant to the wiring. + try { + await worktreeList({}, '/definitely/not/a/repo') + } catch { + // ops layer may reject on a bad path — not what this test asserts. + } + + assert.deepEqual(calls, ['git']) +}) diff --git a/apps/desktop/electron/main.cjs b/apps/desktop/electron/main.cjs index e800034500b..e5dd11120e1 100644 --- a/apps/desktop/electron/main.cjs +++ b/apps/desktop/electron/main.cjs @@ -58,23 +58,7 @@ const { buildRelaunchScript } = require('./update-relaunch.cjs') const { gitRootForIpc } = require('./git-root.cjs') -const { addWorktree, listBranches, listWorktrees, removeWorktree, switchBranch } = require('./git-worktree-ops.cjs') -const { - fileDiffVsHead, - repoStatus, - reviewCommit, - reviewCommitContext, - reviewCreatePr, - reviewDiff, - reviewList, - reviewPush, - reviewRevParse, - reviewRevert, - reviewShipInfo, - reviewStage, - reviewUnstage -} = require('./git-review-ops.cjs') -const { scanGitRepos } = require('./git-repo-scan.cjs') +const { registerGitIpc } = require('./git-ipc.cjs') const { OFFICIAL_REPO_HTTPS_URL, isOfficialSshRemote } = require('./update-remote.cjs') const { resolveBehindCount, shouldCountCommits } = require('./update-count.cjs') const { runRebuildWithRetry } = require('./update-rebuild.cjs') @@ -1361,10 +1345,7 @@ function backendSupportsServe(backend) { let supported = null if (backend.root) { try { - const src = fs.readFileSync( - path.join(backend.root, 'hermes_cli', 'subcommands', 'dashboard.py'), - 'utf8' - ) + const src = fs.readFileSync(path.join(backend.root, 'hermes_cli', 'subcommands', 'dashboard.py'), 'utf8') supported = sourceDeclaresServe(src) } catch { supported = null // source unreadable — fall through to the probe @@ -2292,9 +2273,7 @@ async function handOffWindowsBootstrapRecovery(reason) { // --repair (full venv recreate) and drove reinstall loops. The venv interpreter // and the bootstrap-complete marker are present earlier and are better signals. const haveRealInstall = - fileExists(venvPython) || - fileExists(venvHermes) || - fileExists(path.join(updateRoot, '.hermes-bootstrap-complete')) + fileExists(venvPython) || fileExists(venvHermes) || fileExists(path.join(updateRoot, '.hermes-bootstrap-complete')) const updaterArgs = haveRealInstall ? ['--update', '--branch', branch] : ['--repair', '--branch', branch] await releaseBackendLockForUpdate(updateRoot) @@ -7008,75 +6987,9 @@ ipcMain.handle('hermes:fs:trash', async (_event, targetPath) => { return true }) -// Git-driven worktree management ("Start work" flow). Errors surface to the -// renderer as rejected promises so it can toast a friendly message. -ipcMain.handle('hermes:git:worktreeList', async (_event, repoPath) => listWorktrees(repoPath, resolveGitBinary())) - -ipcMain.handle('hermes:git:worktreeAdd', async (_event, repoPath, options) => - addWorktree(repoPath, options || {}, resolveGitBinary()) -) - -ipcMain.handle('hermes:git:worktreeRemove', async (_event, repoPath, worktreePath, options) => - removeWorktree(repoPath, worktreePath, options || {}, resolveGitBinary()) -) - -ipcMain.handle('hermes:git:branchSwitch', async (_event, repoPath, branch) => - switchBranch(repoPath, branch, resolveGitBinary()) -) - -ipcMain.handle('hermes:git:branchList', async (_event, repoPath) => listBranches(repoPath, resolveGitBinary())) - -// Compact repo status (branch, ahead/behind, change counts + files) for the -// composer coding rail. Returns null on a non-repo / remote backend so the rail -// hides cleanly rather than erroring. -ipcMain.handle('hermes:git:repoStatus', async (_event, repoPath) => repoStatus(repoPath, resolveGitBinary())) - -// Codex-style review pane: list changed files for a scope, fetch one file's -// unified diff, and stage / unstage / revert. Reads return empty on failure; -// mutations reject so the renderer can toast. -ipcMain.handle('hermes:git:review:list', async (_event, repoPath, scope, baseRef) => - reviewList(repoPath, scope, baseRef, resolveGitBinary()) -) -ipcMain.handle('hermes:git:review:diff', async (_event, repoPath, filePath, scope, baseRef, staged) => - reviewDiff(repoPath, filePath, scope, baseRef, staged, resolveGitBinary()) -) -// Working-tree-vs-HEAD diff for one file (the preview's "show the diff" view). -ipcMain.handle('hermes:git:fileDiff', async (_event, repoPath, filePath) => - fileDiffVsHead(repoPath, filePath, resolveGitBinary()) -) -ipcMain.handle('hermes:git:review:stage', async (_event, repoPath, filePath) => - reviewStage(repoPath, filePath ?? null, resolveGitBinary()) -) -ipcMain.handle('hermes:git:review:unstage', async (_event, repoPath, filePath) => - reviewUnstage(repoPath, filePath ?? null, resolveGitBinary()) -) -ipcMain.handle('hermes:git:review:revert', async (_event, repoPath, filePath) => - reviewRevert(repoPath, filePath ?? null, resolveGitBinary()) -) -ipcMain.handle('hermes:git:review:revParse', async (_event, repoPath, ref) => - reviewRevParse(repoPath, ref, resolveGitBinary()) -) -ipcMain.handle('hermes:git:review:commit', async (_event, repoPath, message, push) => - reviewCommit(repoPath, message, Boolean(push), resolveGitBinary()) -) -ipcMain.handle('hermes:git:review:commitContext', async (_event, repoPath) => - reviewCommitContext(repoPath, resolveGitBinary()) -) -ipcMain.handle('hermes:git:review:push', async (_event, repoPath) => reviewPush(repoPath, resolveGitBinary())) -ipcMain.handle('hermes:git:review:shipInfo', async (_event, repoPath) => reviewShipInfo(repoPath, resolveGhBinary())) -ipcMain.handle('hermes:git:review:createPr', async (_event, repoPath) => - reviewCreatePr(repoPath, resolveGitBinary(), resolveGhBinary()) -) - -// Repo-first project discovery: scan bounded roots for git repos (pure fs walk, -// no native addon). Never throws to the renderer — failures yield an empty list. -ipcMain.handle('hermes:git:scanRepos', async (_event, roots, options) => { - try { - return await scanGitRepos(roots || [], options || {}) - } catch { - return [] - } -}) +// Git/worktree/review IPC lives in git-ipc.cjs; the git + gh binary resolvers +// stay here (Windows PATH discovery) and are injected into the registrar. +registerGitIpc({ ipcMain, resolveGitBinary, resolveGhBinary }) ipcMain.handle('hermes:terminal:start', async (event, payload = {}) => { if (!nodePty) { diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 4bf4eaade96..1e54f681288 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -37,7 +37,7 @@ "test:desktop:nsis": "node scripts/test-desktop.mjs nsis", "test:desktop:existing": "node scripts/test-desktop.mjs existing", "test:desktop:fresh": "node scripts/test-desktop.mjs fresh", - "test:desktop:platforms": "node --test electron/bootstrap-platform.test.cjs electron/hardening.test.cjs electron/backend-env.test.cjs electron/backend-probes.test.cjs electron/backend-ready.test.cjs electron/bootstrap-runner.test.cjs electron/connection-config.test.cjs electron/dashboard-token.test.cjs electron/gateway-ws-probe.test.cjs electron/oauth-net-request.test.cjs electron/desktop-uninstall.test.cjs electron/session-windows.test.cjs electron/link-title-window.test.cjs electron/workspace-cwd.test.cjs electron/fs-read-dir.test.cjs electron/git-root.test.cjs electron/git-worktree-ops.test.cjs electron/windows-child-process.test.cjs electron/update-remote.test.cjs electron/update-count.test.cjs electron/update-rebuild.test.cjs electron/update-marker.test.cjs electron/update-relaunch.test.cjs electron/windows-user-env.test.cjs electron/wsl-clipboard-image.test.cjs electron/titlebar-overlay-width.test.cjs electron/window-state.test.cjs electron/windows-hermes-resolution.test.cjs", + "test:desktop:platforms": "node --test electron/bootstrap-platform.test.cjs electron/hardening.test.cjs electron/backend-env.test.cjs electron/backend-probes.test.cjs electron/backend-ready.test.cjs electron/bootstrap-runner.test.cjs electron/connection-config.test.cjs electron/dashboard-token.test.cjs electron/gateway-ws-probe.test.cjs electron/oauth-net-request.test.cjs electron/desktop-uninstall.test.cjs electron/session-windows.test.cjs electron/link-title-window.test.cjs electron/workspace-cwd.test.cjs electron/fs-read-dir.test.cjs electron/git-root.test.cjs electron/git-ipc.test.cjs electron/git-worktree-ops.test.cjs electron/windows-child-process.test.cjs electron/update-remote.test.cjs electron/update-count.test.cjs electron/update-rebuild.test.cjs electron/update-marker.test.cjs electron/update-relaunch.test.cjs electron/windows-user-env.test.cjs electron/wsl-clipboard-image.test.cjs electron/titlebar-overlay-width.test.cjs electron/window-state.test.cjs electron/windows-hermes-resolution.test.cjs", "typecheck": "tsc -p . --noEmit", "lint": "eslint src/ electron/", "lint:fix": "eslint src/ electron/ --fix",