From b29bb6ef9d002f78f1c801d4b390a93a6705aef1 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Tue, 30 Jun 2026 02:05:07 -0500 Subject: [PATCH] refactor(desktop): assert git-ipc surface by invariant, drop channel snapshot --- apps/desktop/electron/git-ipc.cjs | 26 +-------------------- apps/desktop/electron/git-ipc.test.cjs | 32 +++++++++++++------------- 2 files changed, 17 insertions(+), 41 deletions(-) diff --git a/apps/desktop/electron/git-ipc.cjs b/apps/desktop/electron/git-ipc.cjs index 3f765490cfc..6fe044bc5fd 100644 --- a/apps/desktop/electron/git-ipc.cjs +++ b/apps/desktop/electron/git-ipc.cjs @@ -18,30 +18,6 @@ const { } = 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. @@ -117,4 +93,4 @@ function registerGitIpc({ ipcMain, resolveGitBinary, resolveGhBinary }) { }) } -module.exports = { GIT_IPC_CHANNELS, registerGitIpc } +module.exports = { registerGitIpc } diff --git a/apps/desktop/electron/git-ipc.test.cjs b/apps/desktop/electron/git-ipc.test.cjs index 8157698f881..e1264e67475 100644 --- a/apps/desktop/electron/git-ipc.test.cjs +++ b/apps/desktop/electron/git-ipc.test.cjs @@ -3,7 +3,7 @@ const assert = require('node:assert/strict') const test = require('node:test') -const { GIT_IPC_CHANNELS, registerGitIpc } = require('./git-ipc.cjs') +const { registerGitIpc } = require('./git-ipc.cjs') function fakeIpcMain() { const handlers = new Map() @@ -11,33 +11,34 @@ function fakeIpcMain() { return { handlers, handle(channel, handler) { + assert.ok(!handlers.has(channel), `duplicate registration for ${channel}`) handlers.set(channel, handler) } } } -test('registerGitIpc wires every advertised git channel exactly once', () => { +test('registerGitIpc wires only hermes:git:* channels, each to a handler fn', () => { const ipcMain = fakeIpcMain() - registerGitIpc({ - ipcMain, - resolveGitBinary: () => 'git', - resolveGhBinary: () => 'gh' - }) + registerGitIpc({ ipcMain, resolveGitBinary: () => 'git', resolveGhBinary: () => 'gh' }) - assert.deepEqual([...ipcMain.handlers.keys()].sort(), [...GIT_IPC_CHANNELS].sort()) + assert.ok(ipcMain.handlers.size >= 19, `expected the full git surface, got ${ipcMain.handlers.size}`) - for (const channel of GIT_IPC_CHANNELS) { - assert.equal(typeof ipcMain.handlers.get(channel), 'function', `${channel} should register a handler`) + for (const [channel, handler] of ipcMain.handlers) { + assert.match(channel, /^hermes:git:/, `${channel} is not a git channel`) + assert.equal(typeof handler, 'function', `${channel} should register a handler`) + } + + // Spot-check the load-bearing channels across the worktree / review / scan groups. + for (const channel of ['hermes:git:worktreeList', 'hermes:git:review:commit', 'hermes:git:scanRepos']) { + assert.ok(ipcMain.handlers.has(channel), `missing ${channel}`) } }) -test('registerGitIpc delegates worktreeList to the git-worktree-ops module', async () => { +test('handlers thread the injected resolver into the ops layer', 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: () => { @@ -48,11 +49,10 @@ test('registerGitIpc delegates worktreeList to the git-worktree-ops module', asy 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. + // ops layer does with a non-repo path is irrelevant to the wiring. try { - await worktreeList({}, '/definitely/not/a/repo') + await ipcMain.handlers.get('hermes:git:worktreeList')({}, '/definitely/not/a/repo') } catch { // ops layer may reject on a bad path — not what this test asserts. }