fix(engine): Report the recorded finish time on execution end events (#39963)

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
mfsiega 2026-09-30 10:02:13 +00:00 committed by GitHub
parent 3c1aaae176
commit dcda65f7d9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 47 additions and 32 deletions

View File

@ -205,10 +205,10 @@ describe('workflow_execution table (integration)', () => {
const finished = await new TypeOrmExecutionStore(repo).finishExecution(created.id, 'failed');
expect(finished).toBe(true);
const row = await repo.findOneOrFail({ where: { id: created.id } });
expect(row.status).toBe('failed');
expect(row.finishedAt).toBeInstanceOf(Date);
expect(finished).toEqual({ finishedAt: row.finishedAt });
});
it.each<ExecutionStatus>(['completed', 'queued'])(
@ -233,7 +233,7 @@ describe('workflow_execution table (integration)', () => {
const finished = await new TypeOrmExecutionStore(repo).finishExecution(created.id, 'failed');
expect(finished).toBe(false);
expect(finished).toBeNull();
const row = await repo.findOneOrFail({ where: { id: created.id } });
expect(row.status).toBe(status);
expect(row.finishedAt).toEqual(finishedAt);
@ -248,10 +248,11 @@ describe('workflow_execution table (integration)', () => {
const cancelled = await new TypeOrmExecutionStore(repo).cancelExecution(id);
expect(cancelled).toBe(true);
const row = await repo.findOneOrFail({ where: { id } });
expect(row.status).toBe('cancelled');
expect(row.finishedAt).toBeInstanceOf(Date);
// the caller reports the time the row records, not a second reading of the clock
expect(cancelled).toEqual({ finishedAt: row.finishedAt });
},
);
@ -264,7 +265,7 @@ describe('workflow_execution table (integration)', () => {
const cancelled = await new TypeOrmExecutionStore(repo).cancelExecution(id);
expect(cancelled).toBe(false);
expect(cancelled).toBeNull();
const row = await repo.findOneOrFail({ where: { id } });
expect(row.status).toBe(status);
expect(row.finishedAt).toEqual(finishedAt);

View File

@ -49,21 +49,26 @@ export class TypeOrmExecutionStore implements ExecutionStore {
return result.affected === 1;
}
async finishExecution(id: string, status: 'completed' | 'failed'): Promise<boolean> {
async finishExecution(
id: string,
status: 'completed' | 'failed',
): Promise<{ finishedAt: Date } | null> {
// A waiting execution can end too: a failure elsewhere cancels its waits.
const finishedAt = new Date();
const result = await this.repo.update(
{ id, status: In([...LIVE_EXECUTION_STATUSES]) },
{ status, finishedAt: new Date() },
{ status, finishedAt },
);
return result.affected === 1;
return result.affected === 1 ? { finishedAt } : null;
}
async cancelExecution(id: string): Promise<boolean> {
async cancelExecution(id: string): Promise<{ finishedAt: Date } | null> {
const finishedAt = new Date();
const result = await this.repo.update(
{ id, status: In(['queued', ...LIVE_EXECUTION_STATUSES] satisfies ExecutionStatus[]) },
{ status: 'cancelled', finishedAt: new Date() },
{ status: 'cancelled', finishedAt },
);
return result.affected === 1;
return result.affected === 1 ? { finishedAt } : null;
}
async refreshLiveStatus(id: string): Promise<void> {

View File

@ -28,7 +28,9 @@ function makeExecutionStore(overrides: Partial<ExecutionStore> = {}): ExecutionS
loadExecution: vi.fn().mockResolvedValue({ ...execution, status: 'cancelled' }),
transitionStatus: vi.fn(),
finishExecution: vi.fn(),
cancelExecution: vi.fn().mockResolvedValue(true),
cancelExecution: vi
.fn()
.mockResolvedValue({ finishedAt: new Date('2026-09-30T08:00:00.000Z') }),
refreshLiveStatus: vi.fn(),
...overrides,
};
@ -60,7 +62,7 @@ describe('CancelExecutionService', () => {
type: 'execution:cancelled',
executionId: 'exec-1',
workflowId: 'wf-1',
at: expect.any(String),
at: '2026-09-30T08:00:00.000Z',
});
});
@ -68,7 +70,7 @@ describe('CancelExecutionService', () => {
'reports the status of an execution that already ended %s and announces nothing',
async (status) => {
const executionStore = makeExecutionStore({
cancelExecution: vi.fn().mockResolvedValue(false),
cancelExecution: vi.fn().mockResolvedValue(null),
loadExecution: vi.fn().mockResolvedValue({ ...execution, status }),
});
const { service, stepStore, publisher } = makeService(executionStore);
@ -83,7 +85,7 @@ describe('CancelExecutionService', () => {
it('throws for an unknown execution', async () => {
const executionStore = makeExecutionStore({
cancelExecution: vi.fn().mockResolvedValue(false),
cancelExecution: vi.fn().mockResolvedValue(null),
loadExecution: vi.fn().mockRejectedValue(new ExecutionNotFoundError('exec-404')),
});
const { service, stepStore } = makeService(executionStore);

View File

@ -27,8 +27,8 @@ function makeExecutionStore(overrides: Partial<ExecutionStore> = {}): ExecutionS
createExecution: vi.fn(),
loadExecution: vi.fn(),
transitionStatus: vi.fn().mockResolvedValue(true),
finishExecution: vi.fn().mockResolvedValue(true),
cancelExecution: vi.fn().mockResolvedValue(true),
finishExecution: vi.fn().mockResolvedValue(null),
cancelExecution: vi.fn().mockResolvedValue(null),
refreshLiveStatus: vi.fn(),
...overrides,
};

View File

@ -28,8 +28,8 @@ function makeStore(overrides: Partial<ExecutionStore> = {}): ExecutionStore {
createExecution: vi.fn(),
loadExecution: vi.fn(),
transitionStatus: vi.fn().mockResolvedValue(true),
finishExecution: vi.fn().mockResolvedValue(true),
cancelExecution: vi.fn().mockResolvedValue(true),
finishExecution: vi.fn().mockResolvedValue(null),
cancelExecution: vi.fn().mockResolvedValue(null),
refreshLiveStatus: vi.fn(),
...overrides,
};

View File

@ -88,8 +88,8 @@ function makeExecutionStore(overrides: Partial<ExecutionRecord> = {}): Execution
createExecution: vi.fn(),
loadExecution: vi.fn().mockResolvedValue(execution),
transitionStatus: vi.fn().mockResolvedValue(true),
finishExecution: vi.fn().mockResolvedValue(true),
cancelExecution: vi.fn().mockResolvedValue(true),
finishExecution: vi.fn().mockResolvedValue(null),
cancelExecution: vi.fn().mockResolvedValue(null),
refreshLiveStatus: vi.fn(),
};
}

View File

@ -65,8 +65,10 @@ function makeExecutionStore(
createExecution: vi.fn(),
loadExecution: vi.fn().mockResolvedValue(execution),
transitionStatus: vi.fn().mockResolvedValue(true),
finishExecution: vi.fn().mockResolvedValue(true),
cancelExecution: vi.fn().mockResolvedValue(true),
finishExecution: vi
.fn()
.mockResolvedValue({ finishedAt: new Date('2026-09-30T08:00:00.000Z') }),
cancelExecution: vi.fn().mockResolvedValue(null),
refreshLiveStatus: vi.fn(),
...storeOverrides,
};
@ -456,7 +458,7 @@ describe('StepSettledHandler', () => {
const stepStore = makeStepStore({ status: 'failed' });
const executionStore = makeExecutionStore(
{},
{ finishExecution: vi.fn().mockResolvedValue(false) },
{ finishExecution: vi.fn().mockResolvedValue(null) },
);
const { handler } = makeHandler(stepStore, { executionStore });
@ -514,7 +516,7 @@ describe('StepSettledHandler', () => {
});
describe('StepSettledHandler lifecycle events', () => {
const finished = { executionId: 'exec-1', workflowId: 'wf-1', at: expect.any(String) as string };
const finished = { executionId: 'exec-1', workflowId: 'wf-1', at: '2026-09-30T08:00:00.000Z' };
it('announces execution:completed when it records the completion', async () => {
const stepStore = makeStepStore(
@ -639,7 +641,7 @@ describe('StepSettledHandler lifecycle events', () => {
// Another worker already wrote the outcome, so it announces it.
const executionStore = makeExecutionStore(
{},
{ finishExecution: vi.fn().mockResolvedValue(false) },
{ finishExecution: vi.fn().mockResolvedValue(null) },
);
const { handler, lifecycleEventPublisher, responseSender } = makeHandler(
makeStepStore({ status: 'failed' }),

View File

@ -32,7 +32,7 @@ export class CancelExecutionService {
type: 'execution:cancelled',
executionId,
workflowId: execution.workflowId,
at: new Date().toISOString(),
at: cancelled.finishedAt.toISOString(),
});
}

View File

@ -59,12 +59,17 @@ export interface ExecutionStore {
/**
* Record an execution's outcome: writes the final status and the finish time
* together, as a compare-and-set on the live statuses, so they can't be
* observed apart.
* observed apart. Returns the finish time it wrote, so the caller reports the
* time the row records, or `null` when the compare-and-set lost.
*/
finishExecution(id: string, status: 'completed' | 'failed'): Promise<boolean>;
finishExecution(id: string, status: 'completed' | 'failed'): Promise<{ finishedAt: Date } | null>;
/** End an execution on request, from any status that has not ended. */
cancelExecution(id: string): Promise<boolean>;
/**
* End an execution on request, from any status that has not ended. Returns
* the finish time it wrote, so the caller reports the time the row records,
* or `null` when the execution had already ended.
*/
cancelExecution(id: string): Promise<{ finishedAt: Date } | null>;
/**
* Sets a live execution's status from the state of its steps: `waiting` when

View File

@ -96,7 +96,7 @@ export class StepSettledHandler {
type: 'execution:failed',
executionId: execution.id,
workflowId: execution.workflowId,
at: new Date().toISOString(),
at: finished.finishedAt.toISOString(),
});
this.announceEnd(execution, step, node, 'failed');
}
@ -206,7 +206,7 @@ export class StepSettledHandler {
type: failed ? 'execution:failed' : 'execution:completed',
executionId: execution.id,
workflowId: execution.workflowId,
at: new Date().toISOString(),
at: finished.finishedAt.toISOString(),
});
this.announceEnd(execution, step, node, failed ? 'failed' : 'completed');
}