From e2c2657cef6cb3c1e3784e1bc177034644e8b211 Mon Sep 17 00:00:00 2001 From: Ben Villalobos <4691428+benvillalobos@users.noreply.github.com> Date: Fri, 21 Aug 2026 03:59:52 -0700 Subject: [PATCH] automations/ahp: enforce single-run invariant in schedule claim loop Break out of the trigger loop once a schedule trigger has been claimed for an Automation, after advancing that trigger's cursor. Prevents two simultaneously-due schedule triggers on one Automation from both starting sessions and violating the one-non-terminal-run-per-Automation invariant. Deferred triggers keep their cursors untouched so their firings are re-evaluated on the next tick rather than dropped. --- .../node/agentHostAutomationService.ts | 8 + .../node/agentHostAutomationService.test.ts | 162 ++++++++++++++++++ 2 files changed, 170 insertions(+) diff --git a/src/vs/platform/agentHost/node/agentHostAutomationService.ts b/src/vs/platform/agentHost/node/agentHostAutomationService.ts index 3fbd1c21845..2bbc5104517 100644 --- a/src/vs/platform/agentHost/node/agentHostAutomationService.ts +++ b/src/vs/platform/agentHost/node/agentHostAutomationService.ts @@ -499,6 +499,7 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost } const cursors = { ...readScheduleCursors(current._meta) }; let automation = current; + let claimedForAutomation = false; for (const trigger of current.definition.triggers) { if (trigger.kind !== AutomationTriggerKind.Schedule) { continue; @@ -518,10 +519,17 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost nextRuns.set(run.resource, run); automation = withRunSummary(automation, nextRuns); claimed.push({ run, definition: automation.definition }); + claimedForAutomation = true; } scheduledFor = nextAutomationCronOccurrence(trigger.schedule.expression, trigger.schedule.timeZone, now); } cursors[trigger.id] = scheduledFor.toISOString(); + if (claimedForAutomation) { + // One-non-terminal-run-per-Automation invariant. Other due + // triggers keep their cursors untouched and are re-evaluated + // on the next tick, so a second past-due firing isn't lost. + break; + } } const nextAutomation: AutomationState = { ...automation, diff --git a/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts b/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts index ff4f6555fd1..8f95dacbab3 100644 --- a/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts @@ -529,6 +529,7 @@ suite('AgentHostAutomationService', () => { }); const session = URI.parse('mock:/hung-session'); const started = new DeferredPromise(); + const service = createService({ createSession: async () => { stateManager.createSession({ @@ -633,6 +634,7 @@ suite('AgentHostAutomationService', () => { test('failed linked-session cancellation leaves the run non-terminal', async () => { const session = URI.parse('mock:/uncancelled-session'); const started = new DeferredPromise(); + const service = createService({ createSession: async () => { stateManager.createSession({ @@ -699,6 +701,7 @@ suite('AgentHostAutomationService', () => { const session = URI.parse('mock:/scheduled-session'); const started = new DeferredPromise(); + const service = createService({ createSession: async () => { stateManager.createSession({ @@ -745,6 +748,165 @@ suite('AgentHostAutomationService', () => { }); }); + test('claims at most one schedule trigger per Automation per tick and defers the rest', async () => { + const now = new Date(); + const firstScheduledFor = new Date(now.getTime() - 3 * 60_000).toISOString(); + const secondScheduledFor = new Date(now.getTime() - 2 * 60_000).toISOString(); + const automationResource = 'ahp-automation:/multi-trigger'; + const multiTriggerDefinition: AutomationDefinition = { + ...definition(), + triggers: [ + { + id: 'first-trigger', + kind: AutomationTriggerKind.Schedule, + schedule: { expression: '* * * * *', timeZone: 'UTC' }, + misfirePolicy: AutomationMisfirePolicy.RunOnce, + }, + { + id: 'second-trigger', + kind: AutomationTriggerKind.Schedule, + schedule: { expression: '*/2 * * * *', timeZone: 'UTC' }, + misfirePolicy: AutomationMisfirePolicy.RunOnce, + }, + ], + }; + storageService.set('automations', { + catalog: { + automations: [{ + resource: automationResource, + definition: multiTriggerDefinition, + nextRunAt: firstScheduledFor, + runs: [], + operations: [AutomationOperation.Update, AutomationOperation.Remove, AutomationOperation.Run], + createdAt: now.toISOString(), + modifiedAt: now.toISOString(), + _meta: { + 'vscode.scheduleCursors': { + 'first-trigger': firstScheduledFor, + 'second-trigger': secondScheduledFor, + }, + }, + }], + }, + runs: [], + manualRunRequests: [], + migration: { status: 'complete', completedAt: now.toISOString() }, + }); + await storageService.whenIdle(); + + const session = URI.parse('mock:/multi-trigger-session'); + const started = new DeferredPromise(); + + createService({ + createSession: async () => { + stateManager.createSession({ + resource: session.toString(), + provider: 'mock', + title: '', + status: SessionStatus.Idle, + createdAt: new Date().toISOString(), + modifiedAt: new Date().toISOString(), + }); + return session; + }, + startSession: async () => { + await started.complete(); + }, + }); + await started.p; + + const automation = stateManager.getAutomationCatalogState()?.automations[0]; + const cursors = automation?._meta?.['vscode.scheduleCursors'] as Record | undefined; + assert.deepStrictEqual({ + runsClaimed: automation?.runs.length, + claimedTriggerId: automation?.runs[0]?.origin.kind === AutomationRunOriginKind.Trigger ? automation.runs[0].origin.triggerId : undefined, + firstCursorAdvanced: cursors ? Date.parse(cursors['first-trigger']) > now.getTime() : false, + secondCursorUnchanged: cursors?.['second-trigger'] === secondScheduledFor, + }, { + runsClaimed: 1, + claimedTriggerId: 'first-trigger', + firstCursorAdvanced: true, + secondCursorUnchanged: true, + }); + }); + + test('Skip-catch-up on the first trigger does not consume the per-tick claim slot', async () => { + const now = new Date(); + const stale = new Date(now.getTime() - 10 * 60_000).toISOString(); + const dueRecently = new Date(now.getTime() - 30_000).toISOString(); + const automationResource = 'ahp-automation:/skip-first'; + const multiTriggerDefinition: AutomationDefinition = { + ...definition(), + triggers: [ + { + id: 'stale-skip-trigger', + kind: AutomationTriggerKind.Schedule, + schedule: { expression: '* * * * *', timeZone: 'UTC' }, + misfirePolicy: AutomationMisfirePolicy.Skip, + }, + { + id: 'due-run-trigger', + kind: AutomationTriggerKind.Schedule, + schedule: { expression: '*/2 * * * *', timeZone: 'UTC' }, + misfirePolicy: AutomationMisfirePolicy.RunOnce, + }, + ], + }; + storageService.set('automations', { + catalog: { + automations: [{ + resource: automationResource, + definition: multiTriggerDefinition, + nextRunAt: stale, + runs: [], + operations: [AutomationOperation.Update, AutomationOperation.Remove, AutomationOperation.Run], + createdAt: now.toISOString(), + modifiedAt: now.toISOString(), + _meta: { + 'vscode.scheduleCursors': { + 'stale-skip-trigger': stale, + 'due-run-trigger': dueRecently, + }, + }, + }], + }, + runs: [], + manualRunRequests: [], + migration: { status: 'complete', completedAt: now.toISOString() }, + }); + await storageService.whenIdle(); + + const session = URI.parse('mock:/skip-first-session'); + const started = new DeferredPromise(); + + createService({ + createSession: async () => { + stateManager.createSession({ + resource: session.toString(), + provider: 'mock', + title: '', + status: SessionStatus.Idle, + createdAt: new Date().toISOString(), + modifiedAt: new Date().toISOString(), + }); + return session; + }, + startSession: async () => { + await started.complete(); + }, + }); + await started.p; + + const automation = stateManager.getAutomationCatalogState()?.automations[0]; + assert.deepStrictEqual({ + runsClaimed: automation?.runs.length, + claimedTriggerId: automation?.runs[0]?.origin.kind === AutomationRunOriginKind.Trigger ? automation.runs[0].origin.triggerId : undefined, + }, { + runsClaimed: 1, + claimedTriggerId: 'due-run-trigger', + }); + }); + test('bounds catalogue run history and loads older pages by cursor', async () => { const automationResource = 'ahp-automation:/history'; const runs: AutomationRunState[] = Array.from({ length: 51 }, (_, index) => {