diff --git a/src/cli/automation-destination.ts b/src/cli/automation-destination.ts index 6e67ee247b6..18a9b435594 100644 --- a/src/cli/automation-destination.ts +++ b/src/cli/automation-destination.ts @@ -49,13 +49,8 @@ async function sshTargetGeneration( client: RuntimeClient, targetId: string ): Promise { - let targets: SshTargetSummary[] - try { - targets = (await client.call<{ targets: SshTargetSummary[] }>('ssh.listTargetSummaries')).result - .targets - } catch { - return undefined - } + const targets = (await client.call<{ targets: SshTargetSummary[] }>('ssh.listTargetSummaries')) + .result.targets const match = targets.find((candidate) => candidate.id === targetId) if (!match) { throw new AutomationOwnerConflictError(AUTOMATION_OWNER_CONFLICT_CODES.invalidDestination) diff --git a/src/cli/handlers/automation-destination-fencing.test.ts b/src/cli/handlers/automation-destination-fencing.test.ts index c2753702bdb..ba2947e716b 100644 --- a/src/cli/handlers/automation-destination-fencing.test.ts +++ b/src/cli/handlers/automation-destination-fencing.test.ts @@ -23,6 +23,7 @@ function authority(options: { /** Generation the same target carries by the time the write lands. */ generationAtWrite?: number repoConnectionId?: string | null + targetSummaryError?: Error }): { call: ReturnType; writes: { method: string; params: unknown }[] } { const writes: { method: string; params: unknown }[] = [] const connectionId = options.repoConnectionId ?? null @@ -37,6 +38,9 @@ function authority(options: { return ok({ repo: { id: 'r1', connectionId } }) } if (method === 'ssh.listTargetSummaries') { + if (options.targetSummaryError) { + throw options.targetSummaryError + } return ok({ targets: options.registered }) } if (method === 'automation.create' || method === 'automation.update') { @@ -109,6 +113,26 @@ describe('CLI automation writes fence the host they land on', () => { expect(writes).toEqual([]) }) + it('does not drop the fence when reading SSH registrations fails', async () => { + const { call, writes } = authority({ + registered: [], + repoConnectionId: 'box-1', + targetSummaryError: new Error('rpc timeout') + }) + vi.spyOn(console, 'log').mockImplementation(() => undefined) + + await expect( + AUTOMATION_HANDLERS['automations create']!({ + client: { call } as never, + cwd: '/tmp', + flags: new Map([...CREATE_FLAGS, ['repo', 'repo-on-box-1']]), + json: true + }) + ).rejects.toThrow('rpc timeout') + + expect(writes).toEqual([]) + }) + it('refuses a selector-moving edit when the host is re-registered between capture and write', async () => { const { call } = authority({ registered: [{ id: 'box-1', label: 'box 1', generation: 7 }], diff --git a/src/main/automations/automation-owner-fencing.test.ts b/src/main/automations/automation-owner-fencing.test.ts index 4065d5c8d72..47cbd648f93 100644 --- a/src/main/automations/automation-owner-fencing.test.ts +++ b/src/main/automations/automation-owner-fencing.test.ts @@ -194,14 +194,11 @@ describe('owner-fenced mutations', () => { expect(store.listAutomations().find((entry) => entry.id === 'ssh-1-a')?.enabled).toBe(true) }) - it('refuses an unfenced mutation of a record pinned to a generation', async () => { + it('keeps legacy mutations compatible without an owner field', async () => { const store = await createStore() - const conflict = expect.objectContaining({ - code: AUTOMATION_OWNER_CONFLICT_CODES.fencingRequired - }) - expect(() => store.updateAutomation('ssh-1-a', { enabled: false })).toThrowError(conflict) - expect(() => store.deleteAutomation('ssh-1-a')).toThrowError(conflict) - expect(store.listAutomations()).toHaveLength(3) + expect(store.updateAutomation('ssh-1-a', { enabled: false }).enabled).toBe(false) + expect(() => store.deleteAutomation('ssh-1-a')).not.toThrow() + expect(store.listAutomations()).toHaveLength(2) }) it('leaves a legacy client unaffected on a self record', async () => { @@ -256,6 +253,15 @@ describe('owner-fenced mutations', () => { * The projection is what makes the fence satisfiable off-host rather than a wall. */ describe('projected owner precondition', () => { + it('reuses the complete projection until automation state changes', async () => { + const store = await createStore() + const first = store.listAutomationsForScope() + expect(store.listAutomationsForScope()).toBe(first) + + store.updateAutomation('local-1', { enabled: false }) + expect(store.listAutomationsForScope()).not.toBe(first) + }) + it('projects the fenceable owner for each kind of record', async () => { const store = await createStore() expect(store.automationOwnerPrecondition('ssh-1-a')).toEqual(OWNED_SSH) @@ -270,9 +276,7 @@ describe('projected owner precondition', () => { it('satisfies the fence it was projected from, on the record that used to refuse', async () => { const store = await createStore() const owner = store.automationOwnerPrecondition('ssh-1-a')! - expect(() => store.updateAutomation('ssh-1-a', { enabled: false })).toThrowError( - expect.objectContaining({ code: AUTOMATION_OWNER_CONFLICT_CODES.fencingRequired }) - ) + expect(() => store.updateAutomation('ssh-1-a', { enabled: false })).not.toThrow() expect( store.updateAutomation('ssh-1-a', { enabled: false }, { expectedOwner: owner }).enabled ).toBe(false) diff --git a/src/main/automations/automation-owner-migration.test.ts b/src/main/automations/automation-owner-migration.test.ts index 9bf6081b313..ac645907ebe 100644 --- a/src/main/automations/automation-owner-migration.test.ts +++ b/src/main/automations/automation-owner-migration.test.ts @@ -70,6 +70,7 @@ function migrate(input: { folderWorkspaces?: FolderWorkspace[] projectGroups?: ProjectGroup[] sshTargetGenerationCounter?: number + storageAuthority?: 'desktop' | 'runtime' }) { return migrateAutomationOwners({ automations: input.automations ?? [], @@ -79,6 +80,7 @@ function migrate(input: { projectGroups: input.projectGroups, removedSshTargetTombstones: [], sshTargetGenerationCounter: input.sshTargetGenerationCounter, + storageAuthority: input.storageAuthority, now: NOW }) } @@ -283,6 +285,38 @@ describe('migrateAutomationOwners', () => { } ) + it.each([ + { label: 'schedulerOwner', overrides: { schedulerOwner: 'remote_host_service' as const } }, + { + label: 'runContext.hostId', + overrides: { + runContext: { + kind: 'workspace-run' as const, + projectId: 'p', + hostId: 'runtime:env-1' as const, + projectHostSetupId: 's', + repoId: 'repo-1', + path: '/tmp/repo' + } + } + } + ])('keeps a runtime-owned record flagged by $label enabled and owned', ({ overrides }) => { + const result = migrate({ + automations: [makeAutomation(overrides)], + repos: [makeRepo()], + storageAuthority: 'runtime' + }) + const migrated = result.automations[0] + expect(migrated.enabled).toBe(true) + expect( + projectAutomationSelector(migrated, { + storageAuthority: 'runtime', + sshTargetGeneration: () => undefined, + repoConnectionId: () => null + }) + ).toEqual({ kind: 'self' }) + }) + it('leaves a differing captured generation alone so the record stays a replaced orphan', () => { const result = migrate({ automations: [ diff --git a/src/main/automations/automation-owner-migration.ts b/src/main/automations/automation-owner-migration.ts index 5f7e388a771..616231bc311 100644 --- a/src/main/automations/automation-owner-migration.ts +++ b/src/main/automations/automation-owner-migration.ts @@ -31,7 +31,7 @@ import { scanAutomationsForGhostSshTargets } from './automation-ghost-ssh-tombst export type AutomationOwnerClassification = 'owned' | 'orphan' | 'ambiguous' /** - * Storage authority is never inferred from these fields — they only mark a record unrunnable. + * Runtime markers belong to a runtime store; the same markers are ambiguous desktop mirrors. * * `knownSshTargetIds` is the authority's own persisted registry, so a miss is * settled evidence rather than a list that has not loaded. `workspacePin` is @@ -41,11 +41,13 @@ export type AutomationOwnerClassification = 'owned' | 'orphan' | 'ambiguous' export function classifyAutomationOwner( automation: Automation, knownSshTargetIds: ReadonlySet, - workspacePin?: AutomationWorkspaceSshPin + workspacePin?: AutomationWorkspaceSshPin, + storageAuthority: 'desktop' | 'runtime' = 'desktop' ): AutomationOwnerClassification { if ( - automation.schedulerOwner === 'remote_host_service' || - parseExecutionHostId(automation.runContext?.hostId)?.kind === 'runtime' + storageAuthority !== 'runtime' && + (automation.schedulerOwner === 'remote_host_service' || + parseExecutionHostId(automation.runContext?.hostId)?.kind === 'runtime') ) { return 'ambiguous' } @@ -75,6 +77,7 @@ export type AutomationOwnerMigrationInput = { projectGroups?: readonly ProjectGroup[] removedSshTargetTombstones: readonly RemovedSshTargetTombstone[] sshTargetGenerationCounter?: number + storageAuthority?: 'desktop' | 'runtime' now: number } @@ -176,7 +179,8 @@ function indexTargetsByGeneration( function migrateAutomations( automations: readonly Automation[], targetsById: ReadonlyMap, - workspaceState: FolderWorkspaceHostState + workspaceState: FolderWorkspaceHostState, + storageAuthority: 'desktop' | 'runtime' ): MigratedAutomations { const knownIds = new Set(targetsById.keys()) const sshTargetIdForGeneration = indexTargetsByGeneration(targetsById) @@ -185,7 +189,7 @@ function migrateAutomations( const pin = resolveWorkspacePin(automation, workspaceState, targetsById) // Before classification: a followed re-pin is a healthy record, not an orphan. const record = followWorkspaceRepin(automation, pin, sshTargetIdForGeneration) - const classification = classifyAutomationOwner(record, knownIds, pin) + const classification = classifyAutomationOwner(record, knownIds, pin, storageAuthority) // Orphans and ambiguous records must not keep firing on a guessed host, but // they are never moved, rewritten to Self, or deleted. if (classification !== 'owned') { @@ -226,11 +230,16 @@ export function migrateAutomationOwners( }) const stamped = stampTargetGenerations(input.sshTargets, highWaterMark) const targetsById = new Map(stamped.targets.map((target) => [target.id, target])) - const migrated = migrateAutomations(input.automations, targetsById, { - folderWorkspaces: [...(input.folderWorkspaces ?? [])], - projectGroups: [...(input.projectGroups ?? [])], - repos: [...input.repos] - }) + const migrated = migrateAutomations( + input.automations, + targetsById, + { + folderWorkspaces: [...(input.folderWorkspaces ?? [])], + projectGroups: [...(input.projectGroups ?? [])], + repos: [...input.repos] + }, + input.storageAuthority ?? 'desktop' + ) return { automations: migrated.automations, sshTargets: stamped.targets, diff --git a/src/main/index.ts b/src/main/index.ts index 7fd86d94155..d755aff6e36 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -2307,13 +2307,15 @@ void app.whenReady().then(async () => { ) const activeOrcaProfile = ensureActiveOrcaProfile() - store = new Store({ dataFile: activeOrcaProfile.dataFile }) + store = new Store({ + dataFile: activeOrcaProfile.dataFile, + storageAuthority: isServeMode ? 'runtime' : 'desktop' + }) // Why here and not at install time: the report remembers what it last said, and that // state lives beside the profile data file, which does not exist until now. reportSecretProtectionGap({ dataFile: activeOrcaProfile.dataFile, force: process.env.ORCA_ALWAYS_REPORT_SECRET_PROTECTION === '1' - }) // Why here: the host key store is a sidecar of the same profile, and every SSH connect consults // it. Left unbound it reports nothing trusted, which is safe but silently discards our own // accept records on every launch. diff --git a/src/main/persistence/leasing-ssh-ptys/ssh-normalization.ts b/src/main/persistence/leasing-ssh-ptys/ssh-normalization.ts index c51d09f0e70..1e44b47106e 100644 --- a/src/main/persistence/leasing-ssh-ptys/ssh-normalization.ts +++ b/src/main/persistence/leasing-ssh-ptys/ssh-normalization.ts @@ -4,6 +4,7 @@ import type { SshTarget } from '../../../shared/ssh-types' import { LEGACY_DEFAULT_SSH_RELAY_GRACE_PERIOD_SECONDS } from '../../../shared/ssh-types' +import { sanitizeSshTargetGeneration } from '../../../shared/ssh-target-generation' export type LegacySshTarget = SshTarget & { remoteWorkspaceSyncEnabled?: unknown @@ -33,6 +34,9 @@ export function normalizeSshTarget(t: SshTarget): SshTarget { ...target, configHost: target.configHost ?? target.label ?? target.host } + if (sanitizeSshTargetGeneration(target.generation) === undefined) { + delete normalized.generation + } // Why: old SSH form persisted 10800 even without a user choice; treat that legacy default as the new implicit default. if ( relayGracePeriodSeconds !== undefined && diff --git a/src/main/persistence/leasing-ssh-ptys/ssh-target-reassignment.ts b/src/main/persistence/leasing-ssh-ptys/ssh-target-reassignment.ts index e0722e805c0..b04127fd13a 100644 --- a/src/main/persistence/leasing-ssh-ptys/ssh-target-reassignment.ts +++ b/src/main/persistence/leasing-ssh-ptys/ssh-target-reassignment.ts @@ -2,6 +2,7 @@ import type { ProjectHostSetup } from '../../../shared/project-types' import { toSshExecutionHostId } from '../../../shared/execution-host' import type { StoreOwnedPersistedState } from '../loading-store/store-owned-state' import { + migrateFolderWorkspaceHostSshTargetId, migrateUiHostScopeSshTargetId, migrateWorkspaceSessionSshTargetId } from '../../ssh/ssh-target-id-migration' @@ -11,6 +12,11 @@ import { migrateRetirementNamespaceHostIdentity, sshHostIdentity } from '../../worktree-retirement-namespace' +import { + migrateAutomationHostFilterSshTargetId, + migrateAutomationsForSshReadoption +} from '../../automations/automation-ssh-readoption-migration' +import { automationIdsPinnedToSshTarget } from '../scheduling-automations/automation-owner-projection' export type SshTargetReassignmentOperations = { state: StoreOwnedPersistedState @@ -107,6 +113,30 @@ export function reassignSshTargetId( if (migrateRetirementNamespaces(operations.state, oldTargetId, newTargetId)) { carrierChanged = true } + const workspacePinnedAutomationIds = automationIdsPinnedToSshTarget( + operations.state, + oldTargetId + ) + if (migrateFolderWorkspaceHostSshTargetId(operations.state, oldTargetId, newTargetId)) { + carrierChanged = true + } + if ( + migrateAutomationsForSshReadoption({ + automations: operations.state.automations ?? [], + automationRuns: operations.state.automationRuns ?? [], + oldTargetId, + newTargetId, + workspacePinnedAutomationIds, + newTargetGeneration: (operations.state.sshTargets ?? []).find( + (target) => target.id === newTargetId + )?.generation + }) + ) { + carrierChanged = true + } + if (migrateAutomationHostFilterSshTargetId(operations.state.ui, oldTargetId, newTargetId)) { + carrierChanged = true + } for (const lease of operations.state.sshRemotePtyLeases ?? []) { if (lease.targetId === oldTargetId) { lease.targetId = newTargetId diff --git a/src/main/persistence/leasing-ssh-ptys/ssh-target-state.ts b/src/main/persistence/leasing-ssh-ptys/ssh-target-state.ts index dafd76b1b6d..84921457624 100644 --- a/src/main/persistence/leasing-ssh-ptys/ssh-target-state.ts +++ b/src/main/persistence/leasing-ssh-ptys/ssh-target-state.ts @@ -4,9 +4,13 @@ import type { StoreOwnedPersistedState } from '../loading-store/store-owned-stat import type { ProtectedSecretPersistence } from '../../protected-secret-persistence' import { sshPtyOwnerLeaseSecretSlot } from '../../protected-secret-persistence' import { - MAX_CLAUDE_LIVE_PTY_SESSION_IDS, - MAX_REMOVED_SSH_TARGET_TOMBSTONES + MAX_CLAUDE_LIVE_PTY_SESSION_IDS } from '../restoring-sessions/pane-alias-normalization' +import { + MAX_REMOVED_SSH_TARGET_TOMBSTONES, + capRemovedSshTargetTombstones, + collectSshTargetRemovalEvidenceDependencies +} from '../../ssh/removed-ssh-target-tombstone-retention' import { normalizeSshTarget } from './ssh-normalization' import { isRuntimeOwnedSshTargetId } from '../../../shared/execution-host' import { @@ -171,13 +175,30 @@ export function addRemovedSshTargetTombstone( const existing = operations.state.removedSshTargetTombstones ?? [] // Why: dedupe by oldTargetId so re-removing the same id can't stack duplicate tombstones; newest wins. const filtered = existing.filter((entry) => entry.oldTargetId !== tombstone.oldTargetId) - // Cap the history so pathological churn can't grow the state file unbounded. - operations.state.removedSshTargetTombstones = [...filtered, tombstone].slice( - -MAX_REMOVED_SSH_TARGET_TOMBSTONES + operations.state.removedSshTargetTombstones = capRemovedSshTargetTombstones( + [...filtered, tombstone], + sshTargetRemovalEvidenceDependencies(operations.state), + MAX_REMOVED_SSH_TARGET_TOMBSTONES ) operations.scheduleSave() } +function sshTargetRemovalEvidenceDependencies(state: PersistedState): Set { + return collectSshTargetRemovalEvidenceDependencies({ + automations: state.automations ?? [], + automationHostFilter: state.ui?.automationHostFilter + }) +} + +export function releaseRemovedSshTargetTombstone( + operations: SshTargetStateOperations, + oldTargetId: string +): void { + if (!sshTargetRemovalEvidenceDependencies(operations.state).has(oldTargetId)) { + removeRemovedSshTargetTombstone(operations, oldTargetId) + } +} + export function removeRemovedSshTargetTombstone( operations: SshTargetStateOperations, oldTargetId: string diff --git a/src/main/persistence/loading-store/store.ts b/src/main/persistence/loading-store/store.ts index b977f40074e..ce9b4b8d872 100644 --- a/src/main/persistence/loading-store/store.ts +++ b/src/main/persistence/loading-store/store.ts @@ -27,6 +27,17 @@ import type { AutomationRunTrigger, AutomationUpdateInput } from '../../../shared/automations-types' +import type { + AutomationCapturedHostIssue, + AutomationChangeSelector, + AutomationListParams, + AutomationListResult +} from '../../../shared/automation-list-scope' +import type { + AutomationDestination, + AutomationOwnerFenceOperation, + AutomationOwnerPrecondition +} from '../../../shared/automation-owner-precondition' import { normalizeProxyUrl } from '../../../shared/network-proxy' import { normalizeKagiSessionLink } from '../../../shared/browser-url' import type { FolderWorkspace, WorkspaceKey } from '../../../shared/folder-workspace-types' @@ -258,6 +269,7 @@ import { removeClaudeLivePtySessionId as removeClaudeLivePtySessionIdOperation, removeDeletedSshConfigAlias as removeDeletedSshConfigAliasOperation, removeRemovedSshTargetTombstone as removeRemovedSshTargetTombstoneOperation, + releaseRemovedSshTargetTombstone as releaseRemovedSshTargetTombstoneOperation, removeSshTarget as removeSshTargetOperation, updateSshTarget as updateSshTargetOperation } from '../leasing-ssh-ptys/ssh-target-state' @@ -331,6 +343,7 @@ import { import { createAutomationRun as createAutomationRunOperation, listAutomationRuns as listAutomationRunsOperation, + recordRepeatedAutomationSkip as recordRepeatedAutomationSkipOperation, snapshotAutomationRunWorkspaceDisplayName as snapshotAutomationRunWorkspaceDisplayNameOperation, updateAutomationRun as updateAutomationRunOperation, type AutomationRunOperations @@ -358,6 +371,17 @@ import { import { hydrateRepo as hydrateRepoOperation } from '../tracking-repos/repo-hydration' import { RepoUpdatePersistenceOperations } from '../tracking-repos/repo-update-operations' import { ProjectHostSetupPersistenceOperations } from '../tracking-repos/project-host-setup-update' +import { migrateAutomationOwners } from '../../automations/automation-owner-migration' +import { + allocateSshTargetGeneration as allocateSshTargetGenerationOperation, + assertAutomationOwnerFence as assertAutomationOwnerFenceOperation, + automationCapturedHostIssue as automationCapturedHostIssueOperation, + automationChangeSelector as automationChangeSelectorOperation, + automationOwnerPrecondition as automationOwnerPreconditionOperation, + listAutomationsForScope as listAutomationsForScopeOperation, + type AutomationListProjectionCache, + type AutomationStorageAuthority +} from '../scheduling-automations/automation-owner-projection' // Why (issue #1158): keep 5 rolling backups at >=1h spacing so a corrupt/empty write leaves an earlier copy recoverable. const BACKUP_COUNT = 5 @@ -498,6 +522,7 @@ function deleteRemovedTerminalScrollbackSnapshots( export type StoreOptions = { dataFile?: string + storageAuthority?: AutomationStorageAuthority } export type PtyBindingSourceExpectation = { @@ -512,6 +537,7 @@ export class Store { // Why readonly: the operations wrappers below capture this reference once and are memoized. private readonly state: PersistedState private readonly dataFile: string + private readonly storageAuthority: AutomationStorageAuthority private readonly activeViewPreference: ActiveViewPreference private readonly terminalScrollbackSnapshotStorage: TerminalScrollbackSnapshotStorage private writeTimer: ReturnType | null = null @@ -546,10 +572,12 @@ export class Store { ) => void >() private uiChangeListeners = new Set<(ui: PersistedState['ui']) => void>() + private automationListProjectionCache: AutomationListProjectionCache | null = null constructor(options: StoreOptions = {}) { // Why: profile switching yields multiple state paths; capture per Store so late async writes can't follow a global path. this.dataFile = options.dataFile ?? getDataFile() + this.storageAuthority = options.storageAuthority ?? 'desktop' this.staleTempCleanup = removeStaleDurableWriteTempFiles(this.dataFile, { minimumAgeMs: STALE_DURABLE_WRITE_TEMP_AGE_MS }) @@ -1605,6 +1633,28 @@ export class Store { automationRuns: automationContextMigration.state.automationRuns } + const automationOwnerMigration = migrateAutomationOwners({ + automations: result.automations, + sshTargets: result.sshTargets, + repos, + folderWorkspaces: result.folderWorkspaces, + projectGroups: result.projectGroups, + removedSshTargetTombstones: result.removedSshTargetTombstones ?? [], + sshTargetGenerationCounter: result.sshTargetGenerationCounter, + storageAuthority: this.storageAuthority, + now: Date.now() + }) + if (automationOwnerMigration.changed) { + this.loadNeedsSave = true + } + result = { + ...result, + automations: automationOwnerMigration.automations, + sshTargets: automationOwnerMigration.sshTargets, + removedSshTargetTombstones: automationOwnerMigration.removedSshTargetTombstones, + sshTargetGenerationCounter: automationOwnerMigration.sshTargetGenerationCounter + } + const folderScopeConnectionMigration = backfillFolderScopeConnectionIds({ ...result, repos, @@ -1715,6 +1765,7 @@ export class Store { private static SAVE_MAX_WAIT_MS = 5_000 private scheduleSave(): void { + this.automationListProjectionCache = null // Why: once the quit flush has snapshotted, a newly debounced write would fire during // teardown with nothing awaiting it, and the process can exit mid-rename. The quit // flush is the last write by construction. @@ -2403,6 +2454,7 @@ export class Store { private getAutomationDefinitionOperations(): AutomationDefinitionOperations { return { state: this.state, + storageAuthority: this.storageAuthority, flush: () => this.flush(), recordCreated: () => this.recordFeatureInteraction('automation-created') } @@ -2422,20 +2474,67 @@ export class Store { return listAutomationsOperation(this.state) } + listAutomationsForScope(params?: AutomationListParams | null): AutomationListResult { + const projection = listAutomationsForScopeOperation({ + state: this.state, + storageAuthority: this.storageAuthority, + automations: this.listAutomations(), + params, + cache: this.automationListProjectionCache + }) + this.automationListProjectionCache = projection.cache + return projection.result + } + + automationOwnerPrecondition(id: string): AutomationOwnerPrecondition | null { + return automationOwnerPreconditionOperation(this.state, this.storageAuthority, id) + } + + automationChangeSelector(id: string): AutomationChangeSelector | null { + return automationChangeSelectorOperation(this.state, this.storageAuthority, id) + } + + automationCapturedHostIssue(automation: Automation): AutomationCapturedHostIssue | null { + return automationCapturedHostIssueOperation(this.state, this.storageAuthority, automation) + } + + assertAutomationOwnerFence(input: { + id: string + expectedOwner?: AutomationOwnerPrecondition + operation: AutomationOwnerFenceOperation + }): Automation { + return assertAutomationOwnerFenceOperation({ + state: this.state, + storageAuthority: this.storageAuthority, + ...input + }) + } + + allocateSshTargetGeneration(): number { + return allocateSshTargetGenerationOperation(this.state, () => this.scheduleSave()) + } + listAutomationRuns(automationId?: string): AutomationRun[] { return listAutomationRunsOperation(this.state, automationId) } - createAutomation(input: AutomationCreateInput): Automation { - return createAutomationOperation(this.getAutomationDefinitionOperations(), input) + createAutomation( + input: AutomationCreateInput, + options?: { destination?: AutomationDestination } + ): Automation { + return createAutomationOperation(this.getAutomationDefinitionOperations(), input, options) } - updateAutomation(id: string, updates: AutomationUpdateInput): Automation { - return updateAutomationOperation(this.getAutomationDefinitionOperations(), id, updates) + updateAutomation( + id: string, + updates: AutomationUpdateInput, + options?: { expectedOwner?: AutomationOwnerPrecondition; destination?: AutomationDestination } + ): Automation { + return updateAutomationOperation(this.getAutomationDefinitionOperations(), id, updates, options) } - deleteAutomation(id: string): void { - deleteAutomationOperation(this.getAutomationDefinitionOperations(), id) + deleteAutomation(id: string, options?: { expectedOwner?: AutomationOwnerPrecondition }): void { + deleteAutomationOperation(this.getAutomationDefinitionOperations(), id, options) } createAutomationRun( @@ -2451,6 +2550,19 @@ export class Store { ) } + recordRepeatedAutomationSkip( + automationId: string, + error: string, + scheduledFor: number + ): AutomationRun | null { + return recordRepeatedAutomationSkipOperation( + this.getAutomationRunOperations(), + automationId, + error, + scheduledFor + ) + } + updateAutomationRun(result: AutomationDispatchResult): AutomationRun { return updateAutomationRunOperation(this.getAutomationRunOperations(), result) } @@ -3557,6 +3669,10 @@ export class Store { addRemovedSshTargetTombstoneOperation(this.getSshTargetStateOperations(), tombstone) } + releaseRemovedSshTargetTombstone(oldTargetId: string): void { + releaseRemovedSshTargetTombstoneOperation(this.getSshTargetStateOperations(), oldTargetId) + } + removeRemovedSshTargetTombstone(oldTargetId: string): void { removeRemovedSshTargetTombstoneOperation(this.getSshTargetStateOperations(), oldTargetId) } @@ -3686,6 +3802,7 @@ export class Store { // ── Flush (for shutdown) ─────────────────────────────────────────── flush(): void { + this.automationListProjectionCache = null if (this.quitFlushStarted) { return } diff --git a/src/main/persistence/scheduling-automations/automation-definition-operations.ts b/src/main/persistence/scheduling-automations/automation-definition-operations.ts index 677655c575e..505551159cd 100644 --- a/src/main/persistence/scheduling-automations/automation-definition-operations.ts +++ b/src/main/persistence/scheduling-automations/automation-definition-operations.ts @@ -7,6 +7,18 @@ import type { import type { PersistedState } from '../../../shared/persisted-state-types' import { normalizeAutomationPrecheck } from '../../../shared/automation-precheck' import { nextAutomationOccurrenceAfter } from '../../../shared/automation-schedules' +import { + applyAutomationExecutionTarget, + deriveAutomationExecutionTargetForCreate, + deriveAutomationExecutionTargetForUpdate +} from '../../../shared/automation-execution-target' +import { + assertAutomationDestination, + assertAutomationOwnerFence, + assertExecutionTargetMatchesDestination, + type AutomationDestination, + type AutomationOwnerPrecondition +} from '../../../shared/automation-owner-precondition' import type { StoreOwnedPersistedState } from '../loading-store/store-owned-state' import { getAutomationContextsForRepo, @@ -14,9 +26,16 @@ import { normalizeAutomationSessionReuse, normalizeAutomationSetupDecisionForWorkspaceMode } from './automation-context-migration' +import { + automationProjectionContext, + automationWorkspaceSshPin, + sshTargetGenerationForConnection, + type AutomationStorageAuthority +} from './automation-owner-projection' export type AutomationDefinitionOperations = { state: StoreOwnedPersistedState + storageAuthority: AutomationStorageAuthority flush: () => void recordCreated: () => void } @@ -29,11 +48,27 @@ export function listAutomations(state: PersistedState): Automation[] { export function createAutomation( operations: AutomationDefinitionOperations, - input: AutomationCreateInput + input: AutomationCreateInput, + options?: { destination?: AutomationDestination } ): Automation { const repo = operations.state.repos.find((entry) => entry.id === input.projectId) const now = Date.now() - const executionTargetType = repo?.connectionId ? 'ssh' : 'local' + const workspaceId = input.workspaceMode === 'existing' ? (input.workspaceId ?? null) : null + const workspaceSshPin = automationWorkspaceSshPin(operations.state, workspaceId) + const executionTarget = deriveAutomationExecutionTargetForCreate({ + repo, + sshTargetGeneration: sshTargetGenerationForConnection(operations.state, repo?.connectionId), + workspaceSshPin + }) + if (options?.destination) { + const context = automationProjectionContext( + operations.state, + operations.storageAuthority, + false + ) + assertAutomationDestination(options.destination, context) + assertExecutionTargetMatchesDestination(executionTarget, options.destination, workspaceSshPin) + } const schedulerOwner = getAutomationSchedulerOwner(repo) const contexts = getAutomationContextsForRepo(repo, operations.state.projectHostSetups ?? []) const automation: Automation = { @@ -45,11 +80,10 @@ export function createAutomation( runContext: input.runContext ?? contexts.runContext, sourceContext: input.sourceContext ?? contexts.sourceContext, projectId: input.projectId, - executionTargetType, - executionTargetId: executionTargetType === 'ssh' ? (repo?.connectionId ?? '') : 'local', + ...executionTarget, schedulerOwner, workspaceMode: input.workspaceMode, - workspaceId: input.workspaceMode === 'existing' ? (input.workspaceId ?? null) : null, + workspaceId, baseBranch: input.workspaceMode === 'new_per_run' ? (input.baseBranch ?? null) : null, setupDecision: normalizeAutomationSetupDecisionForWorkspaceMode( input.workspaceMode, @@ -75,13 +109,28 @@ export function createAutomation( export function updateAutomation( operations: AutomationDefinitionOperations, id: string, - updates: AutomationUpdateInput + updates: AutomationUpdateInput, + options?: { expectedOwner?: AutomationOwnerPrecondition; destination?: AutomationDestination } ): Automation { const index = (operations.state.automations ?? []).findIndex((entry) => entry.id === id) if (index === -1) { throw new Error('Automation not found.') } const current = operations.state.automations[index] + const projectionContext = automationProjectionContext( + operations.state, + operations.storageAuthority, + false + ) + assertAutomationOwnerFence({ + automation: current, + expectedOwner: options?.expectedOwner, + operation: 'mutate', + context: projectionContext + }) + if (options?.destination) { + assertAutomationDestination(options.destination, projectionContext) + } // Why: the renderer forwards a Partial verbatim, so `{ enabled: undefined }` survives structuredClone // and would blank the stored value in the spread below. Explicit clears go through the `null` branches. const definedUpdates = Object.fromEntries( @@ -89,14 +138,16 @@ export function updateAutomation( ) as AutomationUpdateInput const repoId = updates.projectId ?? current.projectId const repo = operations.state.repos.find((entry) => entry.id === repoId) - const executionTargetType = repo?.connectionId ? 'ssh' : 'local' - const schedulerOwner = getAutomationSchedulerOwner(repo) + const selectorMoveRequested = + updates.projectId !== undefined || options?.destination !== undefined + const schedulerOwner = + selectorMoveRequested && repo ? getAutomationSchedulerOwner(repo) : current.schedulerOwner const contexts = getAutomationContextsForRepo(repo, operations.state.projectHostSetups ?? []) const rrule = updates.rrule ?? current.rrule const dtstart = updates.dtstart ?? current.dtstart const scheduleChanged = updates.rrule !== undefined || updates.dtstart !== undefined const workspaceMode = updates.workspaceMode ?? current.workspaceMode - const updated: Automation = { + const merged: Automation = { ...current, ...definedUpdates, name: updates.name !== undefined ? updates.name.trim() || 'Untitled automation' : current.name, @@ -114,8 +165,6 @@ export function updateAutomation( : updates.projectId !== undefined ? contexts.sourceContext : (current.sourceContext ?? contexts.sourceContext), - executionTargetType, - executionTargetId: executionTargetType === 'ssh' ? (repo?.connectionId ?? '') : 'local', schedulerOwner, workspaceMode, workspaceId: @@ -148,14 +197,47 @@ export function updateAutomation( nextRunAt: scheduleChanged ? nextAutomationOccurrenceAfter(rrule, dtstart, Date.now()) : current.nextRunAt, + enabledDecidedBy: updates.enabled !== undefined ? 'user' : current.enabledDecidedBy, updatedAt: Date.now() } + const previousPin = automationWorkspaceSshPin(operations.state, current.workspaceId) + const workspaceSshPin = automationWorkspaceSshPin(operations.state, merged.workspaceId) + const workspaceSshPinMoved = previousPin?.targetId !== workspaceSshPin?.targetId + const executionTarget = deriveAutomationExecutionTargetForUpdate({ + current, + repo, + selectorMoveRequested, + sshTargetGeneration: sshTargetGenerationForConnection(operations.state, repo?.connectionId), + workspaceSshPin, + workspaceSshPinMoved + }) + if (options?.destination) { + assertExecutionTargetMatchesDestination(executionTarget, options.destination, workspaceSshPin) + } + const updated = applyAutomationExecutionTarget( + merged, + executionTarget, + workspaceSshPinMoved ? undefined : workspaceSshPin + ) operations.state.automations[index] = updated operations.flush() return updated } -export function deleteAutomation(operations: AutomationDefinitionOperations, id: string): void { +export function deleteAutomation( + operations: AutomationDefinitionOperations, + id: string, + options?: { expectedOwner?: AutomationOwnerPrecondition } +): void { + const automation = (operations.state.automations ?? []).find((entry) => entry.id === id) + if (automation) { + assertAutomationOwnerFence({ + automation, + expectedOwner: options?.expectedOwner, + operation: 'mutate', + context: automationProjectionContext(operations.state, operations.storageAuthority, false) + }) + } operations.state.automations = (operations.state.automations ?? []).filter( (entry) => entry.id !== id ) diff --git a/src/main/persistence/scheduling-automations/automation-owner-projection.ts b/src/main/persistence/scheduling-automations/automation-owner-projection.ts new file mode 100644 index 00000000000..3f842a0310d --- /dev/null +++ b/src/main/persistence/scheduling-automations/automation-owner-projection.ts @@ -0,0 +1,268 @@ +import type { Automation, AutomationRun } from '../../../shared/automations-types' +import type { PersistedState } from '../../../shared/persisted-state-types' +import { + automationCapturedHostIssue as resolveAutomationCapturedHostIssue, + automationSelectorMatchesScope, + projectAutomationList, + projectAutomationSelector, + toAutomationChangeSelector, + type AutomationCapturedHostIssue, + type AutomationChangeSelector, + type AutomationListParams, + type AutomationListResult, + type AutomationProjectionContext +} from '../../../shared/automation-list-scope' +import { + assertAutomationOwnerFence as enforceAutomationOwnerFence, + toAutomationOwnerPrecondition, + type AutomationOwnerFenceOperation, + type AutomationOwnerPrecondition +} from '../../../shared/automation-owner-precondition' +import { + AUTOMATION_OWNER_CONFLICT_CODES, + AutomationOwnerConflictError +} from '../../../shared/automation-owner-conflict' +import { + nextSshTargetGeneration, + resolveSshTargetGenerationHighWaterMark, + sanitizeSshTargetGeneration +} from '../../../shared/ssh-target-generation' +import { + resolveAutomationWorkspaceHost, + resolveAutomationWorkspaceSshTargetId, + type AutomationWorkspaceSshPin +} from '../../../shared/automation-workspace-pin' +import { summarizeAutomationRunUsage } from '../../../shared/automation-usage-summary' +import type { FolderWorkspaceHostState } from '../../../shared/folder-workspace-execution-host' +import type { StoreOwnedPersistedState } from '../loading-store/store-owned-state' + +export type AutomationStorageAuthority = 'desktop' | 'runtime' + +export type AutomationListProjectionCache = { + inputs: readonly unknown[] + result: AutomationListResult +} + +export function sshTargetGenerationForConnection( + state: PersistedState, + connectionId: string | null | undefined +): number | undefined { + const targetId = connectionId?.trim() + const target = targetId ? state.sshTargets?.find((entry) => entry.id === targetId) : undefined + return sanitizeSshTargetGeneration(target?.generation) +} + +function automationUsageSummaries(state: PersistedState) { + const runsByAutomation = new Map() + for (const run of state.automationRuns ?? []) { + const runs = runsByAutomation.get(run.automationId) + if (runs) { + runs.push(run) + } else { + runsByAutomation.set(run.automationId, [run]) + } + } + return new Map([...runsByAutomation].map(([id, runs]) => [id, summarizeAutomationRunUsage(runs)])) +} + +function folderWorkspaceHostState(state: PersistedState): FolderWorkspaceHostState { + return { + folderWorkspaces: state.folderWorkspaces ?? [], + projectGroups: state.projectGroups ?? [], + repos: state.repos ?? [] + } +} + +export function automationIdsPinnedToSshTarget( + state: PersistedState, + targetId: string +): Set { + const hostState = folderWorkspaceHostState(state) + return new Set( + (state.automations ?? []) + .filter( + (automation) => + resolveAutomationWorkspaceSshTargetId(hostState, automation.workspaceId) === targetId + ) + .map((automation) => automation.id) + ) +} + +export function automationWorkspaceSshPin( + state: PersistedState, + workspaceId: Automation['workspaceId'] +): AutomationWorkspaceSshPin | undefined { + const targetId = resolveAutomationWorkspaceSshTargetId( + folderWorkspaceHostState(state), + workspaceId + ) + return targetId === undefined + ? undefined + : { + targetId, + generation: sshTargetGenerationForConnection(state, targetId) + } +} + +export function automationProjectionContext( + state: PersistedState, + storageAuthority: AutomationStorageAuthority, + withUsage: boolean +): AutomationProjectionContext { + const usage = withUsage ? automationUsageSummaries(state) : null + const reposById = new Map(state.repos.map((repo) => [repo.id, repo])) + const targetIdByGeneration = new Map( + (state.sshTargets ?? []).flatMap((target) => + target.generation === undefined ? [] : [[target.generation, target.id] as const] + ) + ) + return { + storageAuthority, + sshTargetGeneration: (targetId) => sshTargetGenerationForConnection(state, targetId), + sshTargetIdForGeneration: (generation) => targetIdByGeneration.get(generation), + repoConnectionId: (repoId) => { + const repo = reposById.get(repoId) + return repo ? repo.connectionId?.trim() || null : undefined + }, + workspaceHost: (automation) => + resolveAutomationWorkspaceHost(folderWorkspaceHostState(state), automation.workspaceId), + ...(usage ? { usageSummary: (id: string) => usage.get(id) ?? null } : {}) + } +} + +function projectionInputs(state: PersistedState): readonly unknown[] { + return [ + state.automations, + state.automationRuns, + state.repos, + state.sshTargets, + state.folderWorkspaces, + state.projectGroups + ] +} + +export function listAutomationsForScope(input: { + state: PersistedState + storageAuthority: AutomationStorageAuthority + automations: readonly Automation[] + params?: AutomationListParams | null + cache: AutomationListProjectionCache | null +}): { result: AutomationListResult; cache: AutomationListProjectionCache } { + const scope = input.params?.selector + if (scope?.kind === 'ssh') { + const current = sshTargetGenerationForConnection(input.state, scope.targetId) + if (current === undefined) { + throw new AutomationOwnerConflictError(AUTOMATION_OWNER_CONFLICT_CODES.targetRemoved) + } + if (current !== scope.expectedTargetGeneration) { + throw new AutomationOwnerConflictError(AUTOMATION_OWNER_CONFLICT_CODES.ownerChanged) + } + } + const inputs = projectionInputs(input.state) + const complete = + input.cache && inputs.every((entry, index) => entry === input.cache?.inputs[index]) + ? input.cache + : { + inputs, + result: projectAutomationList( + input.automations, + automationProjectionContext(input.state, input.storageAuthority, true) + ) + } + if (!scope) { + return { result: complete.result, cache: complete } + } + const items = complete.result.items.filter((item) => + automationSelectorMatchesScope(item.selector, scope) + ) + const ids = new Set(items.map((item) => item.automationId)) + return { + result: { + automations: complete.result.automations.filter((automation) => ids.has(automation.id)), + items, + orphanCount: complete.result.orphanCount + }, + cache: complete + } +} + +export function automationOwnerPrecondition( + state: PersistedState, + storageAuthority: AutomationStorageAuthority, + id: string +): AutomationOwnerPrecondition | null { + const automation = (state.automations ?? []).find((entry) => entry.id === id) + return automation + ? toAutomationOwnerPrecondition( + projectAutomationSelector( + automation, + automationProjectionContext(state, storageAuthority, false) + ) + ) + : null +} + +export function automationChangeSelector( + state: PersistedState, + storageAuthority: AutomationStorageAuthority, + id: string +): AutomationChangeSelector | null { + const automation = (state.automations ?? []).find((entry) => entry.id === id) + return automation + ? toAutomationChangeSelector( + projectAutomationSelector( + automation, + automationProjectionContext(state, storageAuthority, false) + ) + ) + : null +} + +export function automationCapturedHostIssue( + state: PersistedState, + storageAuthority: AutomationStorageAuthority, + automation: Automation +): AutomationCapturedHostIssue | null { + return resolveAutomationCapturedHostIssue( + automation, + automationProjectionContext(state, storageAuthority, false) + ) +} + +export function assertAutomationOwnerFence(input: { + state: PersistedState + storageAuthority: AutomationStorageAuthority + id: string + expectedOwner?: AutomationOwnerPrecondition + operation: AutomationOwnerFenceOperation +}): Automation { + const automation = (input.state.automations ?? []).find((entry) => entry.id === input.id) + if (!automation) { + throw new Error('Automation not found.') + } + enforceAutomationOwnerFence({ + automation, + expectedOwner: input.expectedOwner, + operation: input.operation, + context: automationProjectionContext(input.state, input.storageAuthority, false) + }) + return automation +} + +export function allocateSshTargetGeneration( + state: StoreOwnedPersistedState, + scheduleSave: () => void +): number { + const next = nextSshTargetGeneration( + resolveSshTargetGenerationHighWaterMark({ + persistedCounter: state.sshTargetGenerationCounter, + targetGenerations: (state.sshTargets ?? []).map((target) => target.generation), + capturedGenerations: (state.automations ?? []).map( + (automation) => automation.executionTargetGeneration + ) + }) + ) + state.sshTargetGenerationCounter = next + scheduleSave() + return next +} diff --git a/src/main/persistence/scheduling-automations/automation-run-operations.ts b/src/main/persistence/scheduling-automations/automation-run-operations.ts index f83117edf8b..a3e93d8a974 100644 --- a/src/main/persistence/scheduling-automations/automation-run-operations.ts +++ b/src/main/persistence/scheduling-automations/automation-run-operations.ts @@ -89,6 +89,46 @@ export function createAutomationRun( return run } +export function recordRepeatedAutomationSkip( + operations: AutomationRunOperations, + automationId: string, + error: string, + scheduledFor: number +): AutomationRun | null { + const runs = operations.state.automationRuns ?? [] + const latest = runs + .filter((run) => run.automationId === automationId) + .reduce( + (newest, run) => (!newest || run.createdAt > newest.createdAt ? run : newest), + null + ) + if ( + !latest || + latest.status !== 'skipped_unavailable' || + latest.trigger !== 'scheduled' || + latest.error !== error + ) { + return null + } + if ((latest.lastOccurrenceAt ?? latest.scheduledFor) === scheduledFor) { + return latest + } + const now = Date.now() + const updated: AutomationRun = { + ...latest, + occurrenceCount: (latest.occurrenceCount ?? 1) + 1, + lastOccurrenceAt: scheduledFor + } + runs[runs.findIndex((run) => run.id === latest.id)] = updated + const automation = operations.state.automations.find((entry) => entry.id === automationId) + if (automation) { + automation.lastRunAt = now + automation.updatedAt = now + } + operations.flush() + return updated +} + export function updateAutomationRun( operations: AutomationRunOperations, result: AutomationDispatchResult diff --git a/src/main/runtime/rpc/methods/automation-scoped-list-methods.test.ts b/src/main/runtime/rpc/methods/automation-scoped-list-methods.test.ts index 4f07604738f..322cfec00df 100644 --- a/src/main/runtime/rpc/methods/automation-scoped-list-methods.test.ts +++ b/src/main/runtime/rpc/methods/automation-scoped-list-methods.test.ts @@ -1,7 +1,7 @@ /** * The runtime end of the list contract: an old client that sends no params must - * keep receiving the authority's complete list in the legacy shape, and every - * owner precondition must survive schema parsing rather than being stripped. + * keep receiving the authority's complete list through the legacy field while + * current callers also receive owner metadata. */ import { describe, expect, it, vi } from 'vitest' import type { RpcContext, RpcRequest } from '../core' @@ -20,11 +20,22 @@ function method(name: string) { function runtimeStub() { return { listAutomations: vi.fn(() => [{ id: 'a1' }, { id: 'a2' }]), - listAutomationsForScope: vi.fn(() => ({ - automations: [{ id: 'a1' }], - items: [{ automationId: 'a1', selector: { kind: 'self' } }], - orphanCount: 2 - })), + listAutomationsForScope: vi.fn((params?: { selector?: { kind: string } }) => + params?.selector + ? { + automations: [{ id: 'a1' }], + items: [{ automationId: 'a1', selector: { kind: 'self' } }], + orphanCount: 2 + } + : { + automations: [{ id: 'a1' }, { id: 'a2' }], + items: [ + { automationId: 'a1', selector: { kind: 'self' } }, + { automationId: 'a2', selector: { kind: 'orphan', issue: 'missing' } } + ], + orphanCount: 1 + } + ), listAutomationRuns: vi.fn(() => []), showAutomation: vi.fn(() => ({ id: 'a1' })), automationOwnerPrecondition: vi.fn(() => SSH_OWNER), @@ -49,18 +60,20 @@ async function invoke(name: string, params: unknown, runtime: ReturnType { - it('answers a parameterless request with the complete legacy payload', async () => { + it('answers a parameterless request with owner metadata and the legacy field', async () => { const runtime = runtimeStub() - expect(await invoke('automation.list', undefined, runtime)).toEqual({ - automations: [{ id: 'a1' }, { id: 'a2' }] + expect(await invoke('automation.list', undefined, runtime)).toMatchObject({ + automations: [{ id: 'a1' }, { id: 'a2' }], + items: [{ automationId: 'a1' }, { automationId: 'a2' }] }) - expect(runtime.listAutomationsForScope).not.toHaveBeenCalled() + expect(runtime.listAutomationsForScope).toHaveBeenCalledWith({}) }) it('treats an empty object the same as no params', async () => { const runtime = runtimeStub() - expect(await invoke('automation.list', {}, runtime)).toEqual({ - automations: [{ id: 'a1' }, { id: 'a2' }] + expect(await invoke('automation.list', {}, runtime)).toMatchObject({ + automations: [{ id: 'a1' }, { id: 'a2' }], + items: [{ automationId: 'a1' }, { automationId: 'a2' }] }) }) @@ -102,7 +115,7 @@ describe('automation.list from a client that sends literal null params', () => { const response = await dispatcher.dispatch(request) expect(response).toMatchObject({ result: { automations: [{ id: 'a1' }, { id: 'a2' }] } }) - expect(runtime.listAutomationsForScope).not.toHaveBeenCalled() + expect(runtime.listAutomationsForScope).toHaveBeenCalledWith({}) }) }) diff --git a/src/main/runtime/rpc/methods/automations.test.ts b/src/main/runtime/rpc/methods/automations.test.ts index eaa5bee5493..d972bf6a553 100644 --- a/src/main/runtime/rpc/methods/automations.test.ts +++ b/src/main/runtime/rpc/methods/automations.test.ts @@ -13,6 +13,10 @@ describe('automation RPC methods', () => { const runtime = { getRuntimeId: () => 'test-runtime', listAutomations: vi.fn().mockReturnValue([{ id: 'auto-1', name: 'Daily review' }]), + listAutomationsForScope: vi.fn().mockReturnValue({ + automations: [{ id: 'auto-1', name: 'Daily review' }], + items: [{ automationId: 'auto-1', selector: { kind: 'self' } }] + }), showAutomation: vi.fn().mockReturnValue({ id: 'auto-1', name: 'Daily review' }), createAutomation: vi.fn().mockResolvedValue({ id: 'auto-2', name: 'New review' }), updateAutomation: vi.fn().mockResolvedValue({ id: 'auto-1', name: 'Paused' }), @@ -70,7 +74,7 @@ describe('automation RPC methods', () => { await dispatcher.dispatch(makeRequest('automation.runNow', { id: 'auto-1' })) await dispatcher.dispatch(makeRequest('automation.runs', { automationId: 'auto-1' })) - expect(runtime.listAutomations).toHaveBeenCalled() + expect(runtime.listAutomationsForScope).toHaveBeenCalledWith({}) // A legacy client sends no precondition, so each call forwards an absent expected owner. expect(runtime.showAutomation).toHaveBeenCalledWith('auto-1', undefined) expect(runtime.createAutomation).toHaveBeenCalledWith( diff --git a/src/main/runtime/rpc/methods/automations.ts b/src/main/runtime/rpc/methods/automations.ts index cc4573d9a7a..463d1638fac 100644 --- a/src/main/runtime/rpc/methods/automations.ts +++ b/src/main/runtime/rpc/methods/automations.ts @@ -11,11 +11,8 @@ export const AUTOMATION_METHODS: RpcMethod[] = [ defineMethod({ name: 'automation.list', params: AutomationList, - // Why: a scoped caller gets the qualified projection; a parameterless one keeps the legacy payload. - handler: (params, { runtime }) => - params.selector - ? runtime.listAutomationsForScope({ selector: params.selector }) - : { automations: runtime.listAutomations() } + // The projection retains `automations`, so old clients ignore the added owner metadata. + handler: (params, { runtime }) => runtime.listAutomationsForScope(params) }), defineMethod({ name: 'automation.show', diff --git a/src/renderer/src/components/automations/AutomationCreateDestinationField.test.tsx b/src/renderer/src/components/automations/AutomationCreateDestinationField.test.tsx index f194c95039d..1df0a7519bf 100644 --- a/src/renderer/src/components/automations/AutomationCreateDestinationField.test.tsx +++ b/src/renderer/src/components/automations/AutomationCreateDestinationField.test.tsx @@ -8,7 +8,7 @@ import { AutomationCreateDestinationField } from './AutomationCreateDestinationF import type { AutomationHostCatalogEntry } from './automation-host-catalog-types' import type { AutomationOwnerRef } from '../../../../shared/automation-owner-ref' import type { AutomationCreateDestinationControl } from './use-automation-create-destination' -import type { Repo } from '../../../../shared/types' +import type { Repo } from '../../../../shared/repo-types' let container: HTMLDivElement let root: Root diff --git a/src/renderer/src/components/automations/AutomationListHostGroups.test.tsx b/src/renderer/src/components/automations/AutomationListHostGroups.test.tsx index 2288fa79a9d..7c766ad9008 100644 --- a/src/renderer/src/components/automations/AutomationListHostGroups.test.tsx +++ b/src/renderer/src/components/automations/AutomationListHostGroups.test.tsx @@ -77,6 +77,7 @@ async function render(options: { searchActive={options.searchActive ?? false} selectedRowKey={null} isSelectedLocal + lastRunByAutomationId={new Map()} relativeNow={0} repoMap={new Map()} worktreeMap={new Map()} diff --git a/src/renderer/src/components/automations/AutomationListLocalRows.tsx b/src/renderer/src/components/automations/AutomationListLocalRows.tsx index 197f15db915..e157bd2a15c 100644 --- a/src/renderer/src/components/automations/AutomationListLocalRows.tsx +++ b/src/renderer/src/components/automations/AutomationListLocalRows.tsx @@ -60,6 +60,8 @@ export type AutomationListLocalRowsProps = { relativeNow: number repoMap: ReadonlyMap worktreeMap: ReadonlyMap + repoForRow?: (row: AutomationListRow) => Repo | undefined + worktreeForRow?: (row: AutomationListRow, repo: Repo | undefined) => Worktree | undefined projectHostSetups: readonly ProjectHostSetup[] sshConnectionStates: ReadonlyMap> runtimeStatusByEnvironmentId: ReadonlyMap< @@ -77,6 +79,8 @@ export type AutomationListLocalRowsProps = { onDelete: (row: AutomationListRow) => void } +const EMPTY_HOST_LABELS: ReadonlyMap = new Map() + function automationUsageText(summary: AutomationUsageSummary | undefined): string { if (!summary || summary.unavailableRuns > 0) { return summary?.knownRuns @@ -110,12 +114,14 @@ export function AutomationListLocalRows({ relativeNow, repoMap, worktreeMap, + repoForRow, + worktreeForRow, projectHostSetups, sshConnectionStates, runtimeStatusByEnvironmentId, hostTargetFor, automationSourceHostAvailabilityByRowKey, - hostLabelById = new Map(), + hostLabelById = EMPTY_HOST_LABELS, isActionEnabled, onSelect, onRunNow, @@ -129,9 +135,9 @@ export function AutomationListLocalRows({ <> {rows.map((row) => { const { automation } = row - const automationRepo = repoMap.get(getAutomationRunRepoId(automation)) + const automationRepo = repoForRow?.(row) ?? repoMap.get(getAutomationRunRepoId(automation)) const automationWorktree = automation.workspaceId - ? worktreeMap.get(automation.workspaceId) + ? (worktreeForRow?.(row, automationRepo) ?? worktreeMap.get(automation.workspaceId)) : null const automationRunAvailability = getAutomationTargetAvailability({ automation, diff --git a/src/renderer/src/components/automations/AutomationsListPanel.tsx b/src/renderer/src/components/automations/AutomationsListPanel.tsx index 0a1c0efd0e7..692a257f098 100644 --- a/src/renderer/src/components/automations/AutomationsListPanel.tsx +++ b/src/renderer/src/components/automations/AutomationsListPanel.tsx @@ -73,6 +73,8 @@ type AutomationsListPanelProps = { relativeNow: number repoMap: ReadonlyMap worktreeMap: ReadonlyMap + repoForRow?: (row: AutomationListRow) => Repo | undefined + worktreeForRow?: (row: AutomationListRow, repo: Repo | undefined) => Worktree | undefined projectHostSetups: readonly ProjectHostSetup[] sshConnectionStates: ReadonlyMap> runtimeStatusByEnvironmentId: ReadonlyMap< @@ -129,6 +131,8 @@ export function AutomationsListPanel(props: AutomationsListPanelProps): React.JS relativeNow, repoMap, worktreeMap, + repoForRow, + worktreeForRow, projectHostSetups, sshConnectionStates, runtimeStatusByEnvironmentId, @@ -222,6 +226,8 @@ export function AutomationsListPanel(props: AutomationsListPanelProps): React.JS relativeNow, repoMap, worktreeMap, + repoForRow, + worktreeForRow, projectHostSetups, sshConnectionStates, runtimeStatusByEnvironmentId, diff --git a/src/renderer/src/components/automations/AutomationsPage.tsx b/src/renderer/src/components/automations/AutomationsPage.tsx index 5d4852468de..9bcf18d1ea3 100644 --- a/src/renderer/src/components/automations/AutomationsPage.tsx +++ b/src/renderer/src/components/automations/AutomationsPage.tsx @@ -18,7 +18,6 @@ import type { AutomationRun, AutomationUpdateInput } from '../../../../shared/automations-types' -import { getAutomationRunRepoId } from '../../../../shared/automation-run-identity' import { AUTOMATION_OWNER_CONFLICT_CODES, automationOwnerConflictMessage @@ -33,6 +32,7 @@ import { getHostDisplayLabelOverrides } from '../../../../shared/host-setting-ov import type { OrcaHooks } from '../../../../shared/orca-yaml-hook-types' import type { Repo } from '../../../../shared/repo-types' import { getWorktreePathBasenameFromId } from '../../../../shared/worktree/id' +import type { Worktree } from '../../../../shared/worktree/types' import { buildAutomationRrule, isValidAutomationCronSchedule, @@ -98,7 +98,12 @@ import { import { buildExternalAutomationListEntries } from './external-automation-list-entries' import type { ExternalAutomationScope } from './external-automation-scope-client' import { useAutomationListSearch } from './use-automation-list-search' -import { unscopedAutomationListRows, type AutomationListRow } from './automation-list-row-identity' +import { + automationRepoForRow, + automationWorktreeForRow, + unscopedAutomationListRows, + type AutomationListRow +} from './automation-list-row-identity' import { AutomationDeleteDialog, ExternalAutomationDeleteDialog } from './AutomationDeleteDialogs' import { AutomationsListPanel } from './AutomationsListPanel' import { AutomationsDetailPane } from './AutomationsDetailPane' @@ -192,6 +197,19 @@ export default function AutomationsPage(): React.JSX.Element { const setPendingAutomationRunNavigation = useAppStore((s) => s.setPendingAutomationRunNavigation) const repoMap = useRepoMap() const worktreeMap = useWorktreeMap() + const repoForRow = useCallback( + (row: AutomationListRow): Repo | undefined => automationRepoForRow(row, repos, repoMap), + [repoMap, repos] + ) + const worktreeForRow = useCallback( + ( + row: AutomationListRow, + repo: Repo | undefined, + workspaceId: string | null | undefined = row.automation.workspaceId + ): Worktree | undefined => + automationWorktreeForRow(row, worktreesByRepo, repo, worktreeMap, workspaceId), + [worktreeMap, worktreesByRepo] + ) const enabledAgents = filterEnabledTuiAgents(AGENTS, settings?.disabledTuiAgents) const defaultAgent = settings?.defaultTuiAgent && @@ -435,13 +453,15 @@ export default function AutomationsPage(): React.JSX.Element { return run } const displayName = - worktreeMap.get(run.workspaceId)?.displayName ?? + (selectedRow + ? worktreeForRow(selectedRow, repoForRow(selectedRow), run.workspaceId)?.displayName + : worktreeMap.get(run.workspaceId)?.displayName) ?? workspaceNameCacheRef.current.get(run.workspaceId) ?? getWorktreePathBasenameFromId(run.workspaceId) const trimmedDisplayName = displayName?.trim() return trimmedDisplayName ? { ...run, workspaceDisplayName: trimmedDisplayName } : run }), - [selectedAutomationRuns.runs, worktreeMap] + [repoForRow, selectedAutomationRuns.runs, selectedRow, worktreeForRow, worktreeMap] ) const getDraftSetupDecisionDefault = useCallback( ( @@ -536,6 +556,18 @@ export default function AutomationsPage(): React.JSX.Element { const selectedAutomationRunPage = selectedAutomationRunPageId ? (selectedRuns.find((run) => run.id === selectedAutomationRunPageId) ?? null) : null + const selectedRunWorktreeMap = useMemo(() => { + if (!selectedRow) { + return worktreeMap + } + const repo = repoForRow(selectedRow) + return new Map( + selectedRuns.flatMap((run) => { + const worktree = worktreeForRow(selectedRow, repo, run.workspaceId) + return worktree ? [[worktree.id, worktree] as const] : [] + }) + ) + }, [repoForRow, selectedRow, selectedRuns, worktreeForRow, worktreeMap]) const worktrees = useMemo( () => worktreesByRepo[draft.projectId] ?? [], [draft.projectId, worktreesByRepo] @@ -764,7 +796,13 @@ export default function AutomationsPage(): React.JSX.Element { return ids }, [unifiedTabsByWorktree]) const selectedAutomationRunPageWorktree = selectedAutomationRunPage?.workspaceId - ? (worktreeMap.get(selectedAutomationRunPage.workspaceId) ?? null) + ? selectedRow + ? (worktreeForRow( + selectedRow, + repoForRow(selectedRow), + selectedAutomationRunPage.workspaceId + ) ?? null) + : (worktreeMap.get(selectedAutomationRunPage.workspaceId) ?? null) : null const selectedAutomationRunPageWorkspaceDisplay = selectedAutomationRunPage ? getAutomationRunWorkspaceDisplay({ @@ -802,9 +840,11 @@ export default function AutomationsPage(): React.JSX.Element { const isSelectedAutomationRunPageRerunPending = selectedAutomationRunPage !== null && rerunRunIdsInFlight.has(selectedAutomationRunPage.id) const automationSourceHostAvailabilityByRowKey = useAutomationSourceHostAvailability(visibleRows) - const selectedRepo = selected ? (repoMap.get(getAutomationRunRepoId(selected)) ?? null) : null + const selectedRepo = selectedRow ? (repoForRow(selectedRow) ?? null) : null const selectedWorktree = - selected && selected.workspaceId ? (worktreeMap.get(selected.workspaceId) ?? null) : null + selectedRow && selected?.workspaceId + ? (worktreeForRow(selectedRow, selectedRepo ?? undefined) ?? null) + : null const selectedRunNowAvailability = selectedRow ? getAutomationTargetAvailability({ automation: selectedRow.automation, @@ -1498,7 +1538,13 @@ export default function AutomationsPage(): React.JSX.Element { }) setDraft((current) => ({ ...current, name: '', prompt: '' })) await refresh() - selectAutomationId(automation.id) + if (editingAutomationId && editingRowKey) { + setSelectedAutomationRunPageId(null) + setSelectedRowKey(editingRowKey) + setSelectedId(automation.id) + } else { + selectAutomationId(automation.id) + } setCreateOpen(false) if (!editingAutomationId) { useAppStore.getState().recordFeatureInteraction('automation-created') @@ -1616,9 +1662,9 @@ export default function AutomationsPage(): React.JSX.Element { const runNow = async (row: AutomationListRow): Promise => { const automation = row.automation - const repo = repoMap.get(getAutomationRunRepoId(automation)) ?? null + const repo = repoForRow(row) ?? null const workspace = automation.workspaceId - ? (worktreeMap.get(automation.workspaceId) ?? null) + ? (worktreeForRow(row, repo ?? undefined) ?? null) : null const rowHostTarget = automationHostTargetFor(row) const availability = getAutomationTargetAvailability({ @@ -1819,7 +1865,10 @@ export default function AutomationsPage(): React.JSX.Element { } const openRunWorkspace = (run: AutomationRun): void => { - const runWorktree = run.workspaceId ? (worktreeMap.get(run.workspaceId) ?? null) : null + const runWorktree = + run.workspaceId && selectedRow + ? (worktreeForRow(selectedRow, repoForRow(selectedRow), run.workspaceId) ?? null) + : null const store = useAppStore.getState() const openTabId = getAutomationRunOpenTabId(run) const terminalTabExists = openTabId ? Boolean(store.getTab(openTabId)) : false @@ -2078,7 +2127,7 @@ export default function AutomationsPage(): React.JSX.Element { selectedAutomationRunPageViewState={selectedAutomationRunPageViewState} canRerunSelectedAutomationRunPage={canRerunSelectedAutomationRunPage} isSelectedAutomationRunPageRerunPending={isSelectedAutomationRunPageRerunPending} - worktreeMap={worktreeMap} + worktreeMap={selectedRunWorktreeMap} fetchExternalAutomationRuns={fetchExternalAutomationRuns} onActivePaneTabChange={setActivePaneTab} onClearExternalRunPage={() => setSelectedExternalRunPage(null)} @@ -2130,6 +2179,8 @@ export default function AutomationsPage(): React.JSX.Element { relativeNow={relativeNow} repoMap={repoMap} worktreeMap={worktreeMap} + repoForRow={repoForRow} + worktreeForRow={worktreeForRow} projectHostSetups={projectHostSetups} sshConnectionStates={sshConnectionStates} runtimeStatusByEnvironmentId={runtimeStatusByEnvironmentId} diff --git a/src/renderer/src/components/automations/automation-host-list-rows.ts b/src/renderer/src/components/automations/automation-host-list-rows.ts index 3ae397cc535..68b37e34431 100644 --- a/src/renderer/src/components/automations/automation-host-list-rows.ts +++ b/src/renderer/src/components/automations/automation-host-list-rows.ts @@ -139,6 +139,7 @@ export function resolveAutomationHostListRows( const listRow: AutomationListRow = { key: automationListRowKey(entry.stableKey, row.automation.id), automation: row.automation, + catalogRef: entry.stableRef, hostLabel: entry.label, usageSummary: row.usageSummary ?? null } diff --git a/src/renderer/src/components/automations/automation-list-row-identity.test.ts b/src/renderer/src/components/automations/automation-list-row-identity.test.ts index a1e89da8b8c..4dbc19b6944 100644 --- a/src/renderer/src/components/automations/automation-list-row-identity.test.ts +++ b/src/renderer/src/components/automations/automation-list-row-identity.test.ts @@ -17,6 +17,7 @@ import type { } from './automation-host-catalog-types' import type { AutomationHostFilterResolution } from './automation-host-filter-resolution' import type { Repo } from '../../../../shared/repo-types' +import type { Worktree } from '../../../../shared/worktree/types' import { resolveAutomationHostListRows } from './automation-host-list-rows' import { buildAutomationListSearchRows, @@ -24,6 +25,7 @@ import { matchAutomationListSearchRowKeys } from './automation-list-search-rows' import { REPO_ID } from './automations-page-fixtures' +import { automationRepoForRow, automationWorktreeForRow } from './automation-list-row-identity' const repoMap = new Map([ [REPO_ID, { id: REPO_ID, displayName: 'orca', path: '/src/orca' } as Repo] @@ -136,4 +138,43 @@ describe('automation list row identity across hosts', () => { expect(namesMatching('web-01')).toEqual(['Nightly web-01']) expect(namesMatching('this computer')).toEqual(['Nightly desktop']) }) + + it('resolves colliding projects and worktrees inside each row authority', () => { + const rows = collidedListRows().rows + const repos = [ + { ...repoMap.get(REPO_ID)!, displayName: 'Desktop repo', executionHostId: 'local' as const }, + { + ...repoMap.get(REPO_ID)!, + displayName: 'Runtime repo', + executionHostId: 'runtime:gpu' as const + } + ] + const worktrees = [ + { id: 'ws-1', repoId: REPO_ID, hostId: 'local', displayName: 'Desktop workspace' }, + { + id: 'ws-1', + repoId: REPO_ID, + hostId: 'runtime:gpu', + displayName: 'Runtime workspace' + } + ] as Worktree[] + const fallbackRepos = new Map([[REPO_ID, repos[1]]]) + const fallbackWorktrees = new Map([['ws-1', worktrees[1]]]) + + expect(rows.map((row) => automationRepoForRow(row, repos, fallbackRepos)?.displayName)).toEqual( + ['Desktop repo', 'Runtime repo'] + ) + expect( + rows.map((row) => { + const repo = automationRepoForRow(row, repos, fallbackRepos) + return automationWorktreeForRow( + row, + { [REPO_ID]: worktrees }, + repo, + fallbackWorktrees, + 'ws-1' + )?.displayName + }) + ).toEqual(['Desktop workspace', 'Runtime workspace']) + }) }) diff --git a/src/renderer/src/components/automations/automation-list-row-identity.ts b/src/renderer/src/components/automations/automation-list-row-identity.ts index d646699920b..577f323319f 100644 --- a/src/renderer/src/components/automations/automation-list-row-identity.ts +++ b/src/renderer/src/components/automations/automation-list-row-identity.ts @@ -16,11 +16,21 @@ import type { Automation } from '../../../../shared/automations-types' import type { AutomationUsageSummary } from '../../../../shared/automation-usage-summary' +import { getAutomationRunRepoId } from '../../../../shared/automation-run-identity' +import type { StableAutomationCatalogRef } from '../../../../shared/automation-owner-ref' +import { + getRepoExecutionHostId, + getWorktreeExecutionHostId +} from '../../../../shared/execution-host' +import type { Repo } from '../../../../shared/repo-types' +import type { Worktree } from '../../../../shared/worktree/types' export type AutomationListRow = { /** Authority-qualified and incarnation-free; see the module note. */ key: string automation: Automation + /** Storage authority captured with the host query; absent only for the bootstrap list. */ + catalogRef?: StableAutomationCatalogRef | null /** The host this row actually came from; empty for unscoped legacy rows. */ hostLabel: string /** What the owning authority reported; null means unknown, never zero. */ @@ -54,7 +64,48 @@ export function unscopedAutomationListRows( return automations.map((automation) => ({ key: automationListRowKey(null, automation.id), automation, + catalogRef: null, hostLabel: '', usageSummary: null })) } + +export function automationRepoForRow( + row: AutomationListRow, + repos: readonly Repo[], + fallback: ReadonlyMap +): Repo | undefined { + const repoId = getAutomationRunRepoId(row.automation) + const authority = row.catalogRef?.authority + if (!authority) { + return fallback.get(repoId) + } + return repos.find((repo) => { + const host = getRepoExecutionHostId(repo) + return ( + repo.id === repoId && + (authority.kind === 'runtime' + ? host === `runtime:${encodeURIComponent(authority.environmentId)}` + : !host.startsWith('runtime:')) + ) + }) +} + +export function automationWorktreeForRow( + row: AutomationListRow, + worktreesByRepo: Readonly>, + repo: Repo | undefined, + fallback: ReadonlyMap, + workspaceId: string | null | undefined = row.automation.workspaceId +): Worktree | undefined { + if (!workspaceId || !row.catalogRef || !repo) { + return workspaceId ? fallback.get(workspaceId) : undefined + } + const hostId = getRepoExecutionHostId(repo) + return Object.values(worktreesByRepo) + .flat() + .find( + (worktree) => + worktree.id === workspaceId && getWorktreeExecutionHostId(worktree, repo) === hostId + ) +} diff --git a/src/renderer/src/components/automations/automation-list-view.test.ts b/src/renderer/src/components/automations/automation-list-view.test.ts index ef46bc6e428..528630f12d4 100644 --- a/src/renderer/src/components/automations/automation-list-view.test.ts +++ b/src/renderer/src/components/automations/automation-list-view.test.ts @@ -100,7 +100,15 @@ function makeExternalEntry( runs: [], ...overrides } - return { key: `${manager.id}:${job.id}`, manager, job } + return { + key: `${manager.id}:${job.id}`, + scope: { + owner: { authority: { kind: 'desktop' }, selector: { kind: 'self' } }, + provider: 'hermes' + }, + manager, + job + } } describe('automation-list-view', () => { diff --git a/src/renderer/src/components/automations/use-automation-create-destination.ts b/src/renderer/src/components/automations/use-automation-create-destination.ts index 615c05ffc43..35521981294 100644 --- a/src/renderer/src/components/automations/use-automation-create-destination.ts +++ b/src/renderer/src/components/automations/use-automation-create-destination.ts @@ -10,7 +10,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { translate } from '@/i18n/i18n' -import type { Repo } from '../../../../shared/types' +import type { Repo } from '../../../../shared/repo-types' import type { AutomationHostCatalog, AutomationHostCatalogEntry diff --git a/src/renderer/src/components/automations/use-automation-source-host-availability.ts b/src/renderer/src/components/automations/use-automation-source-host-availability.ts index 24f32ec55cc..890a8deb4aa 100644 --- a/src/renderer/src/components/automations/use-automation-source-host-availability.ts +++ b/src/renderer/src/components/automations/use-automation-source-host-availability.ts @@ -16,6 +16,7 @@ import { type RepoBackedAutomationSourceContext } from './automation-source-context' import type { AutomationListRow } from './automation-list-row-identity' +import { createAutomationHostRequestPool } from './automation-host-scheduler-queue' /** * Availability of each automation's *source* host — the host its task provider @@ -43,6 +44,7 @@ export function useAutomationSourceHostAvailability( ) const mountedRef = useRef(true) const requestedHostIdsRef = useRef>(new Set()) + const [requestPool] = useState(createAutomationHostRequestPool) const [statusByHostId, setStatusByHostId] = useState< ReadonlyMap >(() => new Map()) @@ -73,8 +75,9 @@ export function useAutomationSourceHostAvailability( useEffect( () => () => { mountedRef.current = false + requestPool.cancelQueued() }, - [] + [requestPool] ) useEffect(() => { if (!preflightStatusCurrent || !preflightStatusChecked) { @@ -108,16 +111,23 @@ export function useAutomationSourceHostAvailability( if (parsed?.kind !== 'runtime') { continue } - void callRuntimeRpc( - { kind: 'environment', environmentId: parsed.environmentId }, - 'preflight.check', - undefined, - { timeoutMs: 15_000 } - ) - .then((status) => record(hostId, status)) - .catch(() => record(hostId, null)) + void requestPool.submit({ + run: async () => { + try { + const status = await callRuntimeRpc( + { kind: 'environment', environmentId: parsed.environmentId }, + 'preflight.check', + undefined, + { timeoutMs: 15_000 } + ) + record(hostId, status) + } catch { + record(hostId, null) + } + } + }) } - }, [runtimeSourceHostIds]) + }, [requestPool, runtimeSourceHostIds]) return useMemo(() => { const availabilityByRowKey = new Map() diff --git a/src/shared/automation-list-scope.ts b/src/shared/automation-list-scope.ts index 6942f2beab6..3d7ad7a85f4 100644 --- a/src/shared/automation-list-scope.ts +++ b/src/shared/automation-list-scope.ts @@ -72,6 +72,8 @@ export type AutomationWorkspaceHost = | { kind: 'ambiguous' } export type AutomationProjectionContext = { + /** The process that owns this store; runtime scheduling markers are local only on a runtime. */ + storageAuthority?: 'desktop' | 'runtime' /** Current registration generation of a saved SSH target; undefined once it is gone. */ sshTargetGeneration: (targetId: string) => number | undefined /** `undefined` = the repo is gone; `null` = a local repo; a string = its SSH connection. */ @@ -138,17 +140,17 @@ function workspacePinSelector( } /** - * Storage authority is never inferred here — `schedulerOwner` and - * `runContext.hostId` only mark a record unrunnable, and a bare `local` is not - * evidence of Self without a resolvable local project behind it. + * Runtime scheduling markers are owned only in runtime storage. A bare `local` + * still needs a resolvable local project before it is evidence of Self. */ export function projectAutomationSelector( automation: Automation, context: AutomationProjectionContext ): AutomationListItemSelector { if ( - automation.schedulerOwner === 'remote_host_service' || - parseExecutionHostId(automation.runContext?.hostId)?.kind === 'runtime' + context.storageAuthority !== 'runtime' && + (automation.schedulerOwner === 'remote_host_service' || + parseExecutionHostId(automation.runContext?.hostId)?.kind === 'runtime') ) { return orphan(AUTOMATION_ORPHAN_ISSUES.scheduledElsewhere) } @@ -191,7 +193,7 @@ const CAPTURED_HOST_ISSUES: Record = { } function isCapturedHostIssue(issue: string): issue is AutomationCapturedHostIssue { - return Object.prototype.hasOwnProperty.call(CAPTURED_HOST_ISSUES, issue) + return Object.hasOwn(CAPTURED_HOST_ISSUES, issue) } /** diff --git a/src/shared/automation-owner-precondition.test.ts b/src/shared/automation-owner-precondition.test.ts index 3ac6364810c..389fdd3d524 100644 --- a/src/shared/automation-owner-precondition.test.ts +++ b/src/shared/automation-owner-precondition.test.ts @@ -104,11 +104,10 @@ describe('assertAutomationOwnerFence', () => { ) }) - it('requires a precondition before mutating a record pinned to a generation', () => { - expectConflict( - () => assertAutomationOwnerFence({ automation: sshRecord, operation: 'mutate', context }), - AUTOMATION_OWNER_CONFLICT_CODES.fencingRequired - ) + it('keeps legacy mutations compatible when no precondition is present', () => { + expect(() => + assertAutomationOwnerFence({ automation: sshRecord, operation: 'mutate', context }) + ).not.toThrow() }) it('still lets an old client read that record', () => { @@ -125,25 +124,16 @@ describe('assertAutomationOwnerFence', () => { // The stored field is absent on a workspace-pinned record while the projection carries a // generation, so keying the demand off storage let precisely this population write unfenced. - it('requires a precondition on a record the projection presents as SSH-owned', () => { - expectConflict( - () => + it('keeps legacy workspace-pinned mutations and execution compatible', () => { + for (const operation of ['mutate', 'execute'] as const) { + expect(() => assertAutomationOwnerFence({ automation: pinnedLocalRecord, - operation: 'mutate', + operation, context: pinnedContext - }), - AUTOMATION_OWNER_CONFLICT_CODES.fencingRequired - ) - expectConflict( - () => - assertAutomationOwnerFence({ - automation: pinnedLocalRecord, - operation: 'execute', - context: pinnedContext - }), - AUTOMATION_OWNER_CONFLICT_CODES.fencingRequired - ) + }) + ).not.toThrow() + } }) it('still lets an old client read a workspace-pinned record', () => { diff --git a/src/shared/automation-owner-precondition.ts b/src/shared/automation-owner-precondition.ts index 4d284b98fb5..e34c1376f42 100644 --- a/src/shared/automation-owner-precondition.ts +++ b/src/shared/automation-owner-precondition.ts @@ -2,13 +2,8 @@ * The owner precondition an authority enforces before it reads, mutates, or * executes one stored automation. * - * Optional on the wire is not unenforced: a record the projection owns by an SSH - * registration refuses a mutation that names no expected owner, so a client that - * skips the field cannot make unfenced writes permanent server behavior. The - * precondition stays genuinely optional only where nothing can be fenced — Self - * records and orphans. Legacy SSH rows are not a carve-out: the load-time owner - * migration stamps every one whose target still resolves, so a survivor without - * a capture projects as an orphan rather than as an unfenced SSH owner. + * Optional on the wire preserves old-client/new-server compatibility. Current + * clients send the capture and get strict enforcement; legacy clients omit it. */ import type { Automation } from './automations-types' @@ -79,12 +74,6 @@ export function assertAutomationOwnerFence(input: AutomationOwnerFenceInput): vo conflict('targetRemoved') } if (!expected) { - // The projected selector, not the stored field, says what is fenceable: a workspace-pinned - // record is SSH-owned with the capture on its pin, so keying off storage let exactly that - // population write unfenced. An SSH projection always carries a generation. - if (stored.kind === 'ssh' && input.operation !== 'read') { - conflict('fencingRequired') - } return } if (expected.kind !== stored.kind) {