From 86b87a81ccc231b7f3b802e47d8b49228049a29a Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 9 Jan 2026 18:59:45 +0000 Subject: [PATCH 1/3] Fix status precedence in Foreach Step restoration Prioritize derived status from child items over stored parent status in `WorkflowState.restore`. If all items in a foreach step are successful, the step is treated as SUCCESS even if the DB record says RUNNING or PENDING. This prevents unnecessary re-execution or stalls when resuming workflows that crashed after completing all iterations but before updating the parent step. Added a regression test `tests/runner/foreach_restore.test.ts`. --- src/runner/workflow-state.ts | 23 ++++--- tests/runner/foreach_restore.test.ts | 91 ++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 tests/runner/foreach_restore.test.ts diff --git a/src/runner/workflow-state.ts b/src/runner/workflow-state.ts index f8e500f..d42e3bf 100644 --- a/src/runner/workflow-state.ts +++ b/src/runner/workflow-state.ts @@ -295,18 +295,23 @@ export class WorkflowState { ); } const mappedOutputs = isLargeDataset ? {} : ForeachExecutor.aggregateOutputs(outputs); + + // If the DB says the parent is RUNNING/PENDING but we have all items successfully completed, + // trust the derived status to prevent re-execution. + let finalStatus = mainExec.status as StepStatusType; + if ( + allSuccess && + hasAllItems && + finalStatus !== StepStatus.SUCCESS && + finalStatus !== StepStatus.SKIPPED + ) { + finalStatus = StepStatus.SUCCESS; + } + this.stepContexts.set(stepId, { output: isLargeDataset ? [] : outputs, outputs: mappedOutputs, - status: mainExec.status as StepStatusType, // Trust the main status mostly? Or recompute? - // If main status says STARTED but we have all items success, maybe we should trust our recomputation? - // The original code sets status based on items. - // But if mainExec exists and has a status, that's authoritative for the "Parent". - // HOWEVER, if we are resuming, we might want to check if it matches reality. - // Let's stick to original logic: - // if (allSuccess && hasAllItems) status = SUCCESS... - // But wait, if main status is FAILED, using FAILED is correct. - // Let's mostly use the derived status for consistency in "incomplete" resumes. + status: finalStatus, items, foreachItems: persistedItems, } as ForeachStepContext); diff --git a/tests/runner/foreach_restore.test.ts b/tests/runner/foreach_restore.test.ts new file mode 100644 index 0000000..a370992 --- /dev/null +++ b/tests/runner/foreach_restore.test.ts @@ -0,0 +1,91 @@ + +import { afterAll, describe, expect, it } from 'bun:test'; +import { randomUUID } from 'node:crypto'; +import { existsSync, rmSync } from 'node:fs'; +import { WorkflowDb } from '../../src/db/workflow-db'; +import type { Workflow } from '../../src/parser/schema'; +import { WorkflowRunner } from '../../src/runner/workflow-runner'; +import { StepStatus } from '../../src/types/status'; +import { container } from '../../src/utils/container'; +import { ConsoleLogger } from '../../src/utils/logger'; + +describe('WorkflowState Foreach Restoration', () => { + const dbPath = `test-foreach-restore-${randomUUID()}.db`; + + container.register('logger', new ConsoleLogger()); + container.register('db', new WorkflowDb(dbPath)); + + afterAll(() => { + if (existsSync(dbPath)) { + rmSync(dbPath); + } + }); + + it('should treat a RUNNING foreach step as SUCCESS if all items are success', async () => { + // 1. Setup DB with a "RUNNING" foreach step but all items completed + const db = new WorkflowDb(dbPath); + const runId = randomUUID(); + const stepId = 'foreach_step'; + const parentStepExecId = randomUUID(); + + await db.createRun(runId, 'test-workflow', {}); + await db.updateRunStatus(runId, 'running'); + + // Create the parent step as RUNNING + await db.createStep(parentStepExecId, runId, stepId); + await db.startStep(parentStepExecId); + // Mark it as RUNNING, and store __foreachItems so expectedCount can be calculated + const items = [1, 2, 3]; + await db.completeStep(parentStepExecId, StepStatus.RUNNING as any, { __foreachItems: items }); + + // Create the item executions as SUCCESS + for (let i = 0; i < items.length; i++) { + const itemStepExecId = randomUUID(); + // Correctly pass iteration index to createStep + await db.createStep(itemStepExecId, runId, stepId, i); + // We must include output so hydration works and items array is populated + await db.completeStep(itemStepExecId, StepStatus.SUCCESS, { result: items[i] }); + } + + // 2. Define workflow + const workflow: Workflow = { + name: 'test-workflow', + steps: [ + { + id: stepId, + type: 'shell', + run: 'echo ${{ item }}', + foreach: '${{ [1, 2, 3] }}', + needs: [] + }, + { + id: 'next_step', + type: 'shell', + run: 'echo "done"', + needs: [stepId] + } + ], + outputs: { + final: '${{ steps.next_step.output.stdout.trim() }}' + } + } as unknown as Workflow; + + // 3. Restore via WorkflowRunner + const runner = new WorkflowRunner(workflow, { + dbPath, + resumeRunId: runId, + }); + + // 4. Run - it should skip the foreach step (because it detects it as SUCCESS) and run next-step + const outputs = await runner.run(); + expect(outputs.final).toBe('done'); + + // Verify DB state + const parentStep = await db.getMainStep(runId, stepId); + + // Since execution was skipped, the DB status should remain RUNNING (the fix is in-memory only) + expect(parentStep?.status).toBe(StepStatus.RUNNING); + + db.close(); + }); +}); From 64d00a4f83beb07f9b2e42d6c456613c1a5b948d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 9 Jan 2026 20:24:15 +0000 Subject: [PATCH 2/3] Fix status precedence in Foreach Step restoration Prioritize derived status from child items over stored parent status in `WorkflowState.restore`. If all items in a foreach step are successful, the step is treated as SUCCESS even if the DB record says RUNNING or PENDING. This prevents unnecessary re-execution or stalls when resuming workflows that crashed after completing all iterations but before updating the parent step. Added a regression test `tests/runner/foreach_restore.test.ts`. From 1eb9cab855984d76fdf762b29c7bfb3b722fb5cf Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 10 Jan 2026 11:30:33 +0000 Subject: [PATCH 3/3] Fix status precedence in Foreach Step restoration Prioritize derived status from child items over stored parent status in `WorkflowState.restore`. If all items in a foreach step are successful, the step is treated as SUCCESS even if the DB record says RUNNING or PENDING. This prevents unnecessary re-execution or stalls when resuming workflows that crashed after completing all iterations but before updating the parent step. Added a regression test `tests/runner/foreach_restore.test.ts`. --- tests/runner/foreach_restore.test.ts | 39 ++++++++++++++-------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/tests/runner/foreach_restore.test.ts b/tests/runner/foreach_restore.test.ts index a370992..20a16c2 100644 --- a/tests/runner/foreach_restore.test.ts +++ b/tests/runner/foreach_restore.test.ts @@ -1,4 +1,3 @@ - import { afterAll, describe, expect, it } from 'bun:test'; import { randomUUID } from 'node:crypto'; import { existsSync, rmSync } from 'node:fs'; @@ -40,11 +39,11 @@ describe('WorkflowState Foreach Restoration', () => { // Create the item executions as SUCCESS for (let i = 0; i < items.length; i++) { - const itemStepExecId = randomUUID(); - // Correctly pass iteration index to createStep - await db.createStep(itemStepExecId, runId, stepId, i); - // We must include output so hydration works and items array is populated - await db.completeStep(itemStepExecId, StepStatus.SUCCESS, { result: items[i] }); + const itemStepExecId = randomUUID(); + // Correctly pass iteration index to createStep + await db.createStep(itemStepExecId, runId, stepId, i); + // We must include output so hydration works and items array is populated + await db.completeStep(itemStepExecId, StepStatus.SUCCESS, { result: items[i] }); } // 2. Define workflow @@ -52,28 +51,28 @@ describe('WorkflowState Foreach Restoration', () => { name: 'test-workflow', steps: [ { - id: stepId, - type: 'shell', - run: 'echo ${{ item }}', - foreach: '${{ [1, 2, 3] }}', - needs: [] + id: stepId, + type: 'shell', + run: 'echo ${{ item }}', + foreach: '${{ [1, 2, 3] }}', + needs: [], }, { - id: 'next_step', - type: 'shell', - run: 'echo "done"', - needs: [stepId] - } + id: 'next_step', + type: 'shell', + run: 'echo "done"', + needs: [stepId], + }, ], outputs: { - final: '${{ steps.next_step.output.stdout.trim() }}' - } + final: '${{ steps.next_step.output.stdout.trim() }}', + }, } as unknown as Workflow; // 3. Restore via WorkflowRunner const runner = new WorkflowRunner(workflow, { - dbPath, - resumeRunId: runId, + dbPath, + resumeRunId: runId, }); // 4. Run - it should skip the foreach step (because it detects it as SUCCESS) and run next-step