From f97e7086c7cf2968748bc9d59913913396fa1d1a Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sat, 25 Jul 2026 21:34:00 -0500 Subject: [PATCH] fix(desktop): keep the first-run local-start error from being wiped on mount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../desktop-install-overlay.test.tsx | 77 +++++++++++++++++++ .../components/desktop-install-overlay.tsx | 28 ++++--- 2 files changed, 94 insertions(+), 11 deletions(-) diff --git a/apps/desktop/src/components/desktop-install-overlay.test.tsx b/apps/desktop/src/components/desktop-install-overlay.test.tsx index ce3b9a04548f..1e4e384cb111 100644 --- a/apps/desktop/src/components/desktop-install-overlay.test.tsx +++ b/apps/desktop/src/components/desktop-install-overlay.test.tsx @@ -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 { + 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() + + // 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() + + 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({ diff --git a/apps/desktop/src/components/desktop-install-overlay.tsx b/apps/desktop/src/components/desktop-install-overlay.tsx index 8a56f93c8c45..4941f6dd6bc4 100644 --- a/apps/desktop/src/components/desktop-install-overlay.tsx +++ b/apps/desktop/src/components/desktop-install-overlay.tsx @@ -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(null) const [now, setNow] = useState(() => Date.now()) const logEndRef = useRef(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"