fix(desktop): show a queued cron run row immediately after trigger

Run History only rendered sessions the backend had already materialized, so an accepted Trigger now was invisible for the tens of seconds the scheduler took to create the run session (#70826). The queued row is painted from the click, settles when Run History observes a run that started after it (any load — a fast 1s poll while queued, the cron.changed broadcast on event-capable backends, or the visibility poll), and drops at a bounded 90s timeout so a never-materializing run cannot wedge the button. A failed request drops the row immediately. Pause deliberately does not settle it: the pause response only persists enabled=false and never cancels an already-claimed execution. Queued design (queued-run row + polling + settle semantics) originates in #70840 by @nv-cho; this ports it onto current main's createCronTriggerController wiring. Fixes #70826
2026-10-06 15:11:30 -04:00
parent 84692b7d74
commit 44c1ad7f84
5 changed files with 423 additions and 13 deletions
--- a/apps/desktop/src/app/cron/index.test.tsx
+++ b/apps/desktop/src/app/cron/index.test.tsx
@@ -0,0 +1,271 @@
+// @vitest-environment jsdom
+import { QueryClientProvider } from '@tanstack/react-query'
+import { act, cleanup, render, screen, waitFor } from '@testing-library/react'
+import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
+
+import type * as HermesApi from '@/hermes'
+import type { CronJob, SessionInfo } from '@/hermes'
+import { queryClient } from '@/lib/query-client'
+import { $cronFocusJobId, $cronJobs, setCronFocusJobId, setCronJobs } from '@/store/cron'
+import { notifyCronChanged, setChangeEventsAvailable } from '@/store/live-sync'
+
+const getCronJobRuns = vi.fn()
+const getCronJobs = vi.fn()
+const triggerCronJob = vi.fn()
+
+vi.mock('@/hermes', async importOriginal => ({
+  ...(await importOriginal<typeof HermesApi>()),
+  getCronJobRuns: (jobId: string) => getCronJobRuns(jobId),
+  getCronJobs: (profile?: string) => getCronJobs(profile),
+  triggerCronJob: (jobId: string) => triggerCronJob(jobId)
+}))
+
+vi.mock('@/store/notifications', () => ({
+  notify: vi.fn(),
+  notifyError: vi.fn()
+}))
+
+// No runs yet: the queued trigger row must carry the Run History section on
+// its own, which is exactly the gap the issue reports (#70826).
+const job: CronJob = {
+  deliver: 'local',
+  enabled: true,
+  id: 'job-1',
+  name: 'Status report',
+  next_run_at: '2026-10-06T12:00:00Z',
+  prompt: 'Prepare a status report',
+  schedule: { expr: '*/5 * * * *', kind: 'cron' },
+  schedule_display: 'Every 5 minutes',
+  state: 'scheduled'
+} as CronJob
+
+const queuedRow = () => window.document.querySelector('[data-slot="cron-run-pending"]')
+
+async function renderCron() {
+  const { CronView } = await import('./index')
+  let result!: ReturnType<typeof render>
+
+  await act(async () => {
+    result = render(
+      <QueryClientProvider client={queryClient}>
+        <CronView onClose={vi.fn()} />
+      </QueryClientProvider>
+    )
+  })
+
+  return result
+}
+
+async function renderLoadedCron() {
+  const result = await renderCron()
+
+  await screen.findByText('No runs yet')
+
+  return result
+}
+
+beforeAll(() => {
+  HTMLElement.prototype.scrollIntoView ??= () => undefined
+  vi.stubGlobal('CSS', { escape: (value: string) => value })
+})
+
+beforeEach(() => {
+  setCronJobs([job])
+  setCronFocusJobId(null)
+  setChangeEventsAvailable(false)
+  getCronJobs.mockResolvedValue([job])
+  getCronJobRuns.mockResolvedValue([])
+  triggerCronJob.mockResolvedValue(job)
+})
+
+afterEach(() => {
+  vi.clearAllTimers()
+  vi.useRealTimers()
+  cleanup()
+  queryClient.clear()
+  setCronJobs([])
+  setCronFocusJobId(null)
+  setChangeEventsAvailable(false)
+  vi.clearAllMocks()
+})
+
+describe('CronView queued trigger feedback (#70826)', () => {
+  it('paints the queued run row immediately and settles it when the run appears', async () => {
+    const { container } = await renderLoadedCron()
+    const triggerButton = screen.getByRole('button', { name: 'Trigger now' })
+
+    // The trigger request is still in flight — the queued row is pure
+    // optimism from the click, not a backend response.
+    let releaseTrigger!: (value: CronJob) => void
+    triggerCronJob.mockReturnValue(new Promise<CronJob>(resolve => (releaseTrigger = resolve)))
+
+    await act(async () => {
+      triggerButton.click()
+      await Promise.resolve()
+    })
+
+    expect(queuedRow()).toBeTruthy()
+    expect(triggerButton.getAttribute('aria-busy')).toBe('true')
+    expect(getCronJobRuns).toHaveBeenCalled()
+
+    // The backend materializes the run the trigger produced.
+    const observedAt = Date.now() / 1000
+    getCronJobRuns.mockResolvedValue([
+      { id: 'cron_job-1_20261006_120001', last_active: observedAt, started_at: observedAt, title: 'Status report run' } as SessionInfo
+    ])
+    await act(async () => {
+      releaseTrigger(job)
+    })
+
+    await screen.findByText('Status report run')
+    await waitFor(() => expect(queuedRow()).toBeNull())
+    expect((triggerButton as HTMLButtonElement).disabled).toBe(false)
+    expect(container.querySelector('.codicon-zap')).toBeTruthy()
+  })
+
+  it('reconciles the queued row immediately when cron.changed publishes the run', async () => {
+    setChangeEventsAvailable(true)
+    const { container } = await renderLoadedCron()
+    const triggerButton = screen.getByRole('button', { name: 'Trigger now' })
+
+    getCronJobRuns.mockClear()
+    await act(async () => {
+      triggerButton.click()
+    })
+    await waitFor(() => expect(getCronJobRuns).toHaveBeenCalled())
+
+    // Between polls (the event-capable cadence is a slow backstop), the
+    // broadcast must be enough to surface the run and settle the row.
+    const callsBeforeChange = getCronJobRuns.mock.calls.length
+    const observedAt = Date.now() / 1000
+    getCronJobRuns.mockResolvedValue([
+      { id: 'cron_job-1_20261006_120002', last_active: observedAt, started_at: observedAt, title: 'Status report run' } as SessionInfo
+    ])
+    await act(async () => {
+      notifyCronChanged()
+    })
+
+    await waitFor(() => expect(getCronJobRuns.mock.calls.length).toBeGreaterThan(callsBeforeChange))
+    await screen.findByText('Status report run')
+    await waitFor(() => expect(queuedRow()).toBeNull())
+    expect((triggerButton as HTMLButtonElement).disabled).toBe(false)
+    expect(container.querySelector('[data-slot="cron-run-pending"]')).toBeNull()
+  })
+
+  it('polls fast while the run is queued and not before', async () => {
+    await renderLoadedCron()
+
+    getCronJobRuns.mockClear()
+    vi.useFakeTimers()
+    // Idle cadence: 8s legacy poll (change events unavailable in this test).
+    await act(async () => {
+      vi.advanceTimersByTimeAsync(3000)
+    })
+    expect(getCronJobRuns).not.toHaveBeenCalled()
+
+    const triggerButton = screen.getByRole('button', { name: 'Trigger now' })
+
+    await act(async () => {
+      triggerButton.click()
+      await Promise.resolve()
+    })
+
+    // Queued cadence: 1s pending poll fires well inside the idle interval.
+    await act(async () => {
+      vi.advanceTimersByTimeAsync(1500)
+    })
+
+    expect(getCronJobRuns.mock.calls.length).toBeGreaterThanOrEqual(1)
+  })
+
+  it('drops the queued row and unlocks the action at the bounded timeout', async () => {
+    vi.useFakeTimers({ shouldAdvanceTime: true })
+    await renderLoadedCron()
+
+    const triggerButton = screen.getByRole('button', { name: 'Trigger now' })
+
+    await act(async () => {
+      triggerButton.click()
+      await Promise.resolve()
+    })
+
+    expect(queuedRow()).toBeTruthy()
+
+    // The trigger request itself is still pending (the real handler holds it
+    // for the whole run), so only the 90s deadline can settle the row.
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(90_000)
+    })
+
+    expect(queuedRow()).toBeNull()
+    expect((triggerButton as HTMLButtonElement).disabled).toBe(false)
+  })
+
+  it('removes the queued row when the trigger request fails', async () => {
+    await renderLoadedCron()
+    const triggerButton = screen.getByRole('button', { name: 'Trigger now' })
+
+    triggerCronJob.mockRejectedValue(new Error('scheduler unavailable'))
+
+    await act(async () => {
+      triggerButton.click()
+    })
+
+    await waitFor(() => expect(queuedRow()).toBeNull())
+    expect((triggerButton as HTMLButtonElement).disabled).toBe(false)
+  })
+
+  it('does not let pause clear the queued run while the backend is still working', async () => {
+    let releaseTrigger!: (value: CronJob) => void
+    triggerCronJob.mockReturnValue(new Promise<CronJob>(resolve => (releaseTrigger = resolve)))
+    const { container } = await renderLoadedCron()
+
+    await act(async () => {
+      screen.getByRole('button', { name: 'Trigger now' }).click()
+      await Promise.resolve()
+    })
+    expect(queuedRow()).toBeTruthy()
+
+    // The trigger is accepted but the run has not appeared; pausing the job
+    // persists enabled=false but never cancels an in-flight execution
+    // (cron/jobs.py pause path), so the queued row must survive it.
+    const pauseButton = screen.getByRole('button', { name: 'Pause' })
+
+    await act(async () => {
+      pauseButton.click()
+      await Promise.resolve()
+    })
+
+    expect(queuedRow()).toBeTruthy()
+    expect(container.querySelector('[data-slot="cron-run-pending"]')).toBeTruthy()
+
+    releaseTrigger(job)
+  })
+})
+
+describe('CronView trigger duplicate-click guard (regression for f9d64b9 contract)', () => {
+  it('coalesces a same-tick double click into one request', async () => {
+    let releaseTrigger!: (value: CronJob) => void
+    triggerCronJob.mockReturnValue(new Promise<CronJob>(resolve => (releaseTrigger = resolve)))
+    await renderLoadedCron()
+
+    const triggerButton = screen.getByRole('button', { name: 'Trigger now' })
+
+    await act(async () => {
+      triggerButton.click()
+      triggerButton.click()
+      await Promise.resolve()
+    })
+
+    expect(triggerCronJob).toHaveBeenCalledOnce()
+    expect((triggerButton as HTMLButtonElement).disabled).toBe(true)
+
+    await act(async () => {
+      releaseTrigger(job)
+    })
+  })
+})
+
+// Keep the atom import referenced even when no test reads it directly.
+void $cronJobs
+void $cronFocusJobId
--- a/apps/desktop/src/app/cron/index.tsx
+++ b/apps/desktop/src/app/cron/index.tsx
@@ -319,6 +319,11 @@ export function CronView({ onClose, onOpenSession, setStatusbarItemGroup: _setSt
  const [busyJobTokens, setBusyJobTokens] = useState<ReadonlyMap<string, symbol>>(() => new Map())
  const [triggeringJobKeys, setTriggeringJobKeys] = useState<ReadonlySet<string>>(() => new Set())
  const triggerControllerRef = useRef<CronTriggerController | null>(null)
+  // Accepted-but-unmaterialized triggers, by `${profile}:${jobId}`. The ref is
+  // written before the state set, so a queued row appears synchronously with
+  // the click and the controller guard has no untracked window.
+  const pendingTriggersRef = useRef(new Map<string, number>())
+  const [pendingTriggers, setPendingTriggers] = useState<ReadonlyMap<string, number>>(() => new Map())

  // eslint-disable-next-line no-restricted-syntax -- controller mount identity, not an atom mirror
  useEffect(() => {
@@ -504,6 +509,25 @@ export function CronView({ onClose, onOpenSession, setStatusbarItemGroup: _setSt
    }
  }

+  const settlePendingTrigger = useCallback((key: string, requestedAt: number) => {
+    if (pendingTriggersRef.current.get(key) !== requestedAt) {
+      return
+    }
+
+    pendingTriggersRef.current.delete(key)
+    setPendingTriggers(current => {
+      if (current.get(key) !== requestedAt) {
+        return current
+      }
+
+      const next = new Map(current)
+
+      next.delete(key)
+
+      return next
+    })
+  }, [])
+
  async function handleTrigger(job: CronJob) {
    const viewProfile = profile
    const key = `${viewProfile}:${job.id}`
@@ -513,6 +537,15 @@ export function CronView({ onClose, onOpenSession, setStatusbarItemGroup: _setSt
      return
    }

+    const requestedAt = Date.now()
+
+    // Optimistic queued-run feedback: the row is painted from the click, not
+    // from the backend materializing the session (which can take tens of
+    // seconds); it settles when Run History observes the run or times out.
+    // Ref first so the busy render and the controller guard share one instant.
+    pendingTriggersRef.current.set(key, requestedAt)
+    setPendingTriggers(current => new Map(current).set(key, requestedAt))
+
    try {
      const run = await controller.run(
        key,
@@ -544,6 +577,9 @@ export function CronView({ onClose, onOpenSession, setStatusbarItemGroup: _setSt
      if (triggerControllerRef.current === controller && cronProfileForScope($profileScope.get()) === viewProfile) {
        notifyError(err, c.failedTrigger)
      }
+
+      // The request never reached the backend; the queued row is a lie.
+      settlePendingTrigger(key, requestedAt)
    }
  }

@@ -717,7 +753,10 @@ export function CronView({ onClose, onOpenSession, setStatusbarItemGroup: _setSt
              onEdit={() => setEditor({ mode: 'edit', job: selectedJob })}
              onOpenSession={onOpenSession}
              onPauseResume={() => void handlePauseResume(selectedJob)}
+              onPendingRunSettled={settlePendingTrigger}
              onTrigger={() => void handleTrigger(selectedJob)}
+              pendingJobKey={`${profile}:${selectedJob.id}`}
+              pendingRunAt={pendingTriggers.get(`${profile}:${selectedJob.id}`)}
            />
          ) : query.trim() ? (
            // A search with no selected job: search-flavored copy is right.
@@ -806,10 +845,24 @@ interface CronJobDetailProps {
  onEdit: () => void
  onOpenSession?: (sessionId: string, session?: SessionInfo) => void
  onPauseResume: () => void
+  onPendingRunSettled: (key: string, requestedAt: number) => void
  onTrigger: () => void
+  pendingJobKey?: string
+  pendingRunAt?: number
 }

-function CronJobDetail({ busy, c, job, onEdit, onOpenSession, onPauseResume, onTrigger }: CronJobDetailProps) {
+function CronJobDetail({
+  busy,
+  c,
+  job,
+  onEdit,
+  onOpenSession,
+  onPauseResume,
+  onPendingRunSettled,
+  onTrigger,
+  pendingJobKey,
+  pendingRunAt
+}: CronJobDetailProps) {
  const state = jobState(job)
  const isPaused = state === 'paused'
  const deliver = jobDeliver(job)
@@ -831,7 +884,13 @@ function CronJobDetail({ busy, c, job, onEdit, onOpenSession, onPauseResume, onT
            <PanelAction disabled={busy} icon={isPaused ? 'play' : 'debug-pause'} onClick={onPauseResume}>
              {isPaused ? c.resumeTitle : c.pauseTitle}
            </PanelAction>
-            <PanelAction disabled={busy} icon="zap" onClick={onTrigger} primary>
+            <PanelAction
+              disabled={busy}
+              icon={pendingRunAt === undefined ? 'zap' : 'loading'}
+              onClick={onTrigger}
+              primary
+              spinning={pendingRunAt !== undefined}
+            >
              {c.triggerNow}
            </PanelAction>
          </div>
@@ -877,7 +936,14 @@ function CronJobDetail({ busy, c, job, onEdit, onOpenSession, onPauseResume, onT
        </section>
      ) : null}

-      <CronJobRuns c={c} jobId={job.id} onOpenSession={onOpenSession} />
+      <CronJobRuns
+        c={c}
+        jobId={job.id}
+        onOpenSession={onOpenSession}
+        onPendingRunSettled={onPendingRunSettled}
+        pendingJobKey={pendingJobKey}
+        pendingRunAt={pendingRunAt}
+      />
    </PanelDetail>
  )
 }
@@ -902,23 +968,40 @@ function isSyntheticCronOutputRun(run: SessionInfo): boolean {
 // Runs are produced by the background scheduler tick. cron.changed /
 // sessions.changed broadcasts re-load immediately on event-capable backends
 // (the tick dep below), so the poll drops to a slow backstop there; older
-// backends keep the legacy cadence.
+// backends keep the legacy cadence. While a trigger is queued, poll fast.
 const RUNS_POLL_INTERVAL_MS = 8000
 const RUNS_BACKSTOP_INTERVAL_MS = 60_000
+const PENDING_RUNS_POLL_INTERVAL_MS = 1000
+// A run created moments before the click (another surface's trigger, or a
+// scheduler tick racing the button) must not be mistaken for this click's run
+// — but clock skew between renderer and backend can date it slightly early.
+const PENDING_RUN_START_SLACK_MS = 2000
+// Bounded even if the backend never materializes the run (claimed by a window
+// that died, scheduler paused, …): the queued row is transient feedback, not a
+// persistent record.
+const PENDING_RUN_TIMEOUT_MS = 90_000

 function CronJobRuns({
  c,
  jobId,
-  onOpenSession
+  onOpenSession,
+  onPendingRunSettled,
+  pendingJobKey,
+  pendingRunAt
 }: {
  c: Translations['cron']
  jobId: string
  onOpenSession?: (sessionId: string, session?: SessionInfo) => void
+  onPendingRunSettled: (key: string, requestedAt: number) => void
+  pendingJobKey?: string
+  pendingRunAt?: number
 }) {
  const [runs, setRuns] = useState<null | SessionInfo[]>(null)
  const changeEventsAvailable = useStore($changeEventsAvailable)
  const cronChangeTick = useStore($cronChangeTick)

+  const pending = pendingRunAt !== undefined
+
  useEffect(() => {
    let cancelled = false

@@ -930,6 +1013,22 @@ function CronJobRuns({

          if (!cancelled) {
            setRuns(result)
+
+            // The queued row is settled by the run this trigger produced:
+            // any run that started after the click. Looking at the whole
+            // snapshot (not just a diff against the last poll) avoids
+            // missing a run that landed between two loads.
+            if (pendingRunAt !== undefined && pendingJobKey !== undefined) {
+              const started = result.some(run => {
+                const startedAtMs = (run.started_at || run.last_active || 0) * 1000
+
+                return startedAtMs >= pendingRunAt - PENDING_RUN_START_SLACK_MS
+              })
+
+              if (started) {
+                onPendingRunSettled(pendingJobKey, pendingRunAt)
+              }
+            }
          }
        })
        .catch(() => {
@@ -946,7 +1045,7 @@ function CronJobRuns({
          void load()
        }
      },
-      changeEventsAvailable ? RUNS_BACKSTOP_INTERVAL_MS : RUNS_POLL_INTERVAL_MS
+      pending ? PENDING_RUNS_POLL_INTERVAL_MS : changeEventsAvailable ? RUNS_BACKSTOP_INTERVAL_MS : RUNS_POLL_INTERVAL_MS
    )

    const onVisible = () => {
@@ -963,23 +1062,56 @@ function CronJobRuns({
      document.removeEventListener('visibilitychange', onVisible)
    }
    // cronChangeTick: a fired run moves jobs.json bookkeeping → reload now.
-  }, [changeEventsAvailable, cronChangeTick, jobId])
+  }, [changeEventsAvailable, cronChangeTick, jobId, onPendingRunSettled, pending, pendingJobKey, pendingRunAt])
+
+  // Bounded life for the queued row: even on an event-capable backend (where
+  // the fast poll above may be the only prober), the feedback disappears and
+  // the action unlocks once the wait is clearly unrecoverable.
+  useEffect(() => {
+    if (pendingRunAt === undefined || pendingJobKey === undefined) {
+      return
+    }
+
+    const remainingMs = Math.max(0, pendingRunAt + PENDING_RUN_TIMEOUT_MS - Date.now())
+    const timeoutId = window.setTimeout(() => onPendingRunSettled(pendingJobKey, pendingRunAt), remainingMs)
+
+    return () => window.clearTimeout(timeoutId)
+  }, [onPendingRunSettled, pendingJobKey, pendingRunAt])

  return (
    <div>
      <PanelSectionLabel className="mb-1.5">
        {c.runHistory}
-        {runs && runs.length > 0 ? ` · ${runs.length}` : ''}
+        {runs && runs.length + (pending ? 1 : 0) > 0 ? ` · ${runs.length + (pending ? 1 : 0)}` : ''}
      </PanelSectionLabel>
-      {runs === null ? (
+      {runs === null && !pending ? (
        <div className="flex items-center gap-1.5 py-1 text-xs text-muted-foreground">
          <Codicon name="loading" size="0.75rem" spinning />
        </div>
-      ) : runs.length === 0 ? (
+      ) : runs?.length === 0 && !pending ? (
        <div className="py-1 text-xs text-muted-foreground">{c.noRuns}</div>
      ) : (
        <div className="flex flex-col gap-px">
-          {runs.map(run =>
+          {pending && (
+            // The queued row is transient feedback for a trigger the backend
+            // accepted but has not turned into a session yet; it sits above
+            // the authoritative rows and is replaced once one appears.
+            <div
+              aria-live="polite"
+              className="flex items-center justify-between gap-3 rounded-md px-2 py-1 text-xs text-muted-foreground"
+              data-slot="cron-run-pending"
+              role="status"
+            >
+              <span className="flex min-w-0 items-center gap-1.5">
+                <Codicon name="loading" size="0.75rem" spinning />
+                <span className="truncate">{c.queuedRun}</span>
+              </span>
+              <span className="shrink-0 text-[0.62rem] text-muted-foreground/55 tabular-nums">
+                {formatRunTime(pendingRunAt / 1000)}
+              </span>
+            </div>
+          )}
+          {(runs ?? []).map(run =>
            isSyntheticCronOutputRun(run) ? (
              // Output-doc rows have no backing session to open; show the
              // recorded output preview without a chat-navigation affordance.
--- a/apps/desktop/src/app/overlays/panel.tsx
+++ b/apps/desktop/src/app/overlays/panel.tsx
@@ -404,16 +404,19 @@ export function PanelAction({
  disabled,
  icon,
  onClick,
-  primary
+  primary,
+  spinning
 }: {
  children: ReactNode
  disabled?: boolean
  icon: string
  onClick: () => void
  primary?: boolean
+  spinning?: boolean
 }) {
  return (
    <Button
+      aria-busy={spinning || undefined}
      className={cn(
        'gap-1.5',
        !primary && 'text-muted-foreground hover:bg-(--ui-row-hover-background) hover:text-foreground'
@@ -423,7 +426,7 @@ export function PanelAction({
      size="sm"
      variant={primary ? 'default' : 'ghost'}
    >
-      <Codicon name={icon} size="0.875rem" />
+      <Codicon name={icon} size="0.875rem" spinning={spinning} />
      {children}
    </Button>
  )
--- a/apps/desktop/src/i18n/en.ts
+++ b/apps/desktop/src/i18n/en.ts
@@ -3114,6 +3114,9 @@ export const en: Translations = {
    // Replaces `next` when the stored next_run_at is already past the scheduler grace (#114309).
    overdueSince: 'Overdue since:',
    noRuns: 'No runs yet',
+    // Queued trigger feedback in Run History: the backend accepted the trigger
+    // but has not materialized the run session yet (#70826).
+    queuedRun: 'Queued run',
    manage: 'Manage',
    showRuns: 'Show runs',
    hideRuns: 'Hide runs',
--- a/apps/desktop/src/i18n/types.ts
+++ b/apps/desktop/src/i18n/types.ts
@@ -2707,6 +2707,7 @@ export interface Translations extends NoticeTranslations {
    next: string
    overdueSince: string
    noRuns: string
+    queuedRun: string
    manage: string
    showRuns: string
    hideRuns: string