Skip to content

Commit 12c7537

Browse files
jumskijumski
committed
fix skip-cascade type bug: only skip mode makes deps optional
1 parent ff2191e commit 12c7537

3 files changed

Lines changed: 129 additions & 41 deletions

File tree

‎pkgs/core/supabase/tests/condition_evaluation/ifnot_empty_object_absent_dep.test.sql‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
-- Test: ifNot empty object pattern - step runs when dependency is absent/skipped
2+
--
3+
-- Verifies that ifNot: { dep: {} } (empty object pattern) correctly detects
4+
-- when a dependency is absent (skipped) and causes the fallback step to run.
5+
--
6+
-- PostgreSQL containment semantics:
7+
-- - When dep is skipped, deps_output is {} (empty object)
8+
-- - {} @> { dep: {} } = FALSE (empty object doesn't contain dep key)
9+
-- - NOT(FALSE) = TRUE, so ifNot condition is met -> step runs
10+
begin;
11+
select plan(2);
12+
13+
select pgflow_tests.reset_db();
14+
15+
-- Create flow: skippable_dep -> fallback (with ifNot: { skippable_dep: {} })
16+
select pgflow.create_flow('empty_pattern_test');
17+
18+
-- Step A: Skippable based on input pattern
19+
select pgflow.add_step(
20+
flow_slug => 'empty_pattern_test',
21+
step_slug => 'skippable_dep',
22+
required_input_pattern => '{"run_dep": true}'::jsonb,
23+
when_unmet => 'skip'
24+
);
25+
26+
-- Step B: Fallback - runs when A is absent (empty object pattern)
27+
select pgflow.add_step(
28+
flow_slug => 'empty_pattern_test',
29+
step_slug => 'fallback',
30+
deps_slugs => ARRAY['skippable_dep'],
31+
forbidden_input_pattern => '{"skippable_dep": {}}'::jsonb,
32+
when_unmet => 'skip'
33+
);
34+
35+
-- Start flow with input that causes dep to skip (run_dep: false)
36+
with flow as (
37+
select * from pgflow.start_flow('empty_pattern_test', '{"run_dep": false}'::jsonb)
38+
)
39+
select run_id into temporary run_ids from flow;
40+
41+
-- Test 1: skippable_dep should be skipped
42+
select is(
43+
(select status from pgflow.step_states
44+
where run_id = (select run_id from run_ids) and step_slug = 'skippable_dep'),
45+
'skipped',
46+
'Dependency with unmet condition should be skipped'
47+
);
48+
49+
-- Test 2: fallback should be started (empty object pattern matched -> ifNot passed)
50+
select is(
51+
(select status from pgflow.step_states
52+
where run_id = (select run_id from run_ids) and step_slug = 'fallback'),
53+
'started',
54+
'Step with ifNot: {dep: {}} should start when dep is absent'
55+
);
56+
57+
drop table if exists run_ids;
58+
select finish();
59+
rollback;

‎pkgs/dsl/__tests__/types/skippable-deps.test-d.ts‎

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,24 @@ describe('skippable deps type safety', () => {
3030
}>();
3131
});
3232

