fix(desktop): make remote backend updates terminal-state driven

Remote backend updates failed with "Backend update failed." on nearly every run: applyBackendUpdate() polled for only 30×1.5s ≈ 45s, then read exit_code null off the still-running action and called it a failure. Real updates (backup + uv sync + npm install + vite build) routinely run longer, and the no-op "Already up to date" path never restarted the gateway so the old return-check timed out too. A still-running, reachable action is now never converted into failure by an elapsed budget — only a nonzero exit is. The apply loop keeps one in-flight promise, tolerates reconnects during the dashboard restart without extending the fixed six-minute deadline forever, and confirms success by the action-specific receipt that survives the restart, falling back to proving the requested commit / up-to-date check for older backends without action_id support. Inconclusive completion fails closed. Fixes #47359 Fixes #58764 Co-authored-by: Vitor Cepeda Lopes <vitor@vitorcepedalopes.com> Co-authored-by: Mark Vlcek <markvlcek@gmail.com> Co-authored-by: doncazper <caztronics@yahoo.com>
2026-08-05 10:34:18 -06:00
parent 950b55d4d7
commit eb68ffbe43
3 changed files with 429 additions and 67 deletions
--- a/apps/desktop/src/store/updates.test.ts
+++ b/apps/desktop/src/store/updates.test.ts
@@ -290,7 +290,11 @@ describe('requestActiveUpdate', () => {
    vi.useRealTimers()
  })

