Skip to content

Commit 02a873f

Browse files
WyvernMonarchYuriy Butenko
andauthored
fix(workflows): reject divergent root history (#8)
Co-authored-by: Yuriy Butenko <yuriy@MacBook-Pro-Yuriy.local>
1 parent 817c19c commit 02a873f

3 files changed

Lines changed: 55 additions & 21 deletions

File tree

packages/workflows/src/context.ts

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -565,29 +565,20 @@ export class WorkflowContextImpl implements WorkflowContextInterface {
565565
}
566566

567567
/**
568-
* Validate that all expected entries in the branch were visited.
569-
* Throws HistoryDivergedError if there are unvisited entries.
568+
* Validate that every direct entry in this scope was visited. Nested scopes
569+
* validate their own entries when they execute.
570570
*/
571571
validateComplete(): void {
572-
const prefix = locationToKey(this.storage, this.currentLocation);
573-
574-
for (const key of this.storage.history.entries.keys()) {
575-
// Check if this key is under our current location prefix
576-
// Handle root prefix (empty string) specially - all keys are under root
577-
const isUnderPrefix =
578-
prefix === ""
579-
? true // Root: all keys are children
580-
: key.startsWith(`${prefix}/`) || key === prefix;
572+
for (const [key, entry] of this.storage.history.entries) {
573+
const isDirectChild =
574+
entry.location.length === this.currentLocation.length + 1 &&
575+
isLocationPrefix(this.currentLocation, entry.location);
581576

582-
if (isUnderPrefix) {
583-
if (!this.visitedKeys.has(key)) {
584-
// Entry exists in history but wasn't visited
585-
// This means workflow code may have changed
586-
throw new HistoryDivergedError(
587-
`Entry "${key}" exists in history but was not visited. ` +
588-
`Workflow code may have changed. Use ctx.removed() to handle migrations.`,
589-
);
590-
}
577+
if (isDirectChild && !this.visitedKeys.has(key)) {
578+
throw new HistoryDivergedError(
579+
`Entry "${key}" exists in history but was not visited. ` +
580+
"Workflow code may have changed. Use ctx.removed() to handle migrations.",
581+
);
591582
}
592583
}
593584
}
@@ -791,6 +782,13 @@ export class WorkflowContextImpl implements WorkflowContextInterface {
791782

792783
// Check for duplicate name in current execution
793784
this.checkDuplicateName(config.name);
785+
const parentKey = locationToKey(this.storage, this.currentLocation);
786+
const candidateKey = parentKey
787+
? `${parentKey}/${config.name}`
788+
: config.name;
789+
if (!this.storage.history.entries.has(candidateKey)) {
790+
this.validateComplete();
791+
}
794792

795793
const location = appendName(
796794
this.storage,
@@ -2558,6 +2556,14 @@ export class WorkflowContextImpl implements WorkflowContextInterface {
25582556

25592557
// Mark this entry as visited for validateComplete
25602558
this.markVisited(key);
2559+
if (originalType === "message") {
2560+
const generatedKeyPrefix = `${key}:`;
2561+
for (const existingKey of this.storage.history.entries.keys()) {
2562+
if (existingKey.startsWith(generatedKeyPrefix)) {
2563+
this.markVisited(existingKey);
2564+
}
2565+
}
2566+
}
25612567

25622568
this.stopRollbackIfMissing(existing);
25632569

packages/workflows/src/index.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ import {
154154
import {
155155
CriticalError,
156156
EvictedError,
157+
HistoryDivergedError,
157158
MessageWaitError,
158159
RollbackCheckpointError,
159160
RollbackError,
@@ -993,6 +994,7 @@ async function executeWorkflow<TInput, TOutput>(
993994

994995
try {
995996
const output = await workflowFn(ctx, effectiveInput);
997+
ctx.validateComplete();
996998

997999
storage.state = "completed";
9981000
storage.output = output;
@@ -1035,7 +1037,10 @@ async function executeWorkflow<TInput, TOutput>(
10351037
);
10361038
}
10371039

1038-
if (error instanceof RollbackCheckpointError) {
1040+
if (
1041+
error instanceof HistoryDivergedError ||
1042+
error instanceof RollbackCheckpointError
1043+
) {
10391044
await setFailedState(storage, driver, error, historyNotifier);
10401045
if (onError && !isErrorReported(error)) {
10411046
await notifyError(onError, logger, {

packages/workflows/tests/steps.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
EntryInProgressError,
77
HistoryDivergedError,
88
InMemoryDriver,
9+
loadStorage,
910
RollbackError,
1011
runWorkflow,
1112
StepExhaustedError,
@@ -74,6 +75,28 @@ for (const mode of modes) {
7475
.result;
7576
expect(callCount).toBe(1);
7677
});
78+
it("should reject a renamed root step on replay", async () => {
79+
const originalWorkflow = async (ctx: WorkflowContextInterface) => {
80+
return await ctx.step("original-step-name", async () => "original");
81+
};
82+
const renamedWorkflow = async (ctx: WorkflowContextInterface) => {
83+
return await ctx.step("renamed-step-name", async () => "changed");
84+
};
85+
86+
await runWorkflow("wf-1", originalWorkflow, undefined, driver, { mode })
87+
.result;
88+
89+
await expect(
90+
runWorkflow("wf-1", renamedWorkflow, undefined, driver, { mode })
91+
.result,
92+
).rejects.toThrow(HistoryDivergedError);
93+
const storage = await loadStorage(driver);
94+
expect(storage.nameRegistry).toEqual(["original-step-name"]);
95+
expect([...storage.history.entries.keys()]).toEqual([
96+
"original-step-name",
97+
]);
98+
expect(storage.state).not.toBe("completed");
99+
});
77100

78101
it("should replay void step on restart", async () => {
79102
let callCount = 0;

0 commit comments

Comments
 (0)