33-
it('step with whenUnmet: skip-cascade makes output optional for dependents', () => {
33+
it('step with whenUnmet: skip-cascade keeps output required (cascade skips dependents)', () => {
34+
// skip-cascade means if the step is skipped, its dependents are ALSO skipped
35+
// So if the dependent handler runs at all, the parent must have succeeded
36+
// Therefore the dependency should be required, not optional
3437
const flow = new Flow<{ value: number }>({ slug: 'test' })
3538
.step(
3639
{ slug: 'conditional', if: { value: 42 }, whenUnmet: 'skip-cascade' },
3740
(input) => ({ result: input.value * 2 })
3841
)
3942
.step({ slug: 'dependent', dependsOn: ['conditional'] }, (deps) => {
40-
expectTypeOf(deps.conditional).toEqualTypeOf<
41-
{ result: number } | undefined
42-
>();
43+
// With skip-cascade, if we're running, the dependency succeeded
44+
expectTypeOf(deps.conditional).toEqualTypeOf<{ result: number }>();
4345
return { done: true };
4446
});
4547

4648
type DepInput = StepInput<typeof flow, 'dependent'>;
4749
expectTypeOf<DepInput>().toEqualTypeOf<{
48-
conditional?: { result: number };
50+
conditional: { result: number };
4951
}>();
5052
});
5153

@@ -101,21 +103,22 @@ describe('skippable deps type safety', () => {
101103
}>();
102104
});
103105

104-
it('step with retriesExhausted: skip-cascade makes output optional for dependents', () => {
106+
it('step with retriesExhausted: skip-cascade keeps output required (cascade skips dependents)', () => {
107+
// skip-cascade means if the step is skipped, its dependents are ALSO skipped
108+
// So if the dependent handler runs at all, the parent must have succeeded
105109
const flow = new Flow<{ value: number }>({ slug: 'test' })
106110
.step({ slug: 'risky', retriesExhausted: 'skip-cascade' }, (input) => ({
107111
result: input.value * 2,
108112
}))
109113
.step({ slug: 'dependent', dependsOn: ['risky'] }, (deps) => {
110-
expectTypeOf(deps.risky).toEqualTypeOf<
111-
{ result: number } | undefined
112-
>();
114+
// With skip-cascade, if we're running, the dependency succeeded
115+
expectTypeOf(deps.risky).toEqualTypeOf<{ result: number }>();
113116
return { done: true };
114117
});
115118

116119
type DepInput = StepInput<typeof flow, 'dependent'>;
117120
expectTypeOf<DepInput>().toEqualTypeOf<{
118-
risky?: { result: number };
121+
risky: { result: number };
119122
}>();
120123
});
121124

@@ -251,16 +254,17 @@ describe('skippable deps type safety', () => {
251254
}>();
252255
});
253256

254-
it('cascade does NOT propagate: A(skip-cascade) -> B: B output NOT automatically optional', () => {
257+
it('cascade does NOT propagate: A(skip-cascade) -> B: B sees A as required', () => {
255258
// skip-cascade means A and its dependents get skipped at RUNTIME
256-
// but B itself is not marked as skippable in its definition
257-
// so if B does run, its output is required for its own dependents
259+
// If A is skipped, B is also skipped (cascade), so B never runs with undefined A
260+
// Therefore B should see A as required, not optional
258261
const flow = new Flow<{ value: number }>({ slug: 'test' })
259262
.step({ slug: 'a', retriesExhausted: 'skip-cascade' }, () => ({
260263
aVal: 1,
261264
}))
262265
.step({ slug: 'b', dependsOn: ['a'] }, (deps) => {
263-
expectTypeOf(deps.a).toEqualTypeOf<{ aVal: number } | undefined>();
266+
// With skip-cascade, if B runs, A must have succeeded
267+
expectTypeOf(deps.a).toEqualTypeOf<{ aVal: number }>();
264268
return { bVal: 2 };
265269
})
266270
.step({ slug: 'c', dependsOn: ['b'] }, (deps) => {
@@ -269,6 +273,9 @@ describe('skippable deps type safety', () => {
269273
return { cVal: 3 };
270274
});
271275

276+
type BInput = StepInput<typeof flow, 'b'>;
277+
expectTypeOf<BInput>().toEqualTypeOf<{ a: { aVal: number } }>();
278+
272279
type CInput = StepInput<typeof flow, 'c'>;
273280
expectTypeOf<CInput>().toEqualTypeOf<{ b: { bVal: number } }>();
274281
});
@@ -475,26 +482,29 @@ describe('skippable deps compile-time errors', () => {
475482
});
476483
});
477484

478-
it('should reject direct property access with whenUnmet: skip-cascade', () => {
485+
it('should ALLOW direct property access with whenUnmet: skip-cascade (cascade skips dependents)', () => {
486+
// With skip-cascade, if the dependent runs, the parent must have succeeded
487+
// So direct property access should be allowed
479488
new Flow<{ value: number }>({ slug: 'test' })
480489
.step(
481490
{ slug: 'cascading', if: { value: 42 }, whenUnmet: 'skip-cascade' },
482491
() => ({ count: 10 })
483492
)
484493
.step({ slug: 'next', dependsOn: ['cascading'] }, (deps) => {
485-
// @ts-expect-error - deps.cascading is optional due to whenUnmet: skip-cascade
494+
// No error - deps.cascading is required with skip-cascade
486495
const num: number = deps.cascading.count;
487496
return { num };
488497
});
489498
});
490499

491-
it('should reject direct property access with retriesExhausted: skip-cascade', () => {
500+
it('should ALLOW direct property access with retriesExhausted: skip-cascade (cascade skips dependents)', () => {
501+
// With skip-cascade, if the dependent runs, the parent must have succeeded
492502
new Flow<{ value: number }>({ slug: 'test' })
493503
.step({ slug: 'risky', retriesExhausted: 'skip-cascade' }, () => ({
494504
status: 'ok',
495505
}))
496506
.step({ slug: 'next', dependsOn: ['risky'] }, (deps) => {
497-
// @ts-expect-error - deps.risky is optional due to retriesExhausted: skip-cascade
507+
// No error - deps.risky is required with skip-cascade
498508
const s: string = deps.risky.status;
499509
return { s };
500510
});

‎pkgs/dsl/src/dsl.ts‎

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,14 @@ export type AnyInput = Json;
6666
export type AnyOutput = Json;
6767

6868
// Step Types
69+
// Skippable mode: 'skip' makes deps optional, 'skip-cascade' keeps deps required
70+
// (because cascade skips dependents at runtime, so if handler runs, dep succeeded)
71+
export type SkippableMode = 'skip' | 'skip-cascade' | false;
72+
6973
// Step metadata structure - enriched type that tracks output and skippability
7074
export interface StepMeta<
7175
TOutput = AnyOutput,
72-
TSkippable extends boolean = boolean
76+
TSkippable extends SkippableMode = SkippableMode
7377
> {
7478
output: TOutput;
7579
skippable: TSkippable;
@@ -285,23 +289,34 @@ export type StepOutput<
285289
: never;
286290

287291
/**
288-
* Checks if a step is skippable (has whenUnmet: 'skip' | 'skip-cascade' or retriesExhausted: 'skip' | 'skip-cascade')
292+
* Gets the skippable mode for a step ('skip' | 'skip-cascade' | false)
289293
* @template TFlow - The Flow type
290294
* @template TStepSlug - The step slug to check
291295
*/
292-
export type IsStepSkippable<
296+
export type GetSkippableMode<
293297
TFlow extends AnyFlow,
294298
TStepSlug extends string
295299
> = TStepSlug extends keyof ExtractFlowStepsRaw<TFlow>
296300
? ExtractFlowStepsRaw<TFlow>[TStepSlug]['skippable']
297301
: false;
298302

303+
/**
304+
* Checks if a step makes its dependents' deps optional (only 'skip' mode, not 'skip-cascade')
305+
* With 'skip-cascade', dependents are also skipped at runtime, so if handler runs, dep succeeded.
306+
*/
307+
export type IsStepSkippable<
308+
TFlow extends AnyFlow,
309+
TStepSlug extends string
310+
> = GetSkippableMode<TFlow, TStepSlug> extends 'skip' ? true : false;
311+
299312
// Helper types for StepInput with optional skippable deps
313+
// Only 'skip' mode makes deps optional (dependents run with undefined value)
314+
// 'skip-cascade' keeps deps required (dependents also skipped, so value guaranteed if running)
300315
type RequiredDeps<TFlow extends AnyFlow, TStepSlug extends string> = {
301316
[K in Extract<
302317
keyof ExtractFlowSteps<TFlow>,
303318
StepDepsOf<TFlow, TStepSlug>
304-
> as IsStepSkippable<TFlow, K & string> extends true
319+
> as GetSkippableMode<TFlow, K & string> extends 'skip'
305320
? never
306321
: K]: ExtractFlowSteps<TFlow>[K];
307322
};
@@ -310,7 +325,7 @@ type OptionalDeps<TFlow extends AnyFlow, TStepSlug extends string> = {
310325
[K in Extract<
311326
keyof ExtractFlowSteps<TFlow>,
312327
StepDepsOf<TFlow, TStepSlug>
313-
> as IsStepSkippable<TFlow, K & string> extends true
328+
> as GetSkippableMode<TFlow, K & string> extends 'skip'
314329
? K
315330
: never]?: ExtractFlowSteps<TFlow>[K];
316331
};
@@ -319,8 +334,10 @@ type OptionalDeps<TFlow extends AnyFlow, TStepSlug extends string> = {
319334
* Asymmetric step input type:
320335
* - Root steps (no dependencies): receive flow input directly
321336
* - Dependent steps: receive only their dependencies (flow input available via context)
322-
* - Skippable deps (whenUnmet/retriesExhausted: 'skip' | 'skip-cascade') are optional
323-
* - Required deps are required
337+
* - Skippable deps (whenUnmet/retriesExhausted: 'skip') are optional
338+
* - Cascade deps (whenUnmet/retriesExhausted: 'skip-cascade') are required
339+
* (because if handler runs, the dependency must have succeeded)
340+
* - All other deps are required
324341
*
325342
* This enables functional composition where subflows can receive typed inputs
326343
* without the 'run' wrapper that previously blocked type matching.
@@ -446,21 +463,23 @@ export type RetriesExhaustedMode = 'fail' | 'skip' | 'skip-cascade';
446463

447464
/**
448465
* Helper type for dependent step handlers - creates deps object with correct optionality.
449-
* Skippable deps (steps with whenUnmet/retriesExhausted: 'skip' | 'skip-cascade') are optional.
450-
* Required deps are required.
466+
* Only steps with 'skip' mode (not 'skip-cascade') make deps optional.
467+
* With 'skip-cascade', dependents are also skipped at runtime, so if handler runs, dep succeeded.
451468
*/
452469
type DepsWithOptionalSkippable<
453470
TSteps extends AnySteps,
454471
TDeps extends string
455472
> = {
473+
// Required deps: either not skippable or skip-cascade (cascade skips dependents, so value guaranteed)
456474
[K in TDeps as K extends keyof TSteps
457-
? TSteps[K]['skippable'] extends true
475+
? TSteps[K]['skippable'] extends 'skip'
458476
? never
459477
: K
460478
: K]: K extends keyof TSteps ? TSteps[K]['output'] : never;
461479
} & {
480+
// Optional deps: only 'skip' mode (dependents run with undefined value)
462481
[K in TDeps as K extends keyof TSteps
463-
? TSteps[K]['skippable'] extends true
482+
? TSteps[K]['skippable'] extends 'skip'
464483
? K
465484
: never
466485
: never]?: K extends keyof TSteps ? TSteps[K]['output'] : never;
@@ -726,9 +745,9 @@ export class Flow<
726745
[K in Slug]: StepMeta<
727746
Awaited<TOutput>,
728747
TWhenUnmet extends 'skip' | 'skip-cascade'
729-
? true
748+
? TWhenUnmet
730749
: TRetries extends 'skip' | 'skip-cascade'
731-
? true
750+
? TRetries
732751
: false
733752
>;
734753
},
@@ -775,9 +794,9 @@ export class Flow<
775794
[K in Slug]: StepMeta<
776795
Awaited<TOutput>,
777796
TWhenUnmet extends 'skip' | 'skip-cascade'
778-
? true
797+
? TWhenUnmet
779798
: TRetries extends 'skip' | 'skip-cascade'
780-
? true
799+
? TRetries
781800
: false
782801
>;
783802
},
@@ -891,9 +910,9 @@ export class Flow<
891910
[K in Slug]: StepMeta<
892911
Awaited<TOutput>,
893912
TWhenUnmet extends 'skip' | 'skip-cascade'
894-
? true
913+
? TWhenUnmet
895914
: TRetries extends 'skip' | 'skip-cascade'
896-
? true
915+
? TRetries
897916
: false
898917
>;
899918
},
@@ -939,9 +958,9 @@ export class Flow<
939958
[K in Slug]: StepMeta<
940959
Awaited<TOutput>,
941960
TWhenUnmet extends 'skip' | 'skip-cascade'
942-
? true
961+
? TWhenUnmet
943962
: TRetries extends 'skip' | 'skip-cascade'
944-
? true
963+
? TRetries
945964
: false
946965
>;
947966
},
@@ -999,9 +1018,9 @@ export class Flow<
9991018
[K in Slug]: StepMeta<
10001019
AwaitedReturn<THandler>[],
10011020
TWhenUnmet extends 'skip' | 'skip-cascade'
1002-
? true
1021+
? TWhenUnmet
10031022
: TRetries extends 'skip' | 'skip-cascade'
1004-
? true
1023+
? TRetries
10051024
: false
10061025
>;
10071026
},
@@ -1048,9 +1067,9 @@ export class Flow<
10481067
[K in Slug]: StepMeta<
10491068
AwaitedReturn<THandler>[],
10501069
TWhenUnmet extends 'skip' | 'skip-cascade'
1051-
? true
1070+
? TWhenUnmet
10521071
: TRetries extends 'skip' | 'skip-cascade'
1053-
? true
1072+
? TRetries
10541073
: false
10551074
>;
10561075
},

0 commit comments

Comments
 (0)