-  afterEach(() => {
+  afterEach(async () => {
+    // Drain any backend apply this suite kicked off: applyBackendUpdate() now
+    // memoizes the in-flight run, so a dangling promise here would be handed
+    // to the next suite's tests instead of a fresh run.
+    await vi.waitFor(() => expect($backendUpdateApply.get().applying).toBe(false), { timeout: 5000 })
    setRemote(false)
    delete (globalThis as unknown as { window?: unknown }).window
  })
@@ -465,6 +469,7 @@ describe('applyBackendUpdate recovery', () => {
    checkHermesUpdateSpy.mockReset()
    updateHermesSpy.mockReset()
    getActionStatusSpy.mockReset()
+    $backendUpdateStatus.set(null)
    $backendUpdateApply.set({
      applying: false,
      stage: 'idle',
@@ -482,16 +487,14 @@ describe('applyBackendUpdate recovery', () => {
  })

  it('waits for the backend to return after the restart drops the connection, then clears the overlay', async () => {
-    updateHermesSpy.mockResolvedValue({ ok: true, name: 'update', pid: 1 })
-    getActionStatusSpy.mockRejectedValue(new Error('ECONNREFUSED'))
-    checkHermesUpdateSpy.mockResolvedValue({
-      install_method: 'git',
-      current_version: '0.16.0',
-      behind: 0,
-      update_available: false,
-      can_apply: true,
-      update_command: 'hermes update',
-      message: null
+    const actionId = 'd'.repeat(32)
+    updateHermesSpy.mockResolvedValue({ action_id: actionId, ok: true, name: 'update', pid: 1 })
+    getActionStatusSpy.mockRejectedValueOnce(new Error('ECONNREFUSED')).mockResolvedValueOnce({
+      exit_code: null,
+      lines: [`=== hermes-update completed ${actionId} ===`],
+      name: 'update',
+      pid: null,
+      running: false
    })

    const promise = applyBackendUpdate()
@@ -504,7 +507,8 @@ describe('applyBackendUpdate recovery', () => {
  })

  it('surfaces backend update action log lines while the action is running', async () => {
-    updateHermesSpy.mockResolvedValue({ ok: true, name: 'update', pid: 1 })
+    const actionId = 'e'.repeat(32)
+    updateHermesSpy.mockResolvedValue({ action_id: actionId, ok: true, name: 'update', pid: 1 })
    getActionStatusSpy
      .mockResolvedValueOnce({
        exit_code: null,
@@ -514,15 +518,13 @@ describe('applyBackendUpdate recovery', () => {
        running: true
      })
      .mockRejectedValueOnce(new Error('ECONNREFUSED'))
-    checkHermesUpdateSpy.mockResolvedValue({
-      install_method: 'git',
-      current_version: '0.16.0',
-      behind: 0,
-      update_available: false,
-      can_apply: true,
-      update_command: 'hermes update',
-      message: null
-    })
+      .mockResolvedValueOnce({
+        exit_code: null,
+        lines: [`=== hermes-update completed ${actionId} ===`],
+        name: 'update',
+        pid: null,
+        running: false
+      })

    const promise = applyBackendUpdate()
    await vi.advanceTimersByTimeAsync(1500)
@@ -537,18 +539,323 @@ describe('applyBackendUpdate recovery', () => {
    await promise
  })

+  it('keeps waiting past the old 45-second cutoff while the update action is running', async () => {
+    const actionId = 'f'.repeat(32)
+    updateHermesSpy.mockResolvedValue({ action_id: actionId, ok: true, name: 'hermes-update', pid: 1 })
+
+    for (let attempt = 0; attempt < 31; attempt += 1) {
+      getActionStatusSpy.mockResolvedValueOnce({
+        exit_code: null,
+        lines: ['=== hermes-update started now ===', `step ${attempt}`],
+        name: 'hermes-update',
+        pid: 1,
+        running: true
+      })
+    }
+
+    getActionStatusSpy.mockRejectedValueOnce(new Error('ECONNREFUSED')).mockResolvedValueOnce({
+      exit_code: null,
+      lines: [`=== hermes-update completed ${actionId} ===`],
+      name: 'hermes-update',
+      pid: null,
+      running: false
+    })
+
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(46500)
+
+    expect($backendUpdateApply.get().applying).toBe(true)
+    expect($backendUpdateApply.get().stage).toBe('pull')
+
+    await vi.advanceTimersByTimeAsync(5000)
+    await expect(promise).resolves.toMatchObject({ ok: true })
+  })
+
+  it('treats a successful no-op as complete without waiting for a restart', async () => {
+    updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 })
+    getActionStatusSpy.mockResolvedValue({
+      exit_code: 0,
+      lines: ['stale output from another run', '=== hermes-update started now ===', '✓ Already up to date!'],
+      name: 'hermes-update',
+      pid: 1,
+      running: false
+    })
+
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(1500)
+    const result = await promise
+
+    expect(result.ok).toBe(true)
+    expect($backendUpdateApply.get().stage).toBe('idle')
+  })
+
+  it('treats a successful dependency repair as complete without waiting for a restart', async () => {
+    updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 })
+    getActionStatusSpy.mockResolvedValue({
+      exit_code: 0,
+      lines: ['=== hermes-update started now ===', '✓ Dependencies repaired!', '✓ Update complete!'],
+      name: 'hermes-update',
+      pid: 1,
+      running: false
+    })
+
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(1500)
+    await expect(promise).resolves.toMatchObject({ ok: true })
+    expect($backendUpdateApply.get().stage).toBe('idle')
+  })
+
+  it('trusts the current action exit code without parsing its output', async () => {
+    updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 })
+    getActionStatusSpy.mockResolvedValue({
+      exit_code: 0,
+      lines: ['✓ Already up to date!'],
+      name: 'hermes-update',
+      pid: 1,
+      running: false
+    })
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(1500)
+    await expect(promise).resolves.toMatchObject({ ok: true })
+    expect(checkHermesUpdateSpy).not.toHaveBeenCalled()
+  })
+
+  it('waits for current-action completion proof after the backend restarts', async () => {
+    const actionId = 'a'.repeat(32)
+    updateHermesSpy.mockResolvedValue({ action_id: actionId, ok: true, name: 'hermes-update', pid: 1 })
+    getActionStatusSpy
+      .mockRejectedValueOnce(new Error('ECONNREFUSED'))
+      .mockResolvedValueOnce({
+        exit_code: null,
+        lines: ['Update complete!', `=== hermes-update completed ${'c'.repeat(32)} ===`],
+        name: 'hermes-update',
+        pid: null,
+        running: false
+      })
+      .mockResolvedValueOnce({
+        exit_code: null,
+        lines: ['Update complete!', `=== hermes-update completed ${actionId} ===`],
+        name: 'hermes-update',
+        pid: null,
+        running: false
+      })
+
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(5000)
+    await expect(promise).resolves.toMatchObject({ ok: true })
+    expect(checkHermesUpdateSpy).not.toHaveBeenCalled()
+  })
+
+  it('accepts its terminal receipt when a verbose update pushes the start marker out of the log tail', async () => {
+    const actionId = 'b'.repeat(32)
+    updateHermesSpy.mockResolvedValue({ action_id: actionId, ok: true, name: 'hermes-update', pid: 1 })
+    getActionStatusSpy.mockRejectedValueOnce(new Error('ECONNREFUSED')).mockResolvedValueOnce({
+      exit_code: null,
+      lines: ['final build output', 'Update complete!', `=== hermes-update completed ${actionId} ===`],
+      name: 'hermes-update',
+      pid: null,
+      running: false
+    })
+
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(5000)
+
+    await expect(promise).resolves.toMatchObject({ ok: true })
+    expect(getActionStatusSpy).toHaveBeenCalledWith('hermes-update', 2000)
+  })
+
+  it('proves a pre-action-ID backend reached its requested commit after restart', async () => {
+    $backendUpdateStatus.set({
+      behind: 2,
+      commits: [{ at: 1, author: 'Nous', sha: 'requested-target', summary: 'target' }],
+      fetchedAt: 1,
+      supported: true,
+      targetSha: 'backend:0.18.2',
+      updateAvailable: true
+    })
+    updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 })
+    getActionStatusSpy.mockRejectedValueOnce(new Error('ECONNREFUSED')).mockResolvedValue({
+      exit_code: null,
+      lines: ['verbose output', 'Update complete!'],
+      name: 'hermes-update',
+      pid: null,
+      running: false
+    })
+    checkHermesUpdateSpy
+      .mockResolvedValueOnce({
+        behind: null,
+        can_apply: true,
+        commits: [],
+        current_version: '0.18.2',
+        install_method: 'git',
+        message: 'offline',
+        update_available: false,
+        update_command: 'hermes update'
+      })
+      .mockResolvedValueOnce({
+        behind: 1,
+        can_apply: true,
+        commits: [{ at: 2, author: 'Nous', sha: 'newer-commit', summary: 'newer' }],
+        current_version: '0.18.2',
+        install_method: 'git',
+        message: null,
+        update_available: true,
+        update_command: 'hermes update'
+      })
+
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(5000)
+
+    await expect(promise).resolves.toMatchObject({ ok: true })
+    expect(checkHermesUpdateSpy).toHaveBeenCalledTimes(2)
+  })
+
+  it('proves a fast pre-action-ID packaged update by its changed version', async () => {
+    $backendUpdateStatus.set({
+      behind: 1,
+      commits: [],
+      fetchedAt: 1,
+      supported: true,
+      targetSha: 'backend:0.18.2',
+      updateAvailable: true
+    })
+    updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 })
+    getActionStatusSpy.mockResolvedValue({
+      exit_code: null,
+      lines: ['verbose output without a retained start marker'],
+      name: 'hermes-update',
+      pid: null,
+      running: false
+    })
+    checkHermesUpdateSpy.mockResolvedValue({
+      behind: -1,
+      can_apply: true,
+      commits: [],
+      current_version: '0.18.3',
+      install_method: 'pip',
+      message: null,
+      update_available: true,
+      update_command: 'hermes update'
+    })
+
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(1500)
+
+    await expect(promise).resolves.toMatchObject({ ok: true })
+    expect(checkHermesUpdateSpy).toHaveBeenCalledWith(true)
+  })
+
+  it('resumes action polling after a transient status failure', async () => {
+    updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 })
+    getActionStatusSpy
+      .mockRejectedValueOnce(new Error('ECONNRESET'))
+      .mockResolvedValueOnce({
+        exit_code: null,
+        lines: ['=== hermes-update started now ===', 'still running'],
+        name: 'hermes-update',
+        pid: 1,
+        running: true
+      })
+      .mockResolvedValueOnce({
+        exit_code: 0,
+        lines: ['=== hermes-update started now ===', 'Update complete!'],
+        name: 'hermes-update',
+        pid: 1,
+        running: false
+      })
+
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(5000)
+    await expect(promise).resolves.toMatchObject({ ok: true })
+    expect(getActionStatusSpy).toHaveBeenCalledTimes(3)
+  })
+
+  it('restores the fixed action deadline after reconnecting', async () => {
+    updateHermesSpy.mockResolvedValue({ action_id: 'a'.repeat(32), ok: true, name: 'hermes-update', pid: 1 })
+    const running = {
+      exit_code: null,
+      lines: ['still running'],
+      name: 'hermes-update',
+      pid: 1,
+      running: true
+    }
+
+    for (let attempt = 0; attempt < 119; attempt += 1) {
+      getActionStatusSpy.mockResolvedValueOnce(running)
+    }
+    getActionStatusSpy.mockRejectedValueOnce(new Error('ECONNRESET')).mockResolvedValue(running)
+
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(6 * 60 * 1000 + 1500)
+
+    await expect(promise).resolves.toMatchObject({ error: 'apply-failed', ok: false })
+    expect($backendUpdateApply.get().stage).toBe('error')
+  })
+
+  it('shares one in-flight update between concurrent apply requests', async () => {
+    updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 })
+    getActionStatusSpy.mockResolvedValue({
+      exit_code: 0,
+      lines: ['=== hermes-update started now ===', '✓ Already up to date!'],
+      name: 'hermes-update',
+      pid: 1,
+      running: false
+    })
+
+    const first = applyBackendUpdate()
+    const second = applyBackendUpdate()
+
+    expect(second).toBe(first)
+    await vi.advanceTimersByTimeAsync(1500)
+    await Promise.all([first, second])
+    expect(updateHermesSpy).toHaveBeenCalledTimes(1)
+  })
+
+  it('fails closed when the update action never reaches a terminal state', async () => {
+    updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 })
+    getActionStatusSpy.mockResolvedValue({
+      exit_code: null,
+      lines: ['=== hermes-update started now ===', 'still running'],
+      name: 'hermes-update',
+      pid: 1,
+      running: true
+    })
+
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(6 * 60 * 1000 + 1500)
+    await expect(promise).resolves.toMatchObject({ ok: false, error: 'apply-failed' })
+    expect($backendUpdateApply.get().stage).toBe('error')
+  })
+
+  it('fails immediately when the update action exits nonzero', async () => {
+    updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 })
+    getActionStatusSpy.mockResolvedValue({
+      exit_code: 1,
+      lines: ['=== hermes-update started now ===', 'update failed'],
+      name: 'hermes-update',
+      pid: 1,
+      running: false
+    })
+
+    const promise = applyBackendUpdate()
+    await vi.advanceTimersByTimeAsync(1500)
+    await expect(promise).resolves.toMatchObject({ ok: false, error: 'apply-failed' })
+    expect(checkHermesUpdateSpy).not.toHaveBeenCalled()
+    expect($backendUpdateApply.get().stage).toBe('error')
+  })
+
  it('surfaces an error when the backend never comes back after the restart', async () => {
    updateHermesSpy.mockResolvedValue({ ok: true, name: 'update', pid: 1 })
    getActionStatusSpy.mockRejectedValue(new Error('ECONNREFUSED'))
    checkHermesUpdateSpy.mockRejectedValue(new Error('ECONNREFUSED'))

    const promise = applyBackendUpdate()
-    await vi.advanceTimersByTimeAsync(70000)
+    await vi.advanceTimersByTimeAsync(250000)
    const result = await promise

    expect(result.ok).toBe(false)
    expect($backendUpdateApply.get().stage).toBe('error')
-  })
+  }, 10000)
 })

 describe('startUpdatePoller', () => {
--- a/apps/desktop/src/store/updates.ts
+++ b/apps/desktop/src/store/updates.ts
@@ -487,24 +487,9 @@ export async function applyUpdates(opts: DesktopUpdateApplyOptions = {}): Promis
  }
 }

