test(desktop): extract profile-delete routing decision for real unit tests
profile-delete-respawn.test.ts regexed main.ts source text to check that prepareProfileDeleteRequest returns the torn-down profile name, and that the hermes:api ipcMain handler captures that return value and routes to the primary backend instead of respawning a pool backend for the just- deleted profile. Extract the pure decision logic into profile-delete-routing.ts (no Electron import): - profileNameFromDeleteRequest(request): parses a DELETE /api/profiles/<name> path, moved verbatim (already pure). - decideProfileDeleteAction(profile, deps): the branch decision (noop / teardown-primary / teardown-pool) and the profile name to return, parameterized over isDefaultProfile/isValidProfileName/primaryProfileKey. - resolveRouteProfile(tornDownProfile, profile): the routing ternary from the hermes:api handler. prepareProfileDeleteRequest in main.ts now calls decideProfileDeleteAction for the decision and only performs the async side effects (teardown + writeActiveDesktopProfile) the decision calls for. profile-delete-routing.test.ts replaces profile-delete-respawn.test.ts (which was never wired into test:desktop:platforms), importing the pure functions directly and asserting real return values across every branch -- no readFileSync, no regex-on-source. Wired the new test file into test:desktop:platforms in package.json.fix/verification-admin-route-recovery
ethernet
parent
ec2cb3ab47
commit
1b8f1504b3
|
|
@ -28,7 +28,7 @@ import {
|
|||
systemPreferences
|
||||
} from 'electron'
|
||||
import nodePty from 'node-pty'
|
||||
|
||||
import { decideProfileDeleteAction, profileNameFromDeleteRequest, resolveRouteProfile } from './profile-delete-routing'
|
||||
import { stopBackendChild as stopBackendChildImpl } from './backend-child'
|
||||
import { dashboardFallbackArgs, sourceDeclaresServe } from './backend-command'
|
||||
import { buildDesktopBackendEnv, normalizeHermesHomeRoot } from './backend-env'
|
||||
|
|
@ -6612,59 +6612,37 @@ function stopAllPoolBackends() {
|
|||
}
|
||||
}
|
||||
|
||||
function profileNameFromDeleteRequest(request) {
|
||||
if (!request || String(request.method || 'GET').toUpperCase() !== 'DELETE') {
|
||||
return null
|
||||
}
|
||||
|
||||
const match = String(request.path || '').match(/^\/api\/profiles\/([^/?#]+)(?:[?#].*)?$/)
|
||||
|
||||
if (!match) {
|
||||
return null
|
||||
}
|
||||
|
||||
let raw = ''
|
||||
|
||||
try {
|
||||
raw = decodeURIComponent(match[1])
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
|
||||
const name = raw.trim()
|
||||
|
||||
if (!name) {
|
||||
return null
|
||||
}
|
||||
|
||||
if (name.toLowerCase() === 'default') {
|
||||
return 'default'
|
||||
}
|
||||
|
||||
return name.toLowerCase()
|
||||
}
|
||||
|
||||
// Returns the profile name whose backend was torn down, or null when the
|
||||
// request is not a profile-delete. The caller uses this to skip ensureBackend
|
||||
// for the just-torn-down profile — otherwise ensureBackend respawns a pool
|
||||
// backend whose ensure_hermes_home() recreates the deleted profile directory.
|
||||
//
|
||||
// The routing *decision* (which branch fires, what profile name gets
|
||||
// returned) lives in the pure decideProfileDeleteAction() in
|
||||
// profile-delete-routing.ts; this function only performs the side effects
|
||||
// that decision calls for.
|
||||
async function prepareProfileDeleteRequest(request) {
|
||||
const profile = profileNameFromDeleteRequest(request)
|
||||
const decision = decideProfileDeleteAction(profile, {
|
||||
isDefaultProfile: p => p === 'default',
|
||||
isValidProfileName: p => PROFILE_NAME_RE.test(p),
|
||||
primaryProfileKey
|
||||
})
|
||||
|
||||
if (!profile || profile === 'default' || !PROFILE_NAME_RE.test(profile)) {
|
||||
if (decision.action === 'noop') {
|
||||
return null
|
||||
}
|
||||
|
||||
if (profile === primaryProfileKey()) {
|
||||
if (decision.action === 'teardown-primary') {
|
||||
writeActiveDesktopProfile('default')
|
||||
await teardownPrimaryBackendAndWait()
|
||||
|
||||
return profile
|
||||
return decision.profile
|
||||
}
|
||||
|
||||
await teardownPoolBackendAndWait(profile)
|
||||
await teardownPoolBackendAndWait(decision.profile)
|
||||
|
||||
return profile
|
||||
return decision.profile
|
||||
}
|
||||
|
||||
async function startHermes() {
|
||||
|
|
@ -7849,7 +7827,7 @@ ipcMain.handle('hermes:api', async (_event, request) => {
|
|||
// backend instead of spawning a fresh pool backend. A freshly spawned
|
||||
// backend calls ensure_hermes_home() which recreates the profile directory,
|
||||
// defeating the deletion and leaving a zombie process.
|
||||
const routeProfile = tornDownProfile ? null : profile
|
||||
const routeProfile = resolveRouteProfile(tornDownProfile, profile)
|
||||
const connection = await ensureBackend(routeProfile)
|
||||
const timeoutMs = resolveTimeoutMs(request?.timeoutMs, DEFAULT_FETCH_TIMEOUT_MS)
|
||||
|
||||
|
|
|
|||
|
|
@ -1,62 +0,0 @@
|
|||
'use strict'
|
||||
|
||||
import assert from 'node:assert/strict'
|
||||
import fs from 'node:fs'
|
||||
import path from 'node:path'
|
||||
import test from 'node:test'
|
||||
|
||||
const ELECTRON_DIR = import.meta.dirname
|
||||
|
||||
function readElectronFile(name) {
|
||||
return fs.readFileSync(path.join(ELECTRON_DIR, name), 'utf8').replace(/\r\n/g, '\n')
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// prepareProfileDeleteRequest must return the torn-down profile name so the
|
||||
// caller can skip ensureBackend for that profile (issue #52279).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('prepareProfileDeleteRequest returns the torn-down profile name', () => {
|
||||
const source = readElectronFile('main.ts')
|
||||
|
||||
// Locate the function definition and its closing brace.
|
||||
const fnStart = source.indexOf('async function prepareProfileDeleteRequest(')
|
||||
assert.notEqual(fnStart, -1, 'prepareProfileDeleteRequest function not found')
|
||||
|
||||
// The function must contain "return profile" (pool and primary paths).
|
||||
const fnBody = source.slice(fnStart, fnStart + 800)
|
||||
const returnProfileCount = (fnBody.match(/return profile/g) || []).length
|
||||
assert.ok(
|
||||
returnProfileCount >= 2,
|
||||
`expected at least 2 "return profile" statements (primary + pool paths), found ${returnProfileCount}`
|
||||
)
|
||||
|
||||
// The early-exit guard must return null (not void/undefined).
|
||||
assert.match(fnBody, /return null/, 'early-exit guard should return null, not undefined')
|
||||
})
|
||||
|
||||
test('hermes:api handler routes profile-delete requests to the primary backend', () => {
|
||||
const source = readElectronFile('main.ts')
|
||||
|
||||
// The handler must capture prepareProfileDeleteRequest's return value.
|
||||
assert.match(
|
||||
source,
|
||||
/const tornDownProfile = await prepareProfileDeleteRequest\(request\)/,
|
||||
'handler should capture the return value of prepareProfileDeleteRequest'
|
||||
)
|
||||
|
||||
// The handler must use the return value to skip ensureBackend for the
|
||||
// torn-down profile, routing to the primary (null) instead.
|
||||
assert.match(
|
||||
source,
|
||||
/const routeProfile = tornDownProfile \? null : profile/,
|
||||
'handler should route to primary backend when a profile was just torn down'
|
||||
)
|
||||
|
||||
// ensureBackend must be called with the conditional route profile.
|
||||
assert.match(
|
||||
source,
|
||||
/const connection = await ensureBackend\(routeProfile\)/,
|
||||
'handler should pass routeProfile (not raw profile) to ensureBackend'
|
||||
)
|
||||
})
|
||||
|
|
@ -0,0 +1,83 @@
|
|||
'use strict'
|
||||
|
||||
import assert from 'node:assert/strict'
|
||||
import test from 'node:test'
|
||||
|
||||
import { decideProfileDeleteAction, profileNameFromDeleteRequest, resolveRouteProfile } from './profile-delete-routing'
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// profileNameFromDeleteRequest
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('profileNameFromDeleteRequest parses a DELETE /api/profiles/<name> path', () => {
|
||||
assert.equal(profileNameFromDeleteRequest({ method: 'DELETE', path: '/api/profiles/worker' }), 'worker')
|
||||
})
|
||||
|
||||
test('profileNameFromDeleteRequest lowercases the profile name', () => {
|
||||
assert.equal(profileNameFromDeleteRequest({ method: 'DELETE', path: '/api/profiles/Worker' }), 'worker')
|
||||
})
|
||||
|
||||
test('profileNameFromDeleteRequest returns null for non-DELETE methods', () => {
|
||||
assert.equal(profileNameFromDeleteRequest({ method: 'GET', path: '/api/profiles/worker' }), null)
|
||||
})
|
||||
|
||||
test('profileNameFromDeleteRequest returns null when the path does not match', () => {
|
||||
assert.equal(profileNameFromDeleteRequest({ method: 'DELETE', path: '/api/sessions' }), null)
|
||||
})
|
||||
|
||||
test('profileNameFromDeleteRequest returns null for an empty/whitespace name', () => {
|
||||
assert.equal(profileNameFromDeleteRequest({ method: 'DELETE', path: '/api/profiles/%20' }), null)
|
||||
})
|
||||
|
||||
test('profileNameFromDeleteRequest returns null for an undecodable path segment', () => {
|
||||
assert.equal(profileNameFromDeleteRequest({ method: 'DELETE', path: '/api/profiles/%E0%A4%A' }), null)
|
||||
})
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// decideProfileDeleteAction
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const deps = {
|
||||
isDefaultProfile: p => p === 'default',
|
||||
isValidProfileName: p => /^[a-z0-9][a-z0-9_-]{0,63}$/.test(p),
|
||||
primaryProfileKey: () => 'primary-profile'
|
||||
}
|
||||
|
||||
test('decideProfileDeleteAction is a noop for the default profile', () => {
|
||||
assert.deepEqual(decideProfileDeleteAction('default', deps), { action: 'noop', profile: null })
|
||||
})
|
||||
|
||||
test('decideProfileDeleteAction is a noop for null (no profile parsed)', () => {
|
||||
assert.deepEqual(decideProfileDeleteAction(null, deps), { action: 'noop', profile: null })
|
||||
})
|
||||
|
||||
test('decideProfileDeleteAction is a noop for an invalid profile name', () => {
|
||||
assert.deepEqual(decideProfileDeleteAction('Not Valid!', deps), { action: 'noop', profile: null })
|
||||
})
|
||||
|
||||
test('decideProfileDeleteAction tears down the primary backend for the primary profile', () => {
|
||||
assert.deepEqual(decideProfileDeleteAction('primary-profile', deps), {
|
||||
action: 'teardown-primary',
|
||||
profile: 'primary-profile'
|
||||
})
|
||||
})
|
||||
|
||||
test('decideProfileDeleteAction tears down the pool backend for any other valid profile', () => {
|
||||
assert.deepEqual(decideProfileDeleteAction('worker', deps), { action: 'teardown-pool', profile: 'worker' })
|
||||
})
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// resolveRouteProfile
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('resolveRouteProfile routes to the primary backend (null) when a profile was torn down', () => {
|
||||
assert.equal(resolveRouteProfile('worker', 'other-profile'), null)
|
||||
})
|
||||
|
||||
test('resolveRouteProfile passes the requested profile through when nothing was torn down', () => {
|
||||
assert.equal(resolveRouteProfile(null, 'other-profile'), 'other-profile')
|
||||
})
|
||||
|
||||
test('resolveRouteProfile passes through undefined when nothing was torn down and no profile was requested', () => {
|
||||
assert.equal(resolveRouteProfile(null, undefined), undefined)
|
||||
})
|
||||
|
|
@ -0,0 +1,97 @@
|
|||
'use strict'
|
||||
|
||||
// Profile-delete routing logic for the `hermes:api` IPC handler.
|
||||
//
|
||||
// When the renderer issues DELETE /api/profiles/<name>, the handler must
|
||||
// tear down that profile's backend (primary window backend or pool backend)
|
||||
// and then route the *next* request away from the just-deleted profile's
|
||||
// pool backend -- spawning a fresh one would call ensure_hermes_home() and
|
||||
// recreate the profile directory the delete just removed, leaving a zombie
|
||||
// process behind (issue #52279).
|
||||
//
|
||||
// These helpers are pure so they can be unit-tested without Electron.
|
||||
|
||||
/**
|
||||
* Parse a `hermes:api` request into the profile name a DELETE targets, or
|
||||
* null when the request is not a profile-delete at all (wrong method, wrong
|
||||
* path, empty/invalid name).
|
||||
*/
|
||||
export function profileNameFromDeleteRequest(request) {
|
||||
if (!request || String(request.method || 'GET').toUpperCase() !== 'DELETE') {
|
||||
return null
|
||||
}
|
||||
|
||||
const match = String(request.path || '').match(/^\/api\/profiles\/([^/?#]+)(?:[?#].*)?$/)
|
||||
|
||||
if (!match) {
|
||||
return null
|
||||
}
|
||||
|
||||
let raw = ''
|
||||
|
||||
try {
|
||||
raw = decodeURIComponent(match[1])
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
|
||||
const name = raw.trim()
|
||||
|
||||
if (!name) {
|
||||
return null
|
||||
}
|
||||
|
||||
if (name.toLowerCase() === 'default') {
|
||||
return 'default'
|
||||
}
|
||||
|
||||
return name.toLowerCase()
|
||||
}
|
||||
|
||||
export type ProfileDeleteAction = 'noop' | 'teardown-primary' | 'teardown-pool'
|
||||
|
||||
export interface ProfileDeleteDecision {
|
||||
action: ProfileDeleteAction
|
||||
profile: string | null
|
||||
}
|
||||
|
||||
export interface ProfileDeleteDecisionDeps {
|
||||
isDefaultProfile: (profile: string) => boolean
|
||||
isValidProfileName: (profile: string) => boolean
|
||||
primaryProfileKey: () => string
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure decision logic for prepareProfileDeleteRequest: given the parsed
|
||||
* profile name (or null), decide which side-effecting branch the caller
|
||||
* should take and what profile name it should ultimately report as
|
||||
* torn-down. No I/O, no async -- the caller performs the actual teardown
|
||||
* based on `action`.
|
||||
*/
|
||||
export function decideProfileDeleteAction(
|
||||
profile: string | null,
|
||||
deps: ProfileDeleteDecisionDeps
|
||||
): ProfileDeleteDecision {
|
||||
if (!profile || deps.isDefaultProfile(profile) || !deps.isValidProfileName(profile)) {
|
||||
return { action: 'noop', profile: null }
|
||||
}
|
||||
|
||||
if (profile === deps.primaryProfileKey()) {
|
||||
return { action: 'teardown-primary', profile }
|
||||
}
|
||||
|
||||
return { action: 'teardown-pool', profile }
|
||||
}
|
||||
|
||||
/**
|
||||
* Route the next `hermes:api` request away from the primary/window backend
|
||||
* whenever a profile was just torn down -- otherwise ensureBackend would
|
||||
* spawn a fresh pool backend for the deleted profile, whose
|
||||
* ensure_hermes_home() recreates the directory the delete just removed.
|
||||
*/
|
||||
export function resolveRouteProfile(
|
||||
tornDownProfile: string | null,
|
||||
profile: string | undefined
|
||||
): string | null | undefined {
|
||||
return tornDownProfile ? null : profile
|
||||
}
|
||||
|
|
@ -38,7 +38,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.ts electron/hardening.test.ts electron/backend-env.test.ts electron/backend-probes.test.ts electron/backend-ready.test.ts electron/bootstrap-runner.test.ts electron/connection-config.test.ts electron/dashboard-token.test.ts electron/gateway-ws-probe.test.ts electron/oauth-net-request.test.ts electron/desktop-uninstall.test.ts electron/session-windows.test.ts electron/link-title-window.test.ts electron/workspace-cwd.test.ts electron/fs-read-dir.test.ts electron/git-root.test.ts electron/git-worktree-ops.test.ts electron/windows-child-options.test.ts electron/update-remote.test.ts electron/update-count.test.ts electron/update-rebuild.test.ts electron/update-marker.test.ts electron/update-relaunch.test.ts electron/windows-user-env.test.ts electron/wsl-clipboard-image.test.ts electron/titlebar-overlay-width.test.ts electron/window-state.test.ts electron/zoom.test.ts electron/windows-hermes-path.test.ts electron/oauth-session-request.test.ts",
|
||||
"test:desktop:platforms": "node --test electron/bootstrap-platform.test.ts electron/hardening.test.ts electron/backend-env.test.ts electron/backend-probes.test.ts electron/backend-ready.test.ts electron/bootstrap-runner.test.ts electron/connection-config.test.ts electron/dashboard-token.test.ts electron/gateway-ws-probe.test.ts electron/oauth-net-request.test.ts electron/desktop-uninstall.test.ts electron/session-windows.test.ts electron/profile-delete-routing.test.ts electron/link-title-window.test.ts electron/workspace-cwd.test.ts electron/fs-read-dir.test.ts electron/git-root.test.ts electron/git-worktree-ops.test.ts electron/windows-child-options.test.ts electron/update-remote.test.ts electron/update-count.test.ts electron/update-rebuild.test.ts electron/update-marker.test.ts electron/update-relaunch.test.ts electron/windows-user-env.test.ts electron/wsl-clipboard-image.test.ts electron/titlebar-overlay-width.test.ts electron/window-state.test.ts electron/zoom.test.ts electron/windows-hermes-path.test.ts electron/oauth-session-request.test.ts",
|
||||
"typecheck": "tsc -p . --noEmit && tsc -p tsconfig.electron.json --noEmit",
|
||||
"lint": "eslint src/ electron/",
|
||||
"lint:fix": "eslint src/ electron/ --fix",
|
||||
|
|
|
|||
Loading…
Reference in New Issue