From 87ede54eb807d06697c45651b43712d167d3ea04 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:31:32 -0700 Subject: [PATCH] Show all automation destination hosts, disable ineligible ones (#16665) * Show all automation destination hosts, disable ineligible ones Previously, filtering to only eligible hosts hid all connected hosts on pre-host-scoping Orca servers. Now all offered hosts appear in the picker; ineligible ones are disabled with a message naming which servers need updating to support them. * Show all automation destination hosts, keep create available when all ar - Gate the create button on what the picker offers, not on readiness: with every offered host ineligible (e.g. all pre-host-scoping servers), the dialog is where the repair is stated, so the button must still open it. - Fix StrictMode double-mount lifecycle: disposed controller revived by effect, unsubscribe before dispose to prevent event leaks under simulated unmount. - Validate create destination early, before hooks load and trust prompt, so the user never answers a trust dialog for a destination that would reject. - Dedupe capability probes per authority incarnation: concurrent callers share one in-flight status.get; confirmed capabilities never re-probed. - Drop cache payload at retirement so revived hosts refetch instead of showing stale rows. * Add TTL-based capability probe caching and fix automation dialog target Extract capability probing to a separate module with improved caching strategy: confirmations now expire after 60 seconds and the cache is bounded to 32 entries, enabling in-place runtime replacements to invalidate old confirmations. For uncaptured automation owners, resolve the dialog target to the same host the save addresses rather than relying on ambient context, preventing stale host references. Optionally await external managers after mutations to ensure row re-reads reflect recent writes. * Fix automation tests after rebase * Refactor capability probe to fence fencing checks from in-flight probes Fencing checks need fresh probes since in-flight probes may predate in-place runtime replacements. Extract shared probe deduplication into `sharedCapabilityProbe()` and unconditional probe start into `startCapabilityProbe()`, then route based on cache preference. --- .../AutomationCreateDestinationField.test.tsx | 41 ++++- .../AutomationCreateDestinationField.tsx | 27 +++- ...utomationsPage.create-destination.test.tsx | 85 +++++++++- ...sPage.strict-mode-save-visibility.test.tsx | 150 +++++++++++++++++ .../automations/AutomationsPage.test.tsx | 13 ++ .../automations/AutomationsPage.tsx | 152 ++++++++++++------ .../automation-capability-probe.ts | 135 ++++++++++++++++ .../automation-create-admission.ts | 22 --- .../automation-create-destination.test.ts | 72 +++++++++ .../automation-create-destination.ts | 30 ++++ .../automation-external-target-match.test.ts | 36 +---- .../automation-external-target-match.ts | 9 -- .../automation-host-cache-controller.ts | 11 +- .../automations/automation-host-cache.test.ts | 12 ++ .../automations/automation-host-cache.ts | 7 + .../automation-host-client.test.ts | 24 ++- .../automations/automation-host-client.ts | 18 +-- .../automation-host-diagnostics.test.ts | 17 +- .../automation-host-diagnostics.ts | 5 +- .../automation-host-scale-gates.test.ts | 45 ++++-- .../automation-scoped-list-client.test.ts | 111 ++++++++++++- .../automation-scoped-list-client.ts | 78 ++------- .../automation-target-availability.test.ts | 16 -- .../automations-page-runtime-fixtures.ts | 13 ++ .../automations-page-test-harness.tsx | 30 ++-- .../use-automation-host-catalog.ts | 39 ++++- src/renderer/src/i18n/locales/en.json | 4 +- 27 files changed, 915 insertions(+), 287 deletions(-) create mode 100644 src/renderer/src/components/automations/AutomationsPage.strict-mode-save-visibility.test.tsx create mode 100644 src/renderer/src/components/automations/automation-capability-probe.ts delete mode 100644 src/renderer/src/components/automations/automation-create-admission.ts diff --git a/src/renderer/src/components/automations/AutomationCreateDestinationField.test.tsx b/src/renderer/src/components/automations/AutomationCreateDestinationField.test.tsx index 1df0a7519bf..e32e6500d5f 100644 --- a/src/renderer/src/components/automations/AutomationCreateDestinationField.test.tsx +++ b/src/renderer/src/components/automations/AutomationCreateDestinationField.test.tsx @@ -42,9 +42,29 @@ const ENTRY: AutomationHostCatalogEntry = { querySupport: 'scoped' } -function control(projects: Repo[]): AutomationCreateDestinationControl { +const LEGACY_ENTRY: AutomationHostCatalogEntry = { + stableRef: { authority: { kind: 'runtime', environmentId: 'r1' }, selector: { kind: 'self' } }, + owner: { + authority: { kind: 'runtime', environmentId: 'r1', pairingRevision: 1 }, + selector: { kind: 'self' } + }, + stableKey: 'host:runtime:r1:self', + label: 'legacy-box', + authorityLabel: 'legacy-box', + kind: 'self', + catalogState: 'authoritative', + authorityHealth: 'fresh', + executionHealth: 'connected', + querySupport: 'legacy-unscoped', + scopeGap: 'authority-unscoped' +} + +function control( + projects: Repo[], + entries: AutomationHostCatalogEntry[] = [ENTRY] +): AutomationCreateDestinationControl { return { - entries: [ENTRY], + entries, resolution: { status: 'ready', authority: OWNER.authority, @@ -56,11 +76,11 @@ function control(projects: Repo[]): AutomationCreateDestinationControl { } } -function render(projects: Repo[]): void { +function render(projects: Repo[], entries?: AutomationHostCatalogEntry[]): void { act(() => { root.render( - + ) }) @@ -83,4 +103,17 @@ describe('AutomationCreateDestinationField', () => { expect(emptyNote()).toBeNull() expect(container.textContent).toContain('Local Mac') }) + + it('names the hosts a server update would let create, instead of hiding them', () => { + render([{ id: 'repo-1' } as Repo], [ENTRY, LEGACY_ENTRY]) + + const note = container.querySelector('[data-testid="automation-create-update-required"]') + expect(note?.textContent).toContain('legacy-box') + }) + + it('says nothing about updates when every offered host is eligible', () => { + render([{ id: 'repo-1' } as Repo]) + + expect(container.querySelector('[data-testid="automation-create-update-required"]')).toBeNull() + }) }) diff --git a/src/renderer/src/components/automations/AutomationCreateDestinationField.tsx b/src/renderer/src/components/automations/AutomationCreateDestinationField.tsx index 8ba017b0ff7..43a4f2cc08c 100644 --- a/src/renderer/src/components/automations/AutomationCreateDestinationField.tsx +++ b/src/renderer/src/components/automations/AutomationCreateDestinationField.tsx @@ -11,7 +11,11 @@ import { import { translate } from '@/i18n/i18n' import { AutomationHostLabel, AutomationHostStatusBadges } from './AutomationHostBadges' import { Field } from './automation-page-parts' -import { automationCreateHostEligible } from './automation-create-destination' +import { + automationCreateHostEligible, + automationCreateHostOffered, + automationCreateUpdateRequiredAuthorityLabels +} from './automation-create-destination' import { groupAutomationHostEntriesByAuthority } from './automation-host-picker-groups' import type { AutomationCreateDestinationControl } from './use-automation-create-destination' @@ -32,11 +36,12 @@ export function AutomationCreateDestinationField({ labelClassName?: string }): React.JSX.Element { const selected = control.resolution.status === 'ready' ? control.resolution.entry : null - // Offered and resolved by the same predicate, so the list never contains a - // host that selecting would refuse (e.g. a view-only entry). + // Ineligible hosts stay listed but disabled: hiding them read as the host + // being gone, and it hid every connected host on a pre-host-scoping server. const groups = groupAutomationHostEntriesByAuthority( - control.entries.filter(automationCreateHostEligible) + control.entries.filter(automationCreateHostOffered) ) + const updateRequiredAuthorities = automationCreateUpdateRequiredAuthorityLabels(control.entries) const label = translate('auto.components.automations.createDestination.label', 'Create on') return ( @@ -58,6 +63,7 @@ export function AutomationCreateDestinationField({ @@ -91,6 +97,19 @@ export function AutomationCreateDestinationField({ )}

)} + {updateRequiredAuthorities.length > 0 ? ( + // A disabled row's tooltip is unreachable, so the repair is stated here. +

+ {translate( + 'auto.components.automations.createDestination.updateRequired', + 'Update the Orca server on {hosts} to create automations there.' + // Replacer fn: a literal replacement would expand `$` patterns in host labels. + ).replace('{hosts}', () => updateRequiredAuthorities.join(', '))} +

+ ) : null} ) } diff --git a/src/renderer/src/components/automations/AutomationsPage.create-destination.test.tsx b/src/renderer/src/components/automations/AutomationsPage.create-destination.test.tsx index d13ab127e21..da04a8cd58f 100644 --- a/src/renderer/src/components/automations/AutomationsPage.create-destination.test.tsx +++ b/src/renderer/src/components/automations/AutomationsPage.create-destination.test.tsx @@ -11,7 +11,7 @@ */ import { act } from 'react' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { Automation } from '../../../../shared/automations-types' import { hostStableKey } from '../../../../shared/automation-owner-key' import { unscopedAutomationListRows, type AutomationListRow } from './automation-list-row-identity' @@ -131,7 +131,8 @@ describe('AutomationsPage create destination', () => { expect(api.automations.create).not.toHaveBeenCalled() expect(runtimeCreateCalls()).toHaveLength(1) expect(runtimeCreateCalls()[0]?.[2]).toMatchObject({ - repo: RUNTIME_REPO_ID, + repo: `id:${RUNTIME_REPO_ID}`, + workspace: `id:${RUNTIME_WORKSPACE_ID}`, destination: { selector: { kind: 'self' } } }) }) @@ -152,7 +153,7 @@ describe('AutomationsPage create destination', () => { expect(api.automations.create).not.toHaveBeenCalled() expect(runtimeCreateCalls()).toHaveLength(0) - expect(mocks.toastError).toHaveBeenCalled() + expect(mocks.editorDialog?.notice?.message).toBeTruthy() expect(mocks.editorDialog?.open).toBe(true) }) @@ -169,7 +170,7 @@ describe('AutomationsPage create destination', () => { // A repo with no connection ID is not evidence of local: this one is the // runtime's, and the desktop's Self host cannot hold an automation for it. expect(api.automations.create).not.toHaveBeenCalled() - expect(mocks.toastError).toHaveBeenCalled() + expect(mocks.editorDialog?.notice?.message).toBeTruthy() expect(mocks.editorDialog?.open).toBe(true) }) @@ -368,3 +369,79 @@ describe('AutomationsPage edit fencing', () => { expect(mocks.editorDialog?.notice?.message).toBeTruthy() }) }) + +describe('AutomationsPage edit dialog projects', () => { + it('offers the edited row’s own host projects, not the create destination’s', async () => { + const automation = makeAutomation({ + id: 'a-runtime', + projectId: RUNTIME_REPO_ID, + workspaceId: RUNTIME_WORKSPACE_ID + }) + // The ambient list is the desktop's and never held this record, so an + // id lookup there answers with nothing and the row's owner is all there is. + api.automations.list.mockResolvedValue([]) + scopedList([]) + runtimeHost([automation], []) + addRuntimeProject() + + await renderPage() + await settleHostQueries() + await act(async () => { + void mocks.listPanel?.openEditDialog(listedRow(automation.id)) + }) + + expect(mocks.editorDialog?.isEditing).toBe(true) + expect(mocks.editorDialog?.repos?.map((repo) => repo.id)).toEqual([RUNTIME_REPO_ID]) + }) +}) + +describe('AutomationsPage create admission', () => { + it('keeps the create button available while every offered host is ineligible', async () => { + // No host has answered, so nothing resolves ready — but the dialog is where + // an ineligible host's repair is stated, so the button must still open it. + api.automations.listScoped.mockRejectedValue(new Error('offline')) + api.automations.list.mockResolvedValue([]) + + await renderPage() + await settleHostQueries() + + expect(mocks.listPanel?.canCreateAutomation).toBe(true) + }) + + it('refuses a mismatched create destination before the hooks trust prompt', async () => { + // A runtime-owned project under the sole desktop destination: refused, and + // refused before the user is asked to trust that project's setup hooks. + api.automations.list.mockResolvedValue([]) + scopedList([]) + addRuntimeProject() + // A setup hook whose default policy is run-by-default, so the old save + // order would have raised the trust prompt before refusing the create. + const { checkRuntimeHooks } = await import('@/runtime/runtime-hooks-client') + vi.mocked(checkRuntimeHooks).mockResolvedValue({ + status: 'ok', + hooks: { scripts: { setup: 'pnpm install' } } + } as never) + + await renderPage() + await settleHostQueries() + await act(async () => { + mocks.listPanel?.openCreateDialog() + }) + await act(async () => { + mocks.editorDialog?.onDraftChange((current) => ({ + ...(current as Record), + name: 'Sweep', + prompt: 'Do the sweep', + projectId: RUNTIME_REPO_ID, + workspaceMode: 'new_per_run', + workspaceId: '' + })) + }) + await save() + + expect(mocks.editorDialog?.notice?.message).toBeTruthy() + const { ensureHooksConfirmed } = await import('@/lib/ensure-hooks-confirmed') + expect(vi.mocked(ensureHooksConfirmed)).not.toHaveBeenCalled() + expect(api.automations.create).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/automations/AutomationsPage.strict-mode-save-visibility.test.tsx b/src/renderer/src/components/automations/AutomationsPage.strict-mode-save-visibility.test.tsx new file mode 100644 index 00000000000..e1387e88631 --- /dev/null +++ b/src/renderer/src/components/automations/AutomationsPage.strict-mode-save-visibility.test.tsx @@ -0,0 +1,150 @@ +// @vitest-environment happy-dom + +/** + * Save visibility under a StrictMode mount, which is how the dev app runs. + * + * StrictMode's simulated unmount disposes the host query controller after its + * first effect cycle. A controller that stays disposed drops every later + * invalidation — the list keeps its initial rows and a create never appears + * until the app is reloaded. These pin the revive: after the double mount, a + * write and the authority's change event must still refetch the written host. + */ + +import { act } from 'react' +import { describe, expect, it } from 'vitest' +import type { Automation } from '../../../../shared/automations-types' +import { hostStableKey } from '../../../../shared/automation-owner-key' +import { AUTOMATIONS_CHANGED_EVENT } from '@/lib/automations-changed-window-event' +import { + addRuntimeProject, + api, + installAutomationsPageHarness, + mocks, + renderPage, + rows, + runtimeHost, + RUNTIME_ID, + RUNTIME_REPO_ID, + RUNTIME_WORKSPACE_ID, + scopedList, + settleHostQueries +} from './automations-page-test-harness' +import { makeAutomation, REPO_ID, WORKSPACE_ID } from './automations-page-fixtures' + +installAutomationsPageHarness() + +const CREATED = makeAutomation({ id: 'a-new', name: 'Sweep' }) + +const RUNTIME_SELF_KEY = hostStableKey({ + authority: { kind: 'runtime', environmentId: RUNTIME_ID }, + selector: { kind: 'self' } +}) + +function desktopStoreHolds(automations: Automation[]): void { + api.automations.list.mockResolvedValue(automations) + scopedList(automations) +} + +async function createSweep(): Promise { + await act(async () => { + mocks.listPanel?.openCreateDialog() + }) + await act(async () => { + mocks.editorDialog?.onDraftChange((current) => ({ + ...(current as Record), + name: 'Sweep', + prompt: 'Do the sweep', + projectId: REPO_ID, + workspaceMode: 'existing', + workspaceId: WORKSPACE_ID + })) + }) + await act(async () => { + await mocks.editorDialog?.onSave() + }) + await settleHostQueries() +} + +describe('AutomationsPage save visibility under StrictMode', () => { + it('lists a newly created automation without a reload', async () => { + desktopStoreHolds([]) + api.automations.create.mockResolvedValue(CREATED) + + const { container } = await renderPage({ strict: true }) + await settleHostQueries() + expect(rows(container, 'automation-row')).toEqual([]) + + desktopStoreHolds([CREATED]) + await createSweep() + + expect(api.automations.create).toHaveBeenCalledTimes(1) + expect(rows(container, 'automation-row')).toEqual(['Sweep']) + }) + + it('still refetches when the authority publishes its change event', async () => { + desktopStoreHolds([]) + + const { container } = await renderPage({ strict: true }) + await settleHostQueries() + expect(rows(container, 'automation-row')).toEqual([]) + + // A write that lands elsewhere (CLI, another window) only reaches this page + // through the event; a disposed controller would drop it on the floor. + desktopStoreHolds([CREATED]) + await act(async () => { + window.dispatchEvent( + new CustomEvent(AUTOMATIONS_CHANGED_EVENT, { + detail: { reason: 'definition', selector: { kind: 'self' } } + }) + ) + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + expect(rows(container, 'automation-row')).toEqual(['Sweep']) + }) + + it('lists a create on a runtime-host destination without a reload', async () => { + api.automations.list.mockResolvedValue([]) + scopedList([]) + runtimeHost([], []) + addRuntimeProject() + const previous = mocks.callRuntimeRpc.getMockImplementation() + mocks.callRuntimeRpc.mockImplementation( + async (target: unknown, method: string, params: unknown, options: unknown) => { + if (method === 'automation.create') { + // The runtime store now holds the row; later list reads must say so. + mocks.state.runtimeAnswers = { automations: [CREATED], runs: [] } + return { automation: CREATED } + } + return await previous?.(target, method, params, options) + } + ) + + const { container } = await renderPage({ strict: true }) + await settleHostQueries() + expect(rows(container, 'automation-row')).toEqual([]) + + await act(async () => { + mocks.listPanel?.openCreateDialog() + }) + await act(async () => { + mocks.editorDialog?.createDestination?.onSelect(RUNTIME_SELF_KEY) + }) + await act(async () => { + mocks.editorDialog?.onDraftChange((current) => ({ + ...(current as Record), + name: 'Sweep', + prompt: 'Do the sweep', + projectId: RUNTIME_REPO_ID, + workspaceMode: 'existing', + workspaceId: RUNTIME_WORKSPACE_ID + })) + }) + await act(async () => { + await mocks.editorDialog?.onSave() + }) + await settleHostQueries() + + expect(rows(container, 'automation-row')).toEqual(['Sweep']) + }) +}) diff --git a/src/renderer/src/components/automations/AutomationsPage.test.tsx b/src/renderer/src/components/automations/AutomationsPage.test.tsx index 1efff6d98f8..768d3250d80 100644 --- a/src/renderer/src/components/automations/AutomationsPage.test.tsx +++ b/src/renderer/src/components/automations/AutomationsPage.test.tsx @@ -626,3 +626,16 @@ describe('AutomationsPage owner conflicts', () => { expect(mocks.listPanel?.isActionEnabled(listedRow(automation.id), 'delete')).toBe(true) }) }) + +describe('AutomationsPage refresh independence', () => { + // The external-manager probe is per host and can hang on a dead provider; the + // automation list must settle without waiting for it. + it('settles the list even when the external manager probe never answers', async () => { + api.automations.listExternalManagerForOwner.mockReturnValue(new Promise(() => undefined)) + + const { container } = await renderPage() + + expect(rows(container, 'automation-row')).toEqual(['Nightly']) + expect((mocks.listPanel as { isRefreshing?: boolean } | null)?.isRefreshing).toBe(false) + }) +}) diff --git a/src/renderer/src/components/automations/AutomationsPage.tsx b/src/renderer/src/components/automations/AutomationsPage.tsx index 0180ad2ac52..7c908d41af1 100644 --- a/src/renderer/src/components/automations/AutomationsPage.tsx +++ b/src/renderer/src/components/automations/AutomationsPage.tsx @@ -61,7 +61,6 @@ import { } from './automation-setup-decision' import type { AutomationTemplate } from './automation-templates' import { getAutomationTargetAvailability } from './automation-target-availability' -import { getAutomationCreateAvailability } from './automation-create-admission' import { buildAutomationRunContextForRepo } from './automation-run-context' import { repoMatchesExternalAutomationTarget } from './automation-external-target-match' import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed' @@ -140,6 +139,7 @@ import { groupReposByAutomationAuthority } from './automation-authority-identity' import { + automationCreateHostOffered, automationCreateHostStableKey, resolveAutomationCreateDestination, revalidateAutomationCreateDestination, @@ -680,9 +680,9 @@ export default function AutomationsPage(): React.JSX.Element { // The row's own host, from its captured owner. A page-level target cannot // speak for a list spanning authorities, and the legacy arm below it is only // ever reached by rows the desktop's unscoped list produced. - const automationHostTargetFor = useCallback( - (row: AutomationListRow): AutomationHostTarget | null => { - const owner = capturedAutomationOwner(capturedAutomationOwners, row.key).owner + const automationHostTargetForRowKey = useCallback( + (rowKey: string | null): AutomationHostTarget | null => { + const owner = capturedAutomationOwner(capturedAutomationOwners, rowKey).owner if (owner?.authority.kind === 'runtime') { return { kind: 'environment', environmentId: owner.authority.environmentId } } @@ -690,6 +690,10 @@ export default function AutomationsPage(): React.JSX.Element { }, [automationHostTarget, capturedAutomationOwners] ) + const automationHostTargetFor = useCallback( + (row: AutomationListRow): AutomationHostTarget | null => automationHostTargetForRowKey(row.key), + [automationHostTargetForRowKey] + ) const automationDispatchContext = useMemo( () => ({ capturedOwners: capturedAutomationOwners, authority: automationAuthority }), [automationAuthority, capturedAutomationOwners] @@ -981,17 +985,41 @@ export default function AutomationsPage(): React.JSX.Element { } }, [activeWorktreeId, editorProjects, repoMap, worktreeMap, worktreesByRepo]) - const canCreateAutomation = getAutomationCreateAvailability({ - automationHostTarget: getAutomationListTarget(settings), - runtimeStatusByEnvironmentId - }).canRunNow - const editingAutomation = editingAutomationId - ? (automations.find((automation) => automation.id === editingAutomationId) ?? null) + // Gated on what the picker offers, not on eligibility: with every offered + // host ineligible (e.g. all pre-host-scoping servers), the dialog is where + // the repair is stated, so the button must still open it. + const canCreateAutomation = hostCatalog.entries.some(automationCreateHostOffered) + // The edited row's own captured owner names the host, not the ambient list + // target: a remote row need not appear in `automations` at all, and looking it + // up by id there would answer with whichever authority the page last listed. + // An uncaptured row resolves to the same host its save addresses. + const editingRow = editingRowKey + ? (visibleRows.find((row) => row.key === editingRowKey) ?? null) : null - const automationDialogTarget = editingAutomation - ? getAutomationOwnerTarget(editingAutomation, automationHostTarget) - : getAutomationListTarget(settings) + const editingRowCapturedOwner = capturedAutomationOwner( + capturedAutomationOwners, + editingRowKey + ).owner + const automationDialogTarget = ((): AutomationHostTarget => { + if (editingAutomationId === null) { + return getAutomationListTarget(settings) + } + // Uncaptured: the host the legacy save addresses, so a runtime row the + // desktop list produced stops offering desktop projects. + if (editingRow && !editingRowCapturedOwner) { + return getAutomationOwnerTarget(editingRow.automation, automationHostTarget) + } + return ( + automationHostTargetForRowKey(editingRowKey) ?? + getAutomationTargetFromHostId(editingRow?.automation.runContext?.hostId) + ) + })() const isOrcaForm = createTarget === 'orca' && editingExternalTarget === null + const dialogRepos = isOrcaForm + ? editingAutomationId !== null + ? getAutomationCreateRepos(repos, automationDialogTarget) + : editorProjects + : getAutomationCreateRepos(repos, { kind: 'local' }) const destinationForProject = useCallback( (projectId: string): AutomationCreateDestination | null => { @@ -1015,40 +1043,47 @@ export default function AutomationsPage(): React.JSX.Element { const reloadExternalManagers = scopedExternal.reload - const refresh = useCallback(async () => { - setIsLoading(true) - const pendingNavigation = useAppStore.getState().pendingAutomationRunNavigation - // The desktop unless navigation named a host: this arm exists for rows the - // per-host reads have not answered for, and the client's own authority is - // the only one it can address without guessing which server is meant. - const automationHostTarget: AutomationHostTarget = pendingNavigation - ? getAutomationTargetFromHostId(pendingNavigation.hostId) - : { kind: 'local' } - const authorityKey = automationAuthorityCatalogKey( - automationHostTarget.kind === 'environment' - ? { kind: 'runtime', environmentId: automationHostTarget.environmentId } - : { kind: 'desktop' } - ) - try { - const [nextAutomations] = await Promise.all([ - listAutomationsForTarget(automationHostTarget), - // Managers are per host and failures are per provider, so this settles - // on its own and must not be able to fail the automation list with it. - reloadExternalManagers().catch(() => undefined) - ]) - // Selection and run history are deliberately not written here: this call - // addressed one authority, and the selected row may belong to another. - setAutomations(nextAutomations) - setAutomationHostTargetKey(getAutomationHostTargetKey(automationHostTarget)) - setFailedAuthorityKeys((current) => withoutKey(current, authorityKey)) - } catch { - // Why not a toast and not a rethrow: the list keeps whatever it had, and - // the host's own status row is where the failure and its Retry belong. - setFailedAuthorityKeys((current) => new Set(current).add(authorityKey)) - } finally { - setIsLoading(false) - } - }, [reloadExternalManagers]) + const refresh = useCallback( + async (options?: { awaitExternalManagers?: boolean }) => { + setIsLoading(true) + const pendingNavigation = useAppStore.getState().pendingAutomationRunNavigation + // The desktop unless navigation named a host: this arm exists for rows the + // per-host reads have not answered for, and the client's own authority is + // the only one it can address without guessing which server is meant. + const automationHostTarget: AutomationHostTarget = pendingNavigation + ? getAutomationTargetFromHostId(pendingNavigation.hostId) + : { kind: 'local' } + const authorityKey = automationAuthorityCatalogKey( + automationHostTarget.kind === 'environment' + ? { kind: 'runtime', environmentId: automationHostTarget.environmentId } + : { kind: 'desktop' } + ) + // Managers are per host and failures are per provider: the probe settles + // into its own state on its own time, and must neither fail the automation + // list nor keep the list loading while a slow provider answers. An explicit + // external mutation opts in below, so the row set it re-reads reflects the + // write before its success toast lands. + const managersSettled = reloadExternalManagers().catch(() => undefined) + try { + const nextAutomations = await listAutomationsForTarget(automationHostTarget) + // Selection and run history are deliberately not written here: this call + // addressed one authority, and the selected row may belong to another. + setAutomations(nextAutomations) + setAutomationHostTargetKey(getAutomationHostTargetKey(automationHostTarget)) + setFailedAuthorityKeys((current) => withoutKey(current, authorityKey)) + } catch { + // Why not a toast and not a rethrow: the list keeps whatever it had, and + // the host's own status row is where the failure and its Retry belong. + setFailedAuthorityKeys((current) => new Set(current).add(authorityKey)) + } finally { + setIsLoading(false) + } + if (options?.awaitExternalManagers) { + await managersSettled + } + }, + [reloadExternalManagers] + ) useEffect(() => { if (!pendingAutomationRunNavigation || isLoading) { @@ -1514,7 +1549,7 @@ export default function AutomationsPage(): React.JSX.Element { if (!editingExternalTarget) { useAppStore.getState().recordFeatureInteraction('automation-created') } - await refresh() + await refresh({ awaitExternalManagers: true }) setCreateOpen(false) setEditingExternalTarget(null) // Same helper and same captured scope the row's key was built from, so the @@ -1537,7 +1572,11 @@ export default function AutomationsPage(): React.JSX.Element { ) return } - if (isOrcaForm && !editorProjects.some((repo) => repo.id === draft.projectId)) { + if ( + editingAutomationId !== null && + isOrcaForm && + !dialogRepos.some((repo) => repo.id === draft.projectId) + ) { toast.error( translate( 'auto.components.automations.AutomationsPage.destinationProjectUnavailable', @@ -1546,6 +1585,17 @@ export default function AutomationsPage(): React.JSX.Element { ) return } + // Refused here, before the side-effectful steps below (hooks load, trust + // prompt): the user must not answer a trust dialog for a create that the + // destination was always going to reject. `createDraftAutomation` checks + // again after those awaits, which is the fence that actually gates the send. + if (editingAutomationId === null) { + const earlyDestination = createDestination.check(draft.projectId) + if (!earlyDestination.ok) { + setEditorNotice(earlyDestination.notice) + return + } + } const now = Date.now() const timezone = Intl.DateTimeFormat().resolvedOptions().timeZone const rrule = @@ -1962,7 +2012,7 @@ export default function AutomationsPage(): React.JSX.Element { if (action === 'run') { useAppStore.getState().recordFeatureInteraction('automation-run') } - await refresh() + await refresh({ awaitExternalManagers: true }) // Why: full-page detail keeps selection when the deleted external was open; // without this, detail can fall through to an unrelated local automation. if (action === 'delete') { @@ -2224,7 +2274,7 @@ export default function AutomationsPage(): React.JSX.Element { canSave={canSaveDraft} isEditingExternal={editingExternalTarget !== null} createTarget={createTarget} - repos={editorProjects} + repos={dialogRepos} projectHostSetups={projectHostSetups} automationYamlHooksByRepoKey={automationYamlHooksByRepoKey} getAutomationHooksCacheKey={getAutomationHooksCacheKey} diff --git a/src/renderer/src/components/automations/automation-capability-probe.ts b/src/renderer/src/components/automations/automation-capability-probe.ts new file mode 100644 index 00000000000..38cf3c69326 --- /dev/null +++ b/src/renderer/src/components/automations/automation-capability-probe.ts @@ -0,0 +1,135 @@ +import { getRuntimeEnvironmentStatus } from '@/runtime/runtime-rpc-client' +import type { AutomationAuthorityRef } from '../../../../shared/automation-owner-ref' +import { + AUTOMATION_LIST_HOST_SCOPE_RUNTIME_CAPABILITY, + AUTOMATION_LIST_HOST_SCOPE_UPDATE_REQUIRED_MESSAGE, + AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY, + AUTOMATION_OWNER_FENCING_UPDATE_REQUIRED_MESSAGE, + type RuntimeCapability +} from '../../../../shared/protocol-version' +import { automationAuthorityCatalogKey } from './automation-host-catalog-types' +import { automationHostDiagnostics } from './automation-host-diagnostics' + +export const REQUEST_TIMEOUT_MS = 15_000 + +export class AutomationHostScopeUnsupportedError extends Error { + readonly code = 'unsupported_host_scope' + + constructor(message: string) { + super(message) + this.name = 'AutomationHostScopeUnsupportedError' + } +} + +export const AUTHORITY_CAPABILITY_CONFIRMATION_TTL_MS = 60_000 +export const AUTHORITY_CAPABILITY_CONFIRMATION_MAX = 32 +const confirmedAuthorityCapabilities = new Map< + string, + { capabilities: Set; confirmedAt: number } +>() +const inFlightCapabilityProbes = new Map>() + +function capabilityProbeKey(authority: AutomationAuthorityRef & { kind: 'runtime' }): string { + return `${authority.environmentId}:${authority.pairingRevision}` +} + +function rememberAuthorityCapabilities( + key: string, + capabilities: Set, + confirmedAt: number +): void { + confirmedAuthorityCapabilities.delete(key) + confirmedAuthorityCapabilities.set(key, { capabilities, confirmedAt }) + while (confirmedAuthorityCapabilities.size > AUTHORITY_CAPABILITY_CONFIRMATION_MAX) { + const oldest = confirmedAuthorityCapabilities.keys().next() + if (oldest.done) { + break + } + confirmedAuthorityCapabilities.delete(oldest.value) + } +} + +/** Test seam: capability state is module-level and must not leak between tests. */ +export function resetAutomationCapabilityProbes(): void { + confirmedAuthorityCapabilities.clear() + inFlightCapabilityProbes.clear() +} + +export async function assertAuthorityCapability( + authority: AutomationAuthorityRef, + capability: RuntimeCapability, + message: string, + options: { cacheConfirmation?: boolean } = {} +): Promise { + if (authority.kind !== 'runtime') { + return + } + const useCache = options.cacheConfirmation !== false + const key = capabilityProbeKey(authority) + if (useCache) { + const confirmation = confirmedAuthorityCapabilities.get(key) + if ( + confirmation && + Date.now() - confirmation.confirmedAt >= AUTHORITY_CAPABILITY_CONFIRMATION_TTL_MS + ) { + confirmedAuthorityCapabilities.delete(key) + } else if (confirmation?.capabilities.has(capability)) { + return + } + } + // Fencing checks never join an in-flight probe: it may predate an in-place runtime replacement. + const status = useCache + ? await sharedCapabilityProbe(authority, key) + : await startCapabilityProbe(authority) + if (useCache && status.capabilities?.length) { + rememberAuthorityCapabilities(key, new Set(status.capabilities), Date.now()) + } + if (!status.capabilities?.includes(capability)) { + throw new AutomationHostScopeUnsupportedError(message) + } +} + +function startCapabilityProbe( + authority: AutomationAuthorityRef & { kind: 'runtime' } +): Promise<{ capabilities?: string[] }> { + automationHostDiagnostics.recordCapabilityProbe({ + authorityKey: automationAuthorityCatalogKey(authority) + }) + return getRuntimeEnvironmentStatus(authority.environmentId, REQUEST_TIMEOUT_MS) +} + +function sharedCapabilityProbe( + authority: AutomationAuthorityRef & { kind: 'runtime' }, + key: string +): Promise<{ capabilities?: string[] }> { + const existing = inFlightCapabilityProbes.get(key) + if (existing) { + return existing + } + const started = startCapabilityProbe(authority) + inFlightCapabilityProbes.set(key, started) + void started + .catch(() => undefined) + .finally(() => { + if (inFlightCapabilityProbes.get(key) === started) { + inFlightCapabilityProbes.delete(key) + } + }) + return started +} + +export async function assertOwnerFencingSupported( + authority: AutomationAuthorityRef +): Promise { + await assertAuthorityCapability( + authority, + AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY, + AUTOMATION_OWNER_FENCING_UPDATE_REQUIRED_MESSAGE, + { cacheConfirmation: false } + ) +} + +export { + AUTOMATION_LIST_HOST_SCOPE_RUNTIME_CAPABILITY, + AUTOMATION_LIST_HOST_SCOPE_UPDATE_REQUIRED_MESSAGE +} diff --git a/src/renderer/src/components/automations/automation-create-admission.ts b/src/renderer/src/components/automations/automation-create-admission.ts deleted file mode 100644 index b69a597663d..00000000000 --- a/src/renderer/src/components/automations/automation-create-admission.ts +++ /dev/null @@ -1,22 +0,0 @@ -import type { RuntimeStatus } from '../../../../shared/runtime-types' -import type { AutomationHostTarget } from './automation-host-client' -import { - getRuntimeAutomationAvailability, - type AutomationTargetAvailability -} from './automation-target-availability' - -export function getAutomationCreateAvailability(args: { - automationHostTarget: AutomationHostTarget - runtimeStatusByEnvironmentId?: ReadonlyMap< - string, - { status: RuntimeStatus | null; checkedAt: number } - > -}): AutomationTargetAvailability { - if (args.automationHostTarget.kind === 'local') { - return { canRunNow: true, reason: 'available', message: null } - } - return getRuntimeAutomationAvailability( - args.automationHostTarget.environmentId, - args.runtimeStatusByEnvironmentId - ) -} diff --git a/src/renderer/src/components/automations/automation-create-destination.test.ts b/src/renderer/src/components/automations/automation-create-destination.test.ts index bcf20e61c39..faf0905ee37 100644 --- a/src/renderer/src/components/automations/automation-create-destination.test.ts +++ b/src/renderer/src/components/automations/automation-create-destination.test.ts @@ -4,8 +4,10 @@ import type { AutomationHostCatalogEntry } from './automation-host-catalog-types' import { + automationCreateHostOffered, automationCreateHostStableKey, automationCreateProjectMismatch, + automationCreateUpdateRequiredAuthorityLabels, preselectAutomationCreateHost, resolveAutomationCreateDestination, revalidateAutomationCreateDestination, @@ -116,6 +118,76 @@ describe('create destination revalidation', () => { }) }) +describe('offered create hosts', () => { + it('offers an ineligible host rather than hiding it', () => { + // The regression this pins: a connected host on a pre-host-scoping server + // is ineligible, and hiding it removed every connected host from the picker. + expect( + automationCreateHostOffered( + entry({ + querySupport: 'legacy-unscoped', + scopeGap: 'authority-unscoped' + }) + ) + ).toBe(true) + expect(automationCreateHostOffered(entry({ owner: null, catalogState: 'unhydrated' }))).toBe( + true + ) + }) + + it('hides only rows that can never become destinations', () => { + expect(automationCreateHostOffered(entry({ kind: 'orphan' }))).toBe(false) + expect(automationCreateHostOffered(entry({ owner: null, catalogState: 'removed' }))).toBe(false) + }) + + it('names the authorities a server update would repair, once each', () => { + const legacySelf = entry({ + stableKey: 'runtime:r1:self', + authorityLabel: 'legacy-box', + querySupport: 'legacy-unscoped', + scopeGap: 'authority-unscoped' + }) + const legacySshChild = entry({ + stableKey: 'runtime:r1:ssh:box', + kind: 'ssh', + authorityLabel: 'legacy-box', + querySupport: 'legacy-unscoped', + scopeGap: 'authority-unscoped' + }) + const incompatible = entry({ + stableKey: 'runtime:r2:self', + authorityLabel: 'older-box', + querySupport: 'incompatible' + }) + expect( + automationCreateUpdateRequiredAuthorityLabels([ + entry(), + legacySelf, + legacySshChild, + incompatible + ]) + ).toEqual(['legacy-box', 'older-box']) + }) + + it('does not blame the server for hosts another repair or none would fix', () => { + // Unverified since disconnect: the repair is a reconnect, not an update. + const unverified = entry({ + stableKey: 'desktop:ssh:cold', + kind: 'ssh', + querySupport: 'legacy-unscoped', + scopeGap: 'target-unverified' + }) + // Scoped but not yet hydrated: disabled, and no repair to name. + const unhydrated = entry({ + stableKey: 'desktop:ssh:warm', + kind: 'ssh', + owner: null, + catalogState: 'unhydrated' + }) + expect(automationCreateUpdateRequiredAuthorityLabels([unverified, unhydrated])).toEqual([]) + }) +}) + describe('sole create host', () => { const hydrated: AutomationCatalogHydrationEvidence = { runtimeCatalogSettled: true, diff --git a/src/renderer/src/components/automations/automation-create-destination.ts b/src/renderer/src/components/automations/automation-create-destination.ts index 8eff3749059..97a35d78a81 100644 --- a/src/renderer/src/components/automations/automation-create-destination.ts +++ b/src/renderer/src/components/automations/automation-create-destination.ts @@ -29,6 +29,7 @@ import { repoConnectionIdIn, type AutomationAuthorityRepoTables } from './automation-authority-identity' +import { automationHostRecoveryActions } from './automation-host-status-descriptors' export type AutomationCreateDestinationChoiceReason = /** No host is selected and nothing may be assumed. */ @@ -61,6 +62,35 @@ export function automationCreateHostEligible( return entry.kind !== 'orphan' && entry.owner !== null && entry.querySupport === 'scoped' } +/** + * What the picker lists: a superset of eligibility on purpose. An ineligible + * host renders disabled with its status stated, because a host the rest of the + * app still shows must not silently vanish here — omission reads as the host + * being gone. Only rows that can never become destinations stay hidden: the + * orphan bucket and removed targets. + */ +export function automationCreateHostOffered(entry: AutomationHostCatalogEntry): boolean { + return entry.kind !== 'orphan' && entry.catalogState !== 'removed' +} + +/** + * The authorities whose ineligibility a server update would repair, named so + * the field can say which machines need updating. A disabled row cannot explain + * itself — its tooltip sits behind pointer-events: none — and not every + * disabled host is repaired this way (an unverified target needs a reconnect), + * so the hint names exactly the update-repairable ones. + */ +export function automationCreateUpdateRequiredAuthorityLabels( + entries: readonly AutomationHostCatalogEntry[] +): string[] { + const labels = entries + .filter(automationCreateHostOffered) + .filter((entry) => !automationCreateHostEligible(entry)) + .filter((entry) => automationHostRecoveryActions(entry).authority === 'update-server') + .map((entry) => entry.authorityLabel) + return [...new Set(labels)] +} + export function resolveAutomationCreateDestination( entry: AutomationHostCatalogEntry | null | undefined ): AutomationCreateDestinationResolution { diff --git a/src/renderer/src/components/automations/automation-external-target-match.test.ts b/src/renderer/src/components/automations/automation-external-target-match.test.ts index 71a7ff8fd4b..25cdaa65eb8 100644 --- a/src/renderer/src/components/automations/automation-external-target-match.test.ts +++ b/src/renderer/src/components/automations/automation-external-target-match.test.ts @@ -2,10 +2,7 @@ import { describe, expect, it } from 'vitest' import type { ExternalAutomationTarget } from '../../../../shared/automations-types' import { toSshExecutionHostId } from '../../../../shared/execution-host' import type { Repo } from '../../../../shared/repo-types' -import { - repoMatchesExternalAutomationTarget, - getExternalAutomationTargetForRepo -} from './automation-external-target-match' +import { repoMatchesExternalAutomationTarget } from './automation-external-target-match' function repo( overrides: Partial> = {} @@ -51,34 +48,3 @@ describe('repoMatchesExternalAutomationTarget', () => { ).toBe(false) }) }) - -describe('getExternalAutomationTargetForRepo', () => { - it('prefers an explicit executionHostId over a disagreeing connectionId', () => { - expect( - getExternalAutomationTargetForRepo( - repo({ connectionId: 'conn-1', executionHostId: toSshExecutionHostId('conn-2') }) - ) - ).toEqual({ type: 'ssh', connectionId: 'conn-2' }) - }) - - it('derives the ssh target from connectionId when executionHostId is unset', () => { - expect(getExternalAutomationTargetForRepo(repo({ connectionId: 'conn-1' }))).toEqual({ - type: 'ssh', - connectionId: 'conn-1' - }) - }) - - it('round-trips connection ids that need host-id encoding', () => { - expect(getExternalAutomationTargetForRepo(repo({ connectionId: 'user@host:22' }))).toEqual({ - type: 'ssh', - connectionId: 'user@host:22' - }) - }) - - it('returns the local target for a plain local repo and for non-ssh hosts', () => { - expect(getExternalAutomationTargetForRepo(repo())).toEqual({ type: 'local' }) - expect(getExternalAutomationTargetForRepo(repo({ executionHostId: 'runtime:env-1' }))).toEqual({ - type: 'local' - }) - }) -}) diff --git a/src/renderer/src/components/automations/automation-external-target-match.ts b/src/renderer/src/components/automations/automation-external-target-match.ts index 417453d8550..f544c324eb1 100644 --- a/src/renderer/src/components/automations/automation-external-target-match.ts +++ b/src/renderer/src/components/automations/automation-external-target-match.ts @@ -18,12 +18,3 @@ export function repoMatchesExternalAutomationTarget( } return repoHostId === toSshExecutionHostId(target.connectionId) } - -// Why: the manager target must name the repo's authoritative execution host, -// not a possibly-stale connectionId. -export function getExternalAutomationTargetForRepo( - repo: Pick -): ExternalAutomationTarget { - const parsed = parseExecutionHostId(getRepoExecutionHostId(repo)) - return parsed?.kind === 'ssh' ? { type: 'ssh', connectionId: parsed.targetId } : { type: 'local' } -} diff --git a/src/renderer/src/components/automations/automation-host-cache-controller.ts b/src/renderer/src/components/automations/automation-host-cache-controller.ts index 5e50c41a317..fda1e267ab6 100644 --- a/src/renderer/src/components/automations/automation-host-cache-controller.ts +++ b/src/renderer/src/components/automations/automation-host-cache-controller.ts @@ -71,6 +71,8 @@ export type AutomationHostQueryController = { handleAuthorityEvent: (event: AutomationAuthorityChangeEvent) => void /** Latest orphan count the authority reported, so the next catalog can show its orphan entry. */ authorityOrphanCount: (authority: StableAutomationAuthorityRef) => number | null + /** True once disposed. A disposed controller drops every event and refresh, so a mounted owner must replace it. */ + isDisposed: () => boolean dispose: () => void } @@ -130,6 +132,7 @@ export function createAutomationHostQueryController( }) let latest: AutomationHostCatalog | null = null let selected: string | null = null + let disposed = false const targetsFor = ( catalog: AutomationHostCatalog, @@ -161,8 +164,10 @@ export function createAutomationHostQueryController( } }) - // Subscribed here rather than by the page: an unsubscribed controller silently - // stops refreshing on writes, which looks like stale data, not a missing wire. + // Subscribed here by default: an unsubscribed controller silently stops + // refreshing on writes, which looks like stale data, not a missing wire. A + // caller that opts out with `eventTarget: null` (the React hook, whose + // StrictMode-safe lifecycle owns the subscription) takes that duty on itself. const eventTarget = options.eventTarget === undefined ? (globalThis.window ?? null) : options.eventTarget const unsubscribe = eventTarget @@ -211,7 +216,9 @@ export function createAutomationHostQueryController( } return newest?.orphanCount ?? null }, + isDisposed: () => disposed, dispose: () => { + disposed = true unsubscribe?.() invalidation.dispose() scheduler.dispose() diff --git a/src/renderer/src/components/automations/automation-host-cache.test.ts b/src/renderer/src/components/automations/automation-host-cache.test.ts index b68f3144607..f1d87df29f5 100644 --- a/src/renderer/src/components/automations/automation-host-cache.test.ts +++ b/src/renderer/src/components/automations/automation-host-cache.test.ts @@ -157,6 +157,18 @@ describe('automation host cache invalidation scope', () => { expect(cache.commit(desktopFence, { rows: [row('c')] })).toBe(true) }) + it('drops the payload at retirement, so a revived host refetches over showing old rows', () => { + const cache = createCache() + cache.commit(cache.beginRequest(RUNTIME_SELF), { rows: [row('b')], orphanCount: 3 }) + cache.evict(hostStableKey(RUNTIME_SELF)) + cache.beginRequest(RUNTIME_SELF) + const revived = cache.getByKey(hostStableKey(RUNTIME_SELF)) + expect(revived?.data).toEqual([]) + expect(revived?.fetchedAt).toBeNull() + expect(revived?.orphanCount).toBeNull() + expect(cache.freshness(RUNTIME_SELF)).toBe('missing') + }) + it('evicts a departed entry and caps the retired pool', () => { const cache = createCache({ retiredLimit: 1 }) cache.commit(cache.beginRequest(DESKTOP_SELF), { rows: [row('a')] }) diff --git a/src/renderer/src/components/automations/automation-host-cache.ts b/src/renderer/src/components/automations/automation-host-cache.ts index d8b3da1525b..2cf3e31ef0c 100644 --- a/src/renderer/src/components/automations/automation-host-cache.ts +++ b/src/renderer/src/components/automations/automation-host-cache.ts @@ -277,6 +277,13 @@ export function createAutomationHostCache( return } bump(record) + // Retirement exists to fence, not to display: the generations stay, the + // payload goes, or the retired pool holds up to 256 stale row arrays. A + // revived host reads as missing and refetches instead of showing old rows. + record.data = [] + record.fetchedAt = null + record.error = null + record.orphanCount = null records.delete(stableKey) retired.set(stableKey, record) // Insertion order is LRU order here: reviving re-inserts at the end. diff --git a/src/renderer/src/components/automations/automation-host-client.test.ts b/src/renderer/src/components/automations/automation-host-client.test.ts index 266fbe62f92..a8a3eea7da1 100644 --- a/src/renderer/src/components/automations/automation-host-client.test.ts +++ b/src/renderer/src/components/automations/automation-host-client.test.ts @@ -1,10 +1,10 @@ import { describe, expect, it, vi, beforeEach } from 'vitest' import type { Automation, AutomationCreateInput } from '../../../../shared/automations-types' import { - createAutomationForTarget, listAutomationRunsForTarget, listAutomationsForTarget, runAutomationNowForTarget, + toRuntimeAutomationCreateInput, updateAutomationForTarget } from './automation-host-client' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' @@ -127,7 +127,7 @@ describe('automation host client', () => { ) }) - it('uses an exact machine selector for an existing runtime workspace', async () => { + it('encodes exact machine selectors for the create wire input', () => { const automation = makeAutomation({ workspaceMode: 'existing', workspaceId: 'repo-1::/srv/orca' @@ -146,19 +146,15 @@ describe('automation host client', () => { rrule: automation.rrule, dtstart: automation.dtstart } - vi.mocked(callRuntimeRpc).mockResolvedValueOnce({ automation }) - await createAutomationForTarget(input) - - expect(callRuntimeRpc).toHaveBeenCalledWith( - { kind: 'environment', environmentId: 'gpu' }, - 'automation.create', - expect.objectContaining({ - repo: 'id:repo-1', - workspace: 'id:repo-1::/srv/orca' - }), - { timeoutMs: 15_000 } - ) + expect(toRuntimeAutomationCreateInput(input)).toMatchObject({ + repo: 'id:repo-1', + workspace: 'id:repo-1::/srv/orca' + }) + // A per-run workspace states no workspace selector at all. + expect( + toRuntimeAutomationCreateInput({ ...input, workspaceMode: 'new_per_run', workspaceId: null }) + ).toMatchObject({ repo: 'id:repo-1', workspace: undefined }) }) it('updates and manually runs SSH-host automations through the remote server that listed them', async () => { diff --git a/src/renderer/src/components/automations/automation-host-client.ts b/src/renderer/src/components/automations/automation-host-client.ts index 52c2d85e341..2b7c3b7c102 100644 --- a/src/renderer/src/components/automations/automation-host-client.ts +++ b/src/renderer/src/components/automations/automation-host-client.ts @@ -66,11 +66,8 @@ export function getAutomationOwnerTarget( return getAutomationTargetFromHostId(automation.runContext?.hostId) } -export function getAutomationCreateTarget(input: AutomationCreateInput): AutomationHostTarget { - return getAutomationTargetFromHostId(input.runContext?.hostId) -} - -function toRuntimeAutomationCreateInput( +/** Renames the desktop input's target fields to the wire contract every authority speaks. */ +export function toRuntimeAutomationCreateInput( input: AutomationCreateInput ): RuntimeAutomationCreateInput { const { projectId, workspaceId, ...rest } = input @@ -125,17 +122,6 @@ export async function listAutomationRunsForTarget( return result.runs } -export async function createAutomationForTarget(input: AutomationCreateInput): Promise { - const target = getAutomationCreateTarget(input) - const result = await callRuntimeRpc<{ automation: Automation }>( - target, - 'automation.create', - toRuntimeAutomationCreateInput(input), - { timeoutMs: 15_000 } - ) - return result.automation -} - export async function updateAutomationForTarget( automation: Automation, updates: AutomationUpdateInput, diff --git a/src/renderer/src/components/automations/automation-host-diagnostics.test.ts b/src/renderer/src/components/automations/automation-host-diagnostics.test.ts index cc49fee491c..3c13a79ec85 100644 --- a/src/renderer/src/components/automations/automation-host-diagnostics.test.ts +++ b/src/renderer/src/components/automations/automation-host-diagnostics.test.ts @@ -345,15 +345,18 @@ describe('automation host request counters', () => { }) describe('unpooled capability probes', () => { - beforeEach(() => { + beforeEach(async () => { callRuntimeRpc.mockReset() getRuntimeEnvironmentStatus.mockReset() automationHostDiagnostics.reset() + // Confirmed capabilities are module-level and must not leak between tests. + ;(await import('./automation-scoped-list-client')).resetAutomationCapabilityProbes() }) - // The probe rides outside the four-slot pool and always re-fetches, so an - // instrument blind to it would report half the relay traffic a refresh costs. - it('counts the status.get probe every runtime-scoped list rides on', async () => { + // The probe rides outside the four-slot pool, so an instrument blind to it + // would under-report relay traffic. It dedupes per authority incarnation: + // both hosts of this authority ride on one status.get. + it('counts the status.get probe runtime-scoped lists ride on', async () => { const { cache } = harness() getRuntimeEnvironmentStatus.mockResolvedValue({ capabilities: [AUTOMATION_LIST_HOST_SCOPE_RUNTIME_CAPABILITY] @@ -380,9 +383,9 @@ describe('unpooled capability probes', () => { const authority = automationHostDiagnostics.snapshot().byAuthority[authorityKeyOf(RUNTIME_AUTHORITY)] - expect(getRuntimeEnvironmentStatus).toHaveBeenCalledTimes(2) - // Two pooled list calls, and two round trips the pool never sees. - expect(authority).toMatchObject({ requests: 2, capabilityProbes: 2 }) + expect(getRuntimeEnvironmentStatus).toHaveBeenCalledTimes(1) + // Two pooled list calls, and the one shared round trip the pool never sees. + expect(authority).toMatchObject({ requests: 2, capabilityProbes: 1 }) }) }) diff --git a/src/renderer/src/components/automations/automation-host-diagnostics.ts b/src/renderer/src/components/automations/automation-host-diagnostics.ts index 7428cf7d21a..b514c9c7986 100644 --- a/src/renderer/src/components/automations/automation-host-diagnostics.ts +++ b/src/renderer/src/components/automations/automation-host-diagnostics.ts @@ -25,8 +25,9 @@ export type AutomationHostKeyCounters = { legacyRequests: number /** * Unpooled `status.get` capability probes, which ride outside the four-slot - * pool and always re-fetch. Relay traffic is `requests + capabilityProbes`; - * the in-flight ceiling is stated over `requests` alone. + * pool; probes dedupe per authority incarnation, so this counts what was + * actually sent. Relay traffic is `requests + capabilityProbes`; the + * in-flight ceiling is stated over `requests` alone. */ capabilityProbes: number /** Callers that joined a request already in flight, keyed like `requests`. */ diff --git a/src/renderer/src/components/automations/automation-host-scale-gates.test.ts b/src/renderer/src/components/automations/automation-host-scale-gates.test.ts index e85ed6d1ae5..5bec80e92fa 100644 --- a/src/renderer/src/components/automations/automation-host-scale-gates.test.ts +++ b/src/renderer/src/components/automations/automation-host-scale-gates.test.ts @@ -34,6 +34,7 @@ import { type AutomationHostFetchTarget } from './automation-host-scheduler' import { AUTOMATION_HOST_REQUEST_CONCURRENCY } from './automation-host-scheduler-queue' +import { resetAutomationCapabilityProbes } from './automation-scoped-list-client' const callRuntimeRpc = vi.fn() const getRuntimeEnvironmentStatus = vi.fn() @@ -254,7 +255,10 @@ function watchLongTasks(): () => number { } } -function harness(): { cache: AutomationHostCache; refresh: () => Promise } { +function harness(): { + cache: AutomationHostCache + refresh: (options?: { force?: boolean }) => Promise +} { const cache = createAutomationHostCache({ catalogGeneration: () => 0, connectionGeneration: () => 0 @@ -265,7 +269,7 @@ function harness(): { cache: AutomationHostCache; refresh: () => Promise } isVisible: () => true, scheduleRetry: () => () => {} }) - return { cache, refresh: () => scheduler.refresh(ALL_REFS.map(targetFor)) } + return { cache, refresh: (options) => scheduler.refresh(ALL_REFS.map(targetFor), options) } } function snapshot(): AutomationHostDiagnosticsSnapshot { @@ -278,6 +282,8 @@ function authorityCounters(snap: AutomationHostDiagnosticsSnapshot, index: numbe beforeEach(() => { automationHostDiagnostics.reset() + // Confirmed capabilities are module-level and must not leak between tests. + resetAutomationCapabilityProbes() wire = { inFlight: 0, maxInFlight: 0, @@ -384,28 +390,41 @@ describe('one refresh of 50 hosts carrying 1,000 automations', () => { }) describe('relay cost the request pool cannot see', () => { - // The probe is deliberate and unpooled, so the honest number for a 50-host - // refresh is requests plus probes — not the pooled count on its own. - it('counts one capability probe for every scoped list, and none for a legacy one', async () => { + // The probe is deliberate and unpooled, but it dedupes per authority + // incarnation: concurrent callers share the in-flight status.get and a + // confirmed capability is never re-asked, so a 50-host refresh probes each + // scoped authority once — not once per host. + it('counts one capability probe per scoped authority, and none for a legacy one', async () => { const { refresh } = harness() await refresh() const snap = snapshot() const scopedEntries = SCOPED_AUTHORITIES * ENTRIES_PER_AUTHORITY - expect(wire.probes).toBe(scopedEntries) - expect(snap.totals.capabilityProbes).toBe(scopedEntries) + expect(wire.probes).toBe(SCOPED_AUTHORITIES) + expect(snap.totals.capabilityProbes).toBe(SCOPED_AUTHORITIES) for (let index = 0; index < AUTHORITY_COUNT; index += 1) { - expect(authorityCounters(snap, index).capabilityProbes).toBe( - isLegacyAuthority(index) ? 0 : ENTRIES_PER_AUTHORITY - ) + expect(authorityCounters(snap, index).capabilityProbes).toBe(isLegacyAuthority(index) ? 0 : 1) } - // Scoped hosts cost two round trips each, so the pooled count alone reports - // barely half the traffic this refresh actually put on the relay. expect(snap.totals.requests + snap.totals.capabilityProbes).toBe( - scopedEntries * 2 + (AUTHORITY_COUNT - SCOPED_AUTHORITIES) + scopedEntries + SCOPED_AUTHORITIES + (AUTHORITY_COUNT - SCOPED_AUTHORITIES) ) }) + + // The cache is per incarnation, so even a forced second refresh — which + // re-fetches every list — costs no probes at all. + it('does not probe again on a later refresh of the same incarnations', async () => { + const { refresh } = harness() + + await refresh() + const probesAfterFirst = wire.probes + const scopedCallsAfterFirst = wire.scopedCalls + await refresh({ force: true }) + + expect(probesAfterFirst).toBe(SCOPED_AUTHORITIES) + expect(wire.probes).toBe(probesAfterFirst) + expect(wire.scopedCalls).toBeGreaterThan(scopedCallsAfterFirst) + }) }) describe('the commit fence under real concurrency', () => { diff --git a/src/renderer/src/components/automations/automation-scoped-list-client.test.ts b/src/renderer/src/components/automations/automation-scoped-list-client.test.ts index c5e2efe1b07..e1bf8873e97 100644 --- a/src/renderer/src/components/automations/automation-scoped-list-client.test.ts +++ b/src/renderer/src/components/automations/automation-scoped-list-client.test.ts @@ -31,9 +31,11 @@ const ALL_CAPABILITIES = { ] } -beforeEach(() => { +beforeEach(async () => { callRuntimeRpc.mockReset() getRuntimeEnvironmentStatus.mockReset() + // Confirmed capabilities are module-level and must not leak between tests. + ;(await client()).resetAutomationCapabilityProbes() }) async function client() { @@ -133,6 +135,113 @@ describe('listScopedAutomations', () => { }) }) +describe('capability probe dedupe', () => { + it('shares one in-flight probe across concurrent calls to the same incarnation', async () => { + const { listScopedAutomations } = await client() + getRuntimeEnvironmentStatus.mockImplementation(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + return ALL_CAPABILITIES + }) + callRuntimeRpc.mockResolvedValue({ automations: [], items: [], orphanCount: 0 }) + await Promise.all([ + listScopedAutomations(RUNTIME, { kind: 'self' }), + listScopedAutomations(RUNTIME, { kind: 'orphan' }) + ]) + expect(getRuntimeEnvironmentStatus).toHaveBeenCalledTimes(1) + }) + + it('never re-asks about a capability the incarnation already confirmed', async () => { + const { listScopedAutomations, updateAutomationForOwner } = await client() + getRuntimeEnvironmentStatus.mockResolvedValue(ALL_CAPABILITIES) + callRuntimeRpc.mockResolvedValue({ + automations: [], + items: [], + orphanCount: 0, + automation: { id: 'a1' } + }) + await listScopedAutomations(RUNTIME, { kind: 'self' }) + await listScopedAutomations(RUNTIME, { kind: 'orphan' }) + // A different capability confirmed by the same status answer is also cached. + await updateAutomationForOwner(SSH_OWNER, 'a1', { enabled: false }) + // Mutations intentionally re-probe: owner fencing must not trust a positive + // answer after an in-place runtime replacement under the same pairing. + expect(getRuntimeEnvironmentStatus).toHaveBeenCalledTimes(2) + }) + + it('re-asks after a re-pair rather than trusting the old incarnation', async () => { + const { listScopedAutomations } = await client() + getRuntimeEnvironmentStatus.mockResolvedValue(ALL_CAPABILITIES) + callRuntimeRpc.mockResolvedValue({ automations: [], items: [], orphanCount: 0 }) + await listScopedAutomations(RUNTIME, { kind: 'self' }) + await listScopedAutomations({ ...RUNTIME, pairingRevision: 5 }, { kind: 'self' }) + expect(getRuntimeEnvironmentStatus).toHaveBeenCalledTimes(2) + }) + + it('does not cache an absence, so an upgraded server recovers without a re-pair', async () => { + const { listScopedAutomations, AutomationHostScopeUnsupportedError } = await client() + getRuntimeEnvironmentStatus.mockResolvedValueOnce({ capabilities: [] }) + await expect(listScopedAutomations(RUNTIME, { kind: 'self' })).rejects.toBeInstanceOf( + AutomationHostScopeUnsupportedError + ) + getRuntimeEnvironmentStatus.mockResolvedValue(ALL_CAPABILITIES) + callRuntimeRpc.mockResolvedValue({ automations: [], items: [], orphanCount: 0 }) + await expect(listScopedAutomations(RUNTIME, { kind: 'self' })).resolves.toBeTruthy() + expect(getRuntimeEnvironmentStatus).toHaveBeenCalledTimes(2) + }) + + // The fence depends on this: a server downgraded in place ignores + // `expectedOwner`, so a confirmation must not outlive its observation window. + it('re-asks once a confirmation ages out', async () => { + vi.useFakeTimers() + try { + const { listScopedAutomations, AUTHORITY_CAPABILITY_CONFIRMATION_TTL_MS } = await client() + getRuntimeEnvironmentStatus.mockResolvedValue(ALL_CAPABILITIES) + callRuntimeRpc.mockResolvedValue({ automations: [], items: [], orphanCount: 0 }) + await listScopedAutomations(RUNTIME, { kind: 'self' }) + vi.advanceTimersByTime(AUTHORITY_CAPABILITY_CONFIRMATION_TTL_MS) + await listScopedAutomations(RUNTIME, { kind: 'self' }) + expect(getRuntimeEnvironmentStatus).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) + + it('does not cache a failed probe, so the next call retries the host', async () => { + const { listScopedAutomations } = await client() + getRuntimeEnvironmentStatus.mockRejectedValueOnce(new Error('runtime_unavailable')) + await expect(listScopedAutomations(RUNTIME, { kind: 'self' })).rejects.toThrow( + 'runtime_unavailable' + ) + getRuntimeEnvironmentStatus.mockResolvedValue(ALL_CAPABILITIES) + callRuntimeRpc.mockResolvedValue({ automations: [], items: [], orphanCount: 0 }) + await expect(listScopedAutomations(RUNTIME, { kind: 'self' })).resolves.toBeTruthy() + expect(getRuntimeEnvironmentStatus).toHaveBeenCalledTimes(2) + }) + + it('bounds confirmed capability entries across retired environment incarnations', async () => { + const { listScopedAutomations } = await client() + getRuntimeEnvironmentStatus.mockResolvedValue(ALL_CAPABILITIES) + callRuntimeRpc.mockResolvedValue({ automations: [], items: [], orphanCount: 0 }) + + for (let index = 0; index < 32; index += 1) { + await listScopedAutomations( + { kind: 'runtime', environmentId: `env-${index}`, pairingRevision: 1 }, + { kind: 'self' } + ) + } + await listScopedAutomations( + { kind: 'runtime', environmentId: 'env-overflow', pairingRevision: 1 }, + { kind: 'self' } + ) + await listScopedAutomations( + { kind: 'runtime', environmentId: 'env-0', pairingRevision: 1 }, + { kind: 'self' } + ) + + expect(getRuntimeEnvironmentStatus).toHaveBeenCalledTimes(34) + }) +}) + describe('owner-fenced mutations', () => { it('sends the captured owner with the mutation', async () => { const { updateAutomationForOwner } = await client() diff --git a/src/renderer/src/components/automations/automation-scoped-list-client.ts b/src/renderer/src/components/automations/automation-scoped-list-client.ts index 3f649dcbdc0..ec44ac74ece 100644 --- a/src/renderer/src/components/automations/automation-scoped-list-client.ts +++ b/src/renderer/src/components/automations/automation-scoped-list-client.ts @@ -9,11 +9,7 @@ * that would attribute other hosts' automations to the selected one. */ -import { - callRuntimeRpc, - getRuntimeEnvironmentStatus, - type RuntimeClientTarget -} from '@/runtime/runtime-rpc-client' +import { callRuntimeRpc, type RuntimeClientTarget } from '@/runtime/runtime-rpc-client' import type { Automation, AutomationCreateInput, @@ -36,26 +32,24 @@ import type { AutomationOwnerPrecondition } from '../../../../shared/automation-owner-precondition' import { + toRuntimeAutomationCreateInput, + toRuntimeAutomationUpdateInput +} from './automation-host-client' +import { + assertAuthorityCapability, + assertOwnerFencingSupported, AUTOMATION_LIST_HOST_SCOPE_RUNTIME_CAPABILITY, AUTOMATION_LIST_HOST_SCOPE_UPDATE_REQUIRED_MESSAGE, - AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY, - AUTOMATION_OWNER_FENCING_UPDATE_REQUIRED_MESSAGE, - type RuntimeCapability -} from '../../../../shared/protocol-version' -import { automationAuthorityCatalogKey } from './automation-host-catalog-types' -import { toRuntimeAutomationUpdateInput } from './automation-host-client' -import { automationHostDiagnostics } from './automation-host-diagnostics' + AutomationHostScopeUnsupportedError, + REQUEST_TIMEOUT_MS +} from './automation-capability-probe' -const REQUEST_TIMEOUT_MS = 15_000 - -export class AutomationHostScopeUnsupportedError extends Error { - readonly code = 'unsupported_host_scope' - - constructor(message: string) { - super(message) - this.name = 'AutomationHostScopeUnsupportedError' - } -} +export { + AutomationHostScopeUnsupportedError, + AUTHORITY_CAPABILITY_CONFIRMATION_TTL_MS, + AUTHORITY_CAPABILITY_CONFIRMATION_MAX, + resetAutomationCapabilityProbes +} from './automation-capability-probe' export class AutomationListResponseError extends Error { readonly code = 'invalid_response' @@ -119,41 +113,6 @@ async function callAuthority( }) } -/** - * Fails closed on a missing capability, but only on a *known* absence: an - * unreachable authority must classify as unavailable and retry, not as an old - * server the user is told to upgrade. - * - * The probe is counted here because it is counted nowhere else: it deliberately - * re-fetches on every call and rides outside the scheduler's four-slot pool, so - * an instrument that saw only pooled work would report half the relay traffic a - * 50-host refresh actually costs. - */ -async function assertAuthorityCapability( - authority: AutomationAuthorityRef, - capability: RuntimeCapability, - message: string -): Promise { - if (authority.kind !== 'runtime') { - return - } - automationHostDiagnostics.recordCapabilityProbe({ - authorityKey: automationAuthorityCatalogKey(authority) - }) - const status = await getRuntimeEnvironmentStatus(authority.environmentId, REQUEST_TIMEOUT_MS) - if (!status.capabilities?.includes(capability)) { - throw new AutomationHostScopeUnsupportedError(message) - } -} - -async function assertOwnerFencingSupported(authority: AutomationAuthorityRef): Promise { - await assertAuthorityCapability( - authority, - AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY, - AUTOMATION_OWNER_FENCING_UPDATE_REQUIRED_MESSAGE - ) -} - function validated(raw: unknown, selector: AutomationListScopeSelector): ScopedAutomationList { const validation = validateAutomationListResponse(raw, selector) if (!validation.ok) { @@ -328,11 +287,8 @@ export async function createAutomationForDestination( destination: AutomationDestination ): Promise { await assertOwnerFencingSupported(authority) - const { projectId, workspaceId, ...rest } = input const result = await callAuthority<{ automation: Automation }>(authority, 'automation.create', { - ...rest, - repo: projectId, - workspace: input.workspaceMode === 'existing' ? (workspaceId ?? undefined) : undefined, + ...toRuntimeAutomationCreateInput(input), destination }) return result.automation diff --git a/src/renderer/src/components/automations/automation-target-availability.test.ts b/src/renderer/src/components/automations/automation-target-availability.test.ts index 23264de4b53..2b94251144a 100644 --- a/src/renderer/src/components/automations/automation-target-availability.test.ts +++ b/src/renderer/src/components/automations/automation-target-availability.test.ts @@ -5,7 +5,6 @@ import type { ProjectHostSetup } from '../../../../shared/project-types' import type { Repo } from '../../../../shared/repo-types' import type { Worktree } from '../../../../shared/worktree/types' import { getAutomationTargetAvailability } from './automation-target-availability' -import { getAutomationCreateAvailability } from './automation-create-admission' function makeAutomation(overrides: Partial = {}): Automation { return { @@ -88,21 +87,6 @@ function makeRuntimeStatus(overrides: Partial = {}): RuntimeStatu } describe('automation target availability', () => { - it('blocks create admission for a missing or degraded runtime', () => { - const target = { kind: 'environment' as const, environmentId: 'env-1' } - expect(getAutomationCreateAvailability({ automationHostTarget: target }).reason).toBe( - 'runtime-checking' - ) - expect( - getAutomationCreateAvailability({ - automationHostTarget: target, - runtimeStatusByEnvironmentId: new Map([ - ['env-1', { status: makeRuntimeStatus({ graphStatus: 'unavailable' }), checkedAt: 1 }] - ]) - }).reason - ).toBe('runtime-unavailable') - }) - it('allows local automations with an available existing workspace', () => { expect( getAutomationTargetAvailability({ diff --git a/src/renderer/src/components/automations/automations-page-runtime-fixtures.ts b/src/renderer/src/components/automations/automations-page-runtime-fixtures.ts index 6b71a2364e2..61d62d03f0f 100644 --- a/src/renderer/src/components/automations/automations-page-runtime-fixtures.ts +++ b/src/renderer/src/components/automations/automations-page-runtime-fixtures.ts @@ -1,3 +1,4 @@ +import type { Automation } from '../../../../shared/automations-types' import type { ProjectHostSetup } from '../../../../shared/project-types' import type { Repo } from '../../../../shared/repo-types' import type { Worktree } from '../../../../shared/worktree/types' @@ -5,6 +6,18 @@ import type { Worktree } from '../../../../shared/worktree/types' export const RUNTIME_REPO_ID = 'repo-2' export const RUNTIME_WORKSPACE_ID = 'workspace-2' +/** Builds the host-scoped response used by the page harness. */ +export function selfScopedList(automations: Automation[]): Record { + return { + automations, + items: automations.map((automation) => ({ + automationId: automation.id, + selector: { kind: 'self' } + })), + orphanCount: 0 + } +} + type RuntimeFixtureMocks = { state: Record repoMap: Map diff --git a/src/renderer/src/components/automations/automations-page-test-harness.tsx b/src/renderer/src/components/automations/automations-page-test-harness.tsx index c7d3b8ac158..815338927c6 100644 --- a/src/renderer/src/components/automations/automations-page-test-harness.tsx +++ b/src/renderer/src/components/automations/automations-page-test-harness.tsx @@ -12,7 +12,7 @@ * guarantees. */ -import { act, type ReactNode } from 'react' +import { act, createElement, StrictMode, type ReactNode } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, type Mock, vi } from 'vitest' import type { Automation, AutomationRun } from '../../../../shared/automations-types' @@ -25,10 +25,12 @@ import type { AutomationHostCatalogView } from './use-automation-host-catalog' import type { AutomationCreateDestinationControl } from './use-automation-create-destination' import type { ExternalAutomationListEntry } from './external-automation-list-entries' import type { AutomationListRow } from './automation-list-row-identity' +import { resetAutomationCapabilityProbes } from './automation-scoped-list-client' import { addRuntimeProject as addRuntimeProjectFixture, RUNTIME_REPO_ID as RUNTIME_REPO_ID_FIXTURE, - RUNTIME_WORKSPACE_ID as RUNTIME_WORKSPACE_ID_FIXTURE + RUNTIME_WORKSPACE_ID as RUNTIME_WORKSPACE_ID_FIXTURE, + selfScopedList } from './automations-page-runtime-fixtures' export const RUNTIME_REPO_ID = RUNTIME_REPO_ID_FIXTURE @@ -69,6 +71,7 @@ export type ListPanelProps = { toggleAutomation: (row: AutomationListRow) => void requestDeleteAutomation: (row: AutomationListRow) => void openCreateDialog: () => void + canCreateAutomation: boolean } export type DetailPaneProps = { @@ -309,18 +312,6 @@ export const RUNTIME_SELF_FILTER = { } } -/** The scoped-list shape every self-owned host answers with. */ -function selfScopedList(automations: Automation[]): Record { - return { - automations, - items: automations.map((automation) => ({ - automationId: automation.id, - selector: { kind: 'self' } - })), - orphanCount: 0 - } -} - /** The local authority's automation RPC surface, served from the same programmable * `api.automations` mocks; an environment target answers from `runtimeHost()` state. */ async function answerAutomationRpc( @@ -379,7 +370,7 @@ export function scopedList(automations: Automation[]): void { const roots: Root[] = [] -export async function renderPage(): Promise<{ +export async function renderPage(options?: { strict?: boolean }): Promise<{ container: HTMLDivElement rerender: () => Promise }> { @@ -389,7 +380,12 @@ export async function renderPage(): Promise<{ roots.push(root) const rerender = async (): Promise => { await act(async () => { - root.render() + // Strict mounts double-invoke effects the way the dev app does, which is + // where a dispose-without-revive lifecycle bug becomes visible. + const page = options?.strict + ? createElement(StrictMode, null, createElement(AutomationsPage)) + : createElement(AutomationsPage) + root.render(page) }) } await rerender() @@ -434,6 +430,8 @@ export function installAutomationsPageHarness(): void { beforeEach(() => { globalThis.IS_REACT_ACT_ENVIRONMENT = true vi.clearAllMocks() + // Confirmed capabilities are module-level and must not leak between tests. + resetAutomationCapabilityProbes() // A prior test's wholesale mockImplementation must not leak forward. mocks.callRuntimeRpc.mockReset() mocks.callRuntimeRpc.mockImplementation(answerAutomationRpc) diff --git a/src/renderer/src/components/automations/use-automation-host-catalog.ts b/src/renderer/src/components/automations/use-automation-host-catalog.ts index 3d358e44584..bd7cd8dbb67 100644 --- a/src/renderer/src/components/automations/use-automation-host-catalog.ts +++ b/src/renderer/src/components/automations/use-automation-host-catalog.ts @@ -5,6 +5,7 @@ import { getLocalExecutionHostLabel } from '../../../../shared/execution-host' import type { AutomationHostFilter } from '../../../../shared/automation-host-filter' import type { StableAutomationAuthorityRef } from '../../../../shared/automation-owner-ref' import { buildAutomationHostCatalog } from './automation-host-catalog' +import { subscribeAutomationHostInvalidation } from './automation-host-invalidation-window-events' import { buildAutomationHostCatalogSource } from './automation-host-catalog-source' import { automationHostLoadCounts, @@ -100,20 +101,42 @@ export function useAutomationHostCatalog( const repoTables = useMemo(() => groupReposByAutomationAuthority(repos), [repos]) const repoTablesRef = useRef(repoTables) repoTablesRef.current = repoTables - const [controller] = useState(() => - createAutomationHostQueryController({ - // Scoped to the answering authority: another host's identically named repo - // is not evidence about this one, and its absence is not evidence either. - legacyPartitionContext: (authority) => - automationAuthorityPartitionContext(repoTablesRef.current, authority) - }) + const makeController = useCallback( + () => + createAutomationHostQueryController({ + // Scoped to the answering authority: another host's identically named repo + // is not evidence about this one, and its absence is not evidence either. + legacyPartitionContext: (authority) => + automationAuthorityPartitionContext(repoTablesRef.current, authority), + // Wired by the lifecycle effect below instead: a controller built during + // render must hold no window subscription, or StrictMode's doubled + // initializer leaks a live listener per mount. + eventTarget: null + }), + [] ) + const [controller, setController] = useState(makeController) + // Create-in-render, dispose-in-cleanup is asymmetric under StrictMode's + // simulated unmount: the cleanup disposes the only controller, and a disposed + // controller silently drops every write invalidation — the list then never + // shows a create until the app reloads. The effect owns the whole lifecycle: + // it replaces a disposed controller and unsubscribes before disposing. + useEffect(() => { + if (controller.isDisposed()) { + setController(makeController()) + return + } + const unsubscribe = subscribeAutomationHostInvalidation(controller.handleAuthorityEvent) + return () => { + unsubscribe() + controller.dispose() + } + }, [controller, makeController]) const [cacheVersion, setCacheVersion] = useState(0) useEffect(() => { return controller.cache.subscribe(() => setCacheVersion((version) => version + 1)) }, [controller]) - useEffect(() => () => controller.dispose(), [controller]) const orphanCount = useCallback( (authority: StableAutomationAuthorityRef) => controller.authorityOrphanCount(authority), diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index a4743b65757..ac4dae956b3 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -15402,7 +15402,6 @@ "rowActions": "Automation actions", "tableLastRun": "Last run", "noListMatches": "No automations match.", - "destinationUnavailable": "The selected automation destination is not ready. Reconnect it and try again.", "destinationProjectUnavailable": "Choose a project owned by the selected automation destination." }, "AutomationsPageSkeleton": { @@ -15666,7 +15665,8 @@ "unavailable": "That host cannot hold a new automation yet. Choose another host to create this one on.", "stale": "{host} changed while this form was open. Choose it again before saving.", "projectMismatch": "That project is not on {host}. Choose a project on that host, or another host.", - "noProjects": "No projects are set up on {host}. Add one there, or choose another host." + "noProjects": "No projects are set up on {host}. Add one there, or choose another host.", + "updateRequired": "Update the Orca server on {hosts} to create automations there." } }, "agent": {