fix(desktop): clear PrimaryProfilePin on current-owner exit/error and make one launch-profile decision per start (#108417)
Salvages the pin-clear half of PR #109059 (salch-cred) and goes further: - The current child's 'error' and 'exit' handlers now clear the pin after the clearForCurrentProcess guard, so an unexpected exit cannot leave routing pinned to a dead backend while the respawn reads the new --profile. A stale exit (older child) still returns before the clear and never touches a newer primary's pin. - The connection IIFE's catch clears the pin behind the attempt guard: a failed startup releases its routing identity; a superseded attempt's failure never clears the newer attempt's pin. - resolveLaunchProfile(readPreference) makes the launch decision ONE read per attempt: startHermes() now derives routingProfile (pin, setActiveGatewayProfile, remote resolve, child env identity) and argvProfile (the --profile flag) from the same decision, instead of pinning primaryProfileKey() up top and re-reading readActiveDesktopProfile() deep inside the IIFE — the split that let a mid-startup hermes:profile:remember produce 'routing alpha, --profile beta'. Unset preference keeps the legacy flag-less launch. - #108118's invariant is preserved: a live primary still answers primaryProfileKey() from the pin while a preference change lands, so no duplicate backend spawns mid-life.
This commit is contained in:
@@ -394,7 +394,7 @@ import {
|
||||
runPrimaryBackendStartup
|
||||
} from './primary-backend-startup'
|
||||
import { rehomePrimaryConnection } from './primary-connection-rehome'
|
||||
import { PrimaryProfilePin } from './primary-profile-pin'
|
||||
import { PrimaryProfilePin, resolveLaunchProfile } from './primary-profile-pin'
|
||||
import { applyDesktopIdentity, PRODUCT_IDENTITY } from './product-identity'
|
||||
import {
|
||||
assertLocalProfileCanStart,
|
||||
@@ -12384,7 +12384,15 @@ async function runHermesStart({ supervisorRecovery = false }: { supervisorRecove
|
||||
migrateActiveProfileIfMissing()
|
||||
|
||||
const connectionAttempt = backendConnectionState.startAttempt()
|
||||
const primaryProfile = primaryProfileKey()
|
||||
// ONE launch-profile decision for this attempt (#108417): routing pin,
|
||||
// --profile argv, and the child env all derive from the same read, so a
|
||||
// hermes:profile:remember landing mid-startup becomes the NEXT boot's
|
||||
// preference instead of splitting routing identity from the launch
|
||||
// argument. (The pin below still honors a live primary — but a primary
|
||||
// being live means startHermes never got here.)
|
||||
const { argvProfile: activeProfile, routingProfile: primaryProfile } = resolveLaunchProfile(
|
||||
readActiveDesktopProfile
|
||||
)
|
||||
// Pin the routing table to the profile this primary actually boots as; a
|
||||
// later hermes:profile:remember must not retarget requests mid-life.
|
||||
primaryProfilePin.pin(primaryProfile)
|
||||
@@ -12449,9 +12457,8 @@ async function runHermesStart({ supervisorRecovery = false }: { supervisorRecove
|
||||
// deterministic (it wins over the sticky ~/.hermes/active_profile file) and
|
||||
// resolves HERMES_HOME the same way `hermes -p <name>` does on the CLI. An
|
||||
// unset preference keeps the legacy launch so existing installs are
|
||||
// unaffected.
|
||||
const activeProfile = readActiveDesktopProfile()
|
||||
|
||||
// unaffected. `activeProfile` is the SAME decision that pinned routing
|
||||
// above — never re-read here (#108417).
|
||||
if (activeProfile) {
|
||||
backendArgs.unshift('--profile', activeProfile)
|
||||
}
|
||||
@@ -12540,7 +12547,7 @@ async function runHermesStart({ supervisorRecovery = false }: { supervisorRecove
|
||||
await advanceBootProgress('backend.spawn', `Starting Hermes backend via ${backend.label}`, 84)
|
||||
rememberLog(`Starting Hermes backend via ${backend.label}`)
|
||||
|
||||
const profile = primaryProfileKey()
|
||||
const profile = primaryProfile
|
||||
const parentStartMarker = await desktopParentStartMarker()
|
||||
const backendNonce = crypto.randomBytes(16).toString('hex')
|
||||
const parentIdentityEnv = parentWatchdogEnv(process.pid, parentStartMarker, backendNonce)
|
||||
@@ -12649,6 +12656,11 @@ async function runHermesStart({ supervisorRecovery = false }: { supervisorRecove
|
||||
return
|
||||
}
|
||||
|
||||
// The CURRENT owner failed to start: its routing identity must not
|
||||
// outlive it. A newer attempt already re-pins on its own decision
|
||||
// (#108417), and the stale branch above never reaches this clear.
|
||||
primaryProfilePin.clear()
|
||||
|
||||
rememberLog(`Hermes backend failed to start: ${error.message}`)
|
||||
updateBootProgress(
|
||||
{
|
||||
@@ -12679,6 +12691,13 @@ async function runHermesStart({ supervisorRecovery = false }: { supervisorRecove
|
||||
|
||||
rememberLog(formatBackendExitLine('Hermes backend exited', code, signal, primaryOutputTail))
|
||||
|
||||
// The current primary child is gone; release its routing pin so the
|
||||
// next startHermes() re-reads active-profile.json instead of re-pinning
|
||||
// the dead child's profile (#108417). Supervisor respawns go through
|
||||
// startHermes, which makes a fresh decision — a respawn cannot inherit
|
||||
// a pin from a process that no longer exists.
|
||||
primaryProfilePin.clear()
|
||||
|
||||
if (!scheduleUnexpectedPrimaryRecovery({ code, signal, ready: backendReady })) {
|
||||
sendBackendExit({ code, signal })
|
||||
}
|
||||
@@ -12788,6 +12807,12 @@ async function runHermesStart({ supervisorRecovery = false }: { supervisorRecove
|
||||
throw error
|
||||
}
|
||||
|
||||
// The startup attempt this pin belongs to is being torn down: release its
|
||||
// routing identity so a later start re-reads the preference. The
|
||||
// attempt guard above means a superseded attempt's failure never clears
|
||||
// a newer attempt's pin (#108417).
|
||||
primaryProfilePin.clear()
|
||||
|
||||
await backendConnectionState.stopProcess(localBackendLifecycle.stop)
|
||||
|
||||
if (error instanceof FirstRunSetupResetError) {
|
||||
|
||||
@@ -2,7 +2,7 @@ import assert from 'node:assert/strict'
|
||||
|
||||
import { test } from 'vitest'
|
||||
|
||||
import { PrimaryProfilePin } from './primary-profile-pin'
|
||||
import { PrimaryProfilePin, resolveLaunchProfile } from './primary-profile-pin'
|
||||
|
||||
test('a live primary keeps answering for its booted profile after the preference moves', () => {
|
||||
const pin = new PrimaryProfilePin()
|
||||
@@ -38,3 +38,28 @@ test('teardown releases the pin so the next start follows the preference', () =>
|
||||
'claude'
|
||||
)
|
||||
})
|
||||
|
||||
// #108417: one authoritative launch-profile decision per startup attempt.
|
||||
// startHermes used to pin primaryProfileKey() at the top and separately
|
||||
// re-read the preference deep inside the connection IIFE for --profile and
|
||||
// the child env — two reads that a mid-startup hermes:profile:remember could
|
||||
// split into "routing says alpha, argv says beta". resolveLaunchProfile makes
|
||||
// them ONE read with two encodings of the unset case.
|
||||
test('one launch decision feeds routing, argv, and env from the same read', () => {
|
||||
const named = resolveLaunchProfile(() => 'beta')
|
||||
|
||||
assert.equal(named.routingProfile, 'beta')
|
||||
assert.equal(named.argvProfile, 'beta')
|
||||
|
||||
// Unset preference: routing falls back to 'default', the launch argument
|
||||
// keeps the legacy shape (no --profile flag at all).
|
||||
const unset = resolveLaunchProfile(() => null)
|
||||
|
||||
assert.equal(unset.routingProfile, 'default')
|
||||
assert.equal(unset.argvProfile, null)
|
||||
|
||||
const blank = resolveLaunchProfile(() => ' ')
|
||||
|
||||
assert.equal(blank.routingProfile, 'default')
|
||||
assert.equal(blank.argvProfile, null)
|
||||
})
|
||||
|
||||
@@ -49,3 +49,22 @@ export class PrimaryProfilePin {
|
||||
return String(readPreference() ?? '').trim() || 'default'
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* One authoritative launch-profile decision for a startup attempt (#108417).
|
||||
*
|
||||
* `routingProfile` is what the pin and the routing table should answer for
|
||||
* (the preference at decision time, 'default' when unset); `argvProfile` is
|
||||
* what the local launch argument and child env carry (the same preference,
|
||||
* `null` when unset so the legacy flag-less launch is preserved). Both come
|
||||
* from the SAME read so a preference change landing mid-startup becomes the
|
||||
* next boot's decision instead of splitting routing identity from
|
||||
* `--profile`.
|
||||
*/
|
||||
export function resolveLaunchProfile(
|
||||
readPreference: () => null | string | undefined
|
||||
): { argvProfile: null | string; routingProfile: string } {
|
||||
const argvProfile = String(readPreference() ?? '').trim() || null
|
||||
|
||||
return { argvProfile, routingProfile: argvProfile ?? 'default' }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user