diff --git a/packages/openworkflow/core/backend.ts b/packages/openworkflow/core/backend.ts index 926eac0b..926901b1 100644 --- a/packages/openworkflow/core/backend.ts +++ b/packages/openworkflow/core/backend.ts @@ -147,6 +147,7 @@ export interface CreateStepAttemptParams { workflowRunId: string; workerId: string; stepName: string; + stepIndex?: number | null; kind: StepKind; config: JsonValue; context: StepAttemptContext | null; diff --git a/packages/openworkflow/core/step-attempt.test.ts b/packages/openworkflow/core/step-attempt.test.ts index 33325dfb..722ad03c 100644 --- a/packages/openworkflow/core/step-attempt.test.ts +++ b/packages/openworkflow/core/step-attempt.test.ts @@ -368,6 +368,7 @@ function createMockStepAttempt( id: "step-1", workflowRunId: "workflow-1", stepName: "test-step", + stepIndex: null, kind: "function", status: "completed", config: {}, diff --git a/packages/openworkflow/core/step-attempt.ts b/packages/openworkflow/core/step-attempt.ts index 96f51065..08e356d1 100644 --- a/packages/openworkflow/core/step-attempt.ts +++ b/packages/openworkflow/core/step-attempt.ts @@ -60,6 +60,8 @@ export interface StepAttempt { id: string; workflowRunId: string; stepName: string; + /** Zero-based order within the run; null for older attempts. */ + stepIndex: number | null; kind: StepKind; status: StepAttemptStatus; config: JsonValue; // user-defined config diff --git a/packages/openworkflow/postgres/backend.ts b/packages/openworkflow/postgres/backend.ts index 3f84f3bf..f9d0b054 100644 --- a/packages/openworkflow/postgres/backend.ts +++ b/packages/openworkflow/postgres/backend.ts @@ -819,6 +819,7 @@ export class BackendPostgres implements Backend { "id", "workflow_run_id", "step_name", + "step_index", "kind", "status", "config", @@ -832,6 +833,7 @@ export class BackendPostgres implements Backend { gen_random_uuid(), ${params.workflowRunId}, ${params.stepName}, + ${params.stepIndex ?? null}, ${params.kind}, 'running', ${this.pg.json(params.config)}, diff --git a/packages/openworkflow/postgres/postgres.ts b/packages/openworkflow/postgres/postgres.ts index f52f6523..94dcecaa 100644 --- a/packages/openworkflow/postgres/postgres.ts +++ b/packages/openworkflow/postgres/postgres.ts @@ -240,6 +240,18 @@ export function migrations(schema: string): string[] { ON CONFLICT DO NOTHING; COMMIT;`, + + // 6 - add step index to steps + `BEGIN; + + ALTER TABLE ${quotedSchema}."step_attempts" + ADD COLUMN "step_index" INTEGER; + + INSERT INTO ${quotedSchema}."openworkflow_migrations" ("version") + VALUES (6) + ON CONFLICT DO NOTHING; + + COMMIT;`, ]; } diff --git a/packages/openworkflow/sqlite/backend.ts b/packages/openworkflow/sqlite/backend.ts index fc5d0022..a9bc6cda 100644 --- a/packages/openworkflow/sqlite/backend.ts +++ b/packages/openworkflow/sqlite/backend.ts @@ -964,6 +964,7 @@ export class BackendSqlite implements Backend { "id", "workflow_run_id", "step_name", + "step_index", "kind", "status", "config", @@ -972,7 +973,7 @@ export class BackendSqlite implements Backend { "created_at", "updated_at" ) - SELECT ?, ?, ?, ?, ?, 'running', ?, ?, ?, ?, ? + SELECT ?, ?, ?, ?, ?, ?, 'running', ?, ?, ?, ?, ? FROM "workflow_runs" WHERE ${RUNNING_WORKFLOW_RUN_OWNED_WHERE} RETURNING * @@ -984,6 +985,7 @@ export class BackendSqlite implements Backend { id, params.workflowRunId, params.stepName, + params.stepIndex ?? null, params.kind, toJSON(params.config), toJSON(params.context), @@ -1141,6 +1143,7 @@ interface StepAttemptRow extends Record { id: string; workflow_run_id: string; step_name: string; + step_index: number | null; kind: string; status: string; config: string; @@ -1233,6 +1236,7 @@ function rowToStepAttempt(row: StepAttemptRow): StepAttempt { id: row.id, workflowRunId: row.workflow_run_id, stepName: row.step_name, + stepIndex: row.step_index, // safety: the kind column is written from CreateStepAttemptParams.kind. kind: row.kind as StepAttempt["kind"], // safety: the status column is written by backend transitions using the domain status values. diff --git a/packages/openworkflow/sqlite/sqlite.ts b/packages/openworkflow/sqlite/sqlite.ts index daccc91a..ff9fa665 100644 --- a/packages/openworkflow/sqlite/sqlite.ts +++ b/packages/openworkflow/sqlite/sqlite.ts @@ -230,6 +230,16 @@ export function migrations(): string[] { VALUES (5); COMMIT;`, + + // 6 - add step index to steps + `BEGIN; + + ALTER TABLE "step_attempts" ADD COLUMN "step_index" INTEGER; + + INSERT OR IGNORE INTO "openworkflow_migrations" ("version") + VALUES (6); + + COMMIT;`, ]; } diff --git a/packages/openworkflow/testing/backend.testsuite.ts b/packages/openworkflow/testing/backend.testsuite.ts index 6cdf20e8..0e62acc9 100644 --- a/packages/openworkflow/testing/backend.testsuite.ts +++ b/packages/openworkflow/testing/backend.testsuite.ts @@ -1663,6 +1663,7 @@ export function testBackend(options: TestBackendOptions): void { id: "", // - workflowRunId: workflowRun.id, stepName: randomUUID(), + stepIndex: 0, kind: "function", status: "running", config: { key: "val" }, @@ -1681,6 +1682,7 @@ export function testBackend(options: TestBackendOptions): void { workflowRunId: expected.workflowRunId, workerId: workflowRun.workerId!, // oxlint-disable-line typescript/no-non-null-assertion stepName: expected.stepName, + stepIndex: expected.stepIndex, kind: expected.kind, config: expected.config, context: expected.context, @@ -1762,6 +1764,7 @@ export function testBackend(options: TestBackendOptions): void { const got = await backend.getStepAttempt({ stepAttemptId: created.id, }); + expect(got?.stepIndex).toBeNull(); expect(got).toEqual(created); }); }); diff --git a/packages/openworkflow/worker/execution.test.ts b/packages/openworkflow/worker/execution.test.ts index 1e79c92e..b1f0c8ff 100644 --- a/packages/openworkflow/worker/execution.test.ts +++ b/packages/openworkflow/worker/execution.test.ts @@ -4267,6 +4267,7 @@ function createMockStepAttempt( id: "step-attempt-id", workflowRunId: "workflow-run-id", stepName: "step", + stepIndex: null, kind: "function", status, config: {}, diff --git a/packages/openworkflow/worker/execution.ts b/packages/openworkflow/worker/execution.ts index c4b8bd9e..a290f2c3 100644 --- a/packages/openworkflow/worker/execution.ts +++ b/packages/openworkflow/worker/execution.ts @@ -428,6 +428,7 @@ class StepExecutor implements StepApi { workflowRunId: this.workflowRunId, workerId: this.workerId, stepName, + stepIndex: this.history.stepIndex(stepName), kind, config: {}, context, diff --git a/packages/openworkflow/worker/step-history.test.ts b/packages/openworkflow/worker/step-history.test.ts index 374a6bb3..44b9e8a9 100644 --- a/packages/openworkflow/worker/step-history.test.ts +++ b/packages/openworkflow/worker/step-history.test.ts @@ -7,6 +7,10 @@ describe("StepHistory", () => { test("returns the base name on first use", () => { const history = new StepHistory({ attempts: [] }); expect(history.resolveStepName("step")).toBe("step"); + expect(history.stepIndex("step")).toBe(0); + expect(() => history.stepIndex("missing")).toThrow( + 'Unresolved step "missing"', + ); }); test("appends incrementing suffixes for collisions", () => { @@ -14,6 +18,8 @@ describe("StepHistory", () => { expect(history.resolveStepName("step")).toBe("step"); expect(history.resolveStepName("step")).toBe("step:1"); expect(history.resolveStepName("step")).toBe("step:2"); + expect(history.stepIndex("step:1")).toBe(1); + expect(history.stepIndex("step:2")).toBe(2); }); test("skips suffixes that were user-supplied as base names", () => { @@ -22,6 +28,40 @@ describe("StepHistory", () => { history.resolveStepName("step:1"); // user-supplied collision expect(history.resolveStepName("step")).toBe("step:2"); }); + + test("preserves indices when parallel branches replay in a different order", () => { + const history = new StepHistory({ + attempts: [ + createMockStepAttempt({ stepName: "a", stepIndex: 0 }), + createMockStepAttempt({ stepName: "b", stepIndex: 1 }), + createMockStepAttempt({ stepName: "b:1", stepIndex: 2 }), + createMockStepAttempt({ + stepName: "a:1", + stepIndex: 3, + status: "failed", + }), + ], + }); + + // Cached results let the a branch reach its second step before b. + const indices = ["a", "b", "a", "b", "new"].map((name) => + history.stepIndex(history.resolveStepName(name)), + ); + expect(indices).toEqual([0, 1, 3, 2, 4]); + }); + + test("reserves persisted indices before resolving new and legacy steps", () => { + const history = new StepHistory({ + attempts: [ + createMockStepAttempt({ stepName: "legacy", stepIndex: null }), + createMockStepAttempt({ stepName: "saved", stepIndex: 5 }), + ], + }); + + expect(history.stepIndex(history.resolveStepName("new"))).toBe(6); + expect(history.stepIndex(history.resolveStepName("legacy"))).toBe(7); + expect(history.stepIndex(history.resolveStepName("saved"))).toBe(5); + }); }); describe("find*", () => { @@ -307,6 +347,7 @@ function createMockStepAttempt( id: "step-attempt-id", workflowRunId: "workflow-run-id", stepName: "step", + stepIndex: null, kind: "function", status, config: {}, diff --git a/packages/openworkflow/worker/step-history.ts b/packages/openworkflow/worker/step-history.ts index 9e28c5ad..19304435 100644 --- a/packages/openworkflow/worker/step-history.ts +++ b/packages/openworkflow/worker/step-history.ts @@ -184,8 +184,10 @@ export class StepHistory { private readonly failedCountsByStepName: Map; private readonly failedByStepName: Map; private readonly runningByStepName: Map; - private readonly resolvedStepNames = new Set(); + private readonly persistedStepIndices = new Map(); + private readonly resolvedStepNames = new Map(); private readonly expectedNextStepIndexByName = new Map(); + private nextStepIndex = 0; private readonly stepLimit: number; private stepCount: number; @@ -193,6 +195,12 @@ export class StepHistory { this.stepLimit = Math.max(1, options.stepLimit ?? WORKFLOW_STEP_LIMIT); this.stepCount = options.attempts.length; + for (const attempt of options.attempts) { + if (attempt.stepIndex === null) continue; + this.persistedStepIndices.set(attempt.stepName, attempt.stepIndex); + this.nextStepIndex = Math.max(this.nextStepIndex, attempt.stepIndex + 1); + } + const state = createStepExecutionStateFromAttempts(options.attempts); this.cache = new Map(state.cache); this.failedCountsByStepName = new Map(state.failedCountsByStepName); @@ -210,8 +218,7 @@ export class StepHistory { */ resolveStepName(baseStepName: string): string { if (!this.resolvedStepNames.has(baseStepName)) { - this.resolvedStepNames.add(baseStepName); - return baseStepName; + return this.recordResolvedStepName(baseStepName); } const expectedNextIndex = @@ -223,15 +230,32 @@ export class StepHistory { } this.expectedNextStepIndexByName.set(baseStepName, index + 1); - this.resolvedStepNames.add(resolvedName); - return resolvedName; + return this.recordResolvedStepName(resolvedName); } } + private recordResolvedStepName(stepName: string): string { + const index = + this.persistedStepIndices.get(stepName) ?? this.nextStepIndex++; + this.resolvedStepNames.set(stepName, index); + return stepName; + } + findCached(stepName: string): StepAttempt | undefined { return this.cache.get(stepName); } + /** + * Read the persisted index or the index assigned at invocation, before any awaits. + * @param stepName - Resolved step name + * @returns Zero-based invocation index + */ + stepIndex(stepName: string): number { + const index = this.resolvedStepNames.get(stepName); + if (index === undefined) throw new Error(`Unresolved step "${stepName}"`); + return index; + } + findRunning(stepName: string): StepAttempt | undefined { return this.runningByStepName.get(stepName); } diff --git a/packages/openworkflow/worker/worker.test.ts b/packages/openworkflow/worker/worker.test.ts index 72ad2347..f3a684c6 100644 --- a/packages/openworkflow/worker/worker.test.ts +++ b/packages/openworkflow/worker/worker.test.ts @@ -85,6 +85,11 @@ describe("Worker", () => { .map((stepAttempt) => stepAttempt.stepName) .toSorted((a, b) => a.localeCompare(b)); expect(stepNames).toEqual(["once", "once:1"]); + expect( + Object.fromEntries( + steps.data.map((attempt) => [attempt.stepName, attempt.stepIndex]), + ), + ).toEqual({ once: 0, "once:1": 1 }); }); test("reschedules workflow when definition is missing", async () => { @@ -266,6 +271,17 @@ describe("Worker", () => { const backend = await createTestBackend(); const client = new OpenWorkflow({ backend }); + const thirdRecorded = Promise.withResolvers(); + const createStepAttempt = backend.createStepAttempt.bind(backend); + vi.spyOn(backend, "createStepAttempt").mockImplementation( + async (params) => { + if (params.stepName === "step-a") await thirdRecorded.promise; + const attempt = await createStepAttempt(params); + if (params.stepName === "step-c") thirdRecorded.resolve(); + return attempt; + }, + ); + const startedSteps = new Set(); let resolveAllStepsStarted: (() => void) | null = null; const allStepsStarted = new Promise((resolve) => { @@ -309,6 +325,14 @@ describe("Worker", () => { const result = await handle.result(); expect(result).toEqual({ a: "a", b: "b", c: "c" }); expect(startedSteps).toEqual(new Set(["step-a", "step-b", "step-c"])); + const attempts = await backend.listStepAttempts({ + workflowRunId: handle.workflowRun.id, + }); + expect( + Object.fromEntries( + attempts.data.map((attempt) => [attempt.stepName, attempt.stepIndex]), + ), + ).toEqual({ "step-a": 0, "step-b": 1, "step-c": 2 }); }); test("respects worker concurrency limit", { timeout: 15_000 }, async () => { @@ -665,6 +689,18 @@ describe("Worker", () => { b: "b-result", c: "c-result", }); + const attempts = await backend.listStepAttempts({ + workflowRunId: handle.workflowRun.id, + }); + expect( + attempts.data + .filter((attempt) => attempt.stepName === "step-b") + .map((attempt) => attempt.stepIndex), + ).toEqual([1, 1]); + expect( + attempts.data.find((attempt) => attempt.stepName === "step-c") + ?.stepIndex, + ).toBe(2); }, );