-const BACKEND_RETURN_POLL_MS = 1500
-const BACKEND_RETURN_MAX_ATTEMPTS = 40
-
-async function waitForBackendReturn(): Promise<boolean> {
-  for (let attempt = 0; attempt < BACKEND_RETURN_MAX_ATTEMPTS; attempt += 1) {
-    await new Promise(resolve => globalThis.setTimeout(resolve, BACKEND_RETURN_POLL_MS))
-
-    try {
-      await checkHermesUpdate()
-
-      return true
-    } catch {
-      continue
-    }
-  }
-
-  return false
-}
+const BACKEND_ACTION_POLL_MS = 1500
+const BACKEND_ACTION_MAX_MS = 6 * 60 * 1000
+const BACKEND_RETURN_MAX_MS = 4 * 60 * 1000

 function finishBackendApply(returned: boolean): DesktopUpdateApplyResult {
  if (returned) {
@@ -547,7 +532,32 @@ function ingestBackendActionStatus(status: Awaited<ReturnType<typeof getActionSt
  })
 }

-export async function applyBackendUpdate(): Promise<DesktopUpdateApplyResult> {
+function completedAfterRestart(
+  status: Awaited<ReturnType<typeof getActionStatus>>,
+  actionId: string | undefined
+): boolean {
+  return !!actionId && status.lines.some(line => line === `=== hermes-update completed ${actionId} ===`)
+}
+
+function legacyBackendReachedTarget(
+  status: BackendUpdateCheckResponse,
+  targetSha: string | undefined,
+  previousVersion: string | undefined
+): boolean {
+  if (status.behind === 0) {
+    return true
+  }
+
+  if (previousVersion && status.current_version !== previousVersion) {
+    return true
+  }
+
+  return !!targetSha && !!status.commits?.length && !status.commits.some(commit => commit.sha === targetSha)
+}
+
+let backendUpdateInFlight: Promise<DesktopUpdateApplyResult> | null = null
+
+async function runBackendUpdate(): Promise<DesktopUpdateApplyResult> {
  dismissNotification(UPDATE_TOAST_ID)
  $backendUpdateApply.set({
    ...IDLE,
@@ -557,6 +567,11 @@ export async function applyBackendUpdate(): Promise<DesktopUpdateApplyResult> {
  })

  try {
+    const previousStatus = $backendUpdateStatus.get()
+    const requestedTargetSha = previousStatus?.commits?.at(0)?.sha
+    const previousVersion = previousStatus?.targetSha?.startsWith('backend:')
+      ? previousStatus.targetSha.slice('backend:'.length)
+      : undefined
    const started = await updateHermes()

    if (!started.ok) {
@@ -575,43 +590,69 @@ export async function applyBackendUpdate(): Promise<DesktopUpdateApplyResult> {
    })

    let last: Awaited<ReturnType<typeof getActionStatus>> | null = null
+    // Backups, dependency repair, and builds can legitimately take several
+    // minutes. Keep the generous cap only as a guard against a stuck action.
+    const actionDeadline = Date.now() + BACKEND_ACTION_MAX_MS
+    let deadline = actionDeadline
+    let reconnecting = false

-    for (let attempt = 0; attempt < 30; attempt += 1) {
-      await new Promise(resolve => globalThis.setTimeout(resolve, 1500))
+    while (Date.now() < deadline) {
+      await new Promise(resolve => globalThis.setTimeout(resolve, BACKEND_ACTION_POLL_MS))

      try {
-        last = await getActionStatus(started.name, 200)
+        last = await getActionStatus(started.name, 2000)
        ingestBackendActionStatus(last)
      } catch {
-        // The dashboard restarts mid-update, dropping this connection — expected, not a failure.
-        $backendUpdateApply.set({
-          ...$backendUpdateApply.get(),
-          applying: true,
-          stage: 'restart',
-          message: translateNow('updates.applyStatus.restarting')
-        })
+        if (!reconnecting) {
+          reconnecting = true
+          deadline = Date.now() + BACKEND_RETURN_MAX_MS
+          $backendUpdateApply.set({
+            ...$backendUpdateApply.get(),
+            applying: true,
+            stage: 'restart',
+            message: translateNow('updates.applyStatus.restarting')
+          })
+        }

-        return finishBackendApply(await waitForBackendReturn())
+        continue
      }

-      if (last && !last.running) {
+      if (last.running) {
+        if (reconnecting) {
+          reconnecting = false
+          deadline = actionDeadline
+          $backendUpdateApply.set({
+            ...$backendUpdateApply.get(),
+            applying: true,
+            stage: 'pull',
+            message: translateNow('updates.applyStatus.pulling')
+          })
+        }
+
+        continue
+      }
+
+      if (last.exit_code === 0 || (last.exit_code === null && completedAfterRestart(last, started.action_id))) {
+        return finishBackendApply(true)
+      }
+
+      if (!started.action_id && last.exit_code === null) {
+        try {
+          const status = await checkHermesUpdate(true)
+
+          if (legacyBackendReachedTarget(status, requestedTargetSha, previousVersion)) {
+            return finishBackendApply(true)
+          }
+        } catch {
+          continue
+        }
+      }
+
+      if (last.exit_code !== null) {
        break
      }
    }

-    const ok = !!last && (last.exit_code ?? 1) === 0
-
-    if (ok) {
-      $backendUpdateApply.set({
-        ...$backendUpdateApply.get(),
-        applying: true,
-        stage: 'restart',
-        message: translateNow('updates.applyStatus.restarting')
-      })
-
-      return finishBackendApply(await waitForBackendReturn())
-    }
-
    $backendUpdateApply.set({
      ...$backendUpdateApply.get(),
      applying: false,
@@ -635,6 +676,18 @@ export async function applyBackendUpdate(): Promise<DesktopUpdateApplyResult> {
  }
 }

+export function applyBackendUpdate(): Promise<DesktopUpdateApplyResult> {
+  if (backendUpdateInFlight) {
+    return backendUpdateInFlight
+  }
+
+  backendUpdateInFlight = runBackendUpdate().finally(() => {
+    backendUpdateInFlight = null
+  })
+
+  return backendUpdateInFlight
+}
+
 function ingestProgress(payload: DesktopUpdateProgress): void {
  const current = $updateApply.get()
  const log = [...current.log, { stage: payload.stage, message: payload.message, at: payload.at }].slice(-50)
--- a/apps/desktop/src/types/hermes.ts
+++ b/apps/desktop/src/types/hermes.ts
@@ -1144,6 +1144,8 @@ export interface ActionResponse {
  name: string
  ok: boolean
  pid: number
+  action_id?: string
+  already_running?: boolean
 }

 export interface ActionStatusResponse {