fix(desktop): keep the first-run local-start error from being wiped on mount

Clicking "Install Hermes locally" surfaced no error when the bridge was
missing, if the click landed before React drained the passive effect that
reacted to the first bootstrap snapshot. The effect cleared the transient
button state on every change of setupChoice.activeRoot, and the very first
snapshot counts as a change: it moves the root from null to its initial
value. The choice paints as soon as that snapshot commits, so a fast click
produced an error that the effect then cleared before it ever rendered.

The state now records the root it was produced under and is read back only
under that same root, so leaving a phase still discards it but arriving in
one cannot. Deriving it removes the effect rather than adding a guard to it.

Reproduced by observing the DOM with a MutationObserver instead of findBy*,
which only settles on a timer tick — by then the effect has already run and
the window is closed. Reverting the component change fails the new test.
This commit is contained in:
Brooklyn Nicholson 2026-07-25 21:34:00 -05:00
parent 243a01d5d7
commit f97e7086c7
2 changed files with 94 additions and 11 deletions

View file

@ -52,6 +52,33 @@ function installDesktopMock(state: DesktopBootstrapState) {
return desktop
}
// Resolve the instant a node commits, via MutationObserver rather than
// waitFor's polling timer. findBy* only settles on a timer tick, by which
// point React has already drained its passive effects — that hides any bug
// living in the window between paint and effect.
function whenPresent(text: string): Promise<HTMLElement> {
return new Promise(resolve => {
const existing = screen.queryByText(text)
if (existing) {
resolve(existing)
return
}
const observer = new MutationObserver(() => {
const node = screen.queryByText(text)
if (node) {
observer.disconnect()
resolve(node)
}
})
observer.observe(document.body, { childList: true, subtree: true, characterData: true })
})
}
beforeEach(() => {
vi.restoreAllMocks()
})
@ -120,6 +147,56 @@ describe('DesktopInstallOverlay first-run setup', () => {
expect(install.disabled).toBe(false)
})
it('keeps the local-start error when the first snapshot commits under the click', async () => {
const desktop = installDesktopMock(
bootstrapState({
setupChoice: { platform: 'win32', activeRoot: 'C:\\Users\\me\\AppData\\Local\\hermes\\hermes-agent' }
})
)
desktop.continueBootstrapLocal = undefined as never
render(<DesktopInstallOverlay />)
// Click the instant the choice paints, before React drains the passive
// effect that reacts to the first snapshot. A loaded runner hits this
// window by accident; observing the DOM directly hits it every time.
const install = (await whenPresent('Install Hermes locally')).closest('button') as HTMLButtonElement
fireEvent.click(install)
await act(async () => {
await Promise.resolve()
})
expect(screen.queryByText('Local installation could not start. Restart Hermes Desktop and try again.')).toBeTruthy()
})
it('clears a stale local-start error when a repair presents a different root', async () => {
const desktop = installDesktopMock(
bootstrapState({
setupChoice: { platform: 'win32', activeRoot: 'C:\\Users\\me\\AppData\\Local\\hermes\\hermes-agent' }
})
)
desktop.continueBootstrapLocal = undefined as never
render(<DesktopInstallOverlay />)
fireEvent.click((await screen.findByText('Install Hermes locally')).closest('button') as HTMLButtonElement)
expect(
await screen.findByText('Local installation could not start. Restart Hermes Desktop and try again.')
).toBeTruthy()
act(() => {
desktop.emitBootstrapEvent({
type: 'setup-choice',
active: false,
platform: 'win32',
activeRoot: 'C:\\Users\\me\\AppData\\Local\\hermes\\hermes-agent-repaired'
})
})
expect(screen.queryByText('Local installation could not start. Restart Hermes Desktop and try again.')).toBeNull()
})
it('opens the remote connection form from the first-run choice', async () => {
installDesktopMock(
bootstrapState({

View file

@ -277,8 +277,6 @@ export function DesktopInstallOverlay({ enabled = true }: DesktopInstallOverlayP
const [copied, setCopied] = useState(false)
const [cancelling, setCancelling] = useState(false)
const [remoteOpen, setRemoteOpen] = useState(false)
const [localStarting, setLocalStarting] = useState(false)
const [localStartError, setLocalStartError] = useState<string | null>(null)
const [now, setNow] = useState(() => Date.now())
const logEndRef = useRef<HTMLDivElement | null>(null)
@ -347,11 +345,21 @@ export function DesktopInstallOverlay({ enabled = true }: DesktopInstallOverlayP
// The choice remains mounted while main hands off to local bootstrap. Once
// a manifest/failure takes ownership (or a later repair presents a fresh
// choice), clear the transient button state so it cannot leak across phases.
useEffect(() => {
setLocalStarting(false)
setLocalStartError(null)
}, [state.setupChoice?.activeRoot])
// choice), this transient button state must not leak across phases — so it
// records the root it was produced under and is read back only under that
// same root. Deriving it beats clearing it in an effect: the choice paints
// as soon as the first snapshot commits, and a click landing before such an
// effect flushed would have its error wiped before it ever rendered.
const [localStart, setLocalStart] = useState<{
root: string | null
starting: boolean
error: string | null
}>({ root: null, starting: false, error: null })
const activeRoot = state.setupChoice?.activeRoot ?? null
const forActiveRoot = localStart.root === activeRoot
const localStarting = forActiveRoot && localStart.starting
const localStartError = forActiveRoot ? localStart.error : null
// Mount logic: show whenever a bootstrap is in flight, completed-with-error,
// or actively running with a manifest. Hide entirely after a successful
@ -417,8 +425,7 @@ export function DesktopInstallOverlay({ enabled = true }: DesktopInstallOverlayP
className="rounded-lg border border-(--ui-stroke-tertiary) bg-(--ui-bg-quinary) p-4 text-left transition hover:bg-(--chrome-action-hover) disabled:cursor-wait disabled:opacity-60"
disabled={localStarting}
onClick={async () => {
setLocalStarting(true)
setLocalStartError(null)
setLocalStart({ root: activeRoot, starting: true, error: null })
try {
const desktop = window.hermesDesktop
@ -429,8 +436,7 @@ export function DesktopInstallOverlay({ enabled = true }: DesktopInstallOverlayP
await desktop.continueBootstrapLocal()
} catch (err) {
setLocalStartError(errorMessage(err))
setLocalStarting(false)
setLocalStart({ root: activeRoot, starting: false, error: errorMessage(err) })
}
}}
type="button"