@@ -463,12 +463,19 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
463463 expect ( issues . some ( i => i . where . includes ( 'requiredWhen' ) && / b a r e r e f e r e n c e ` s t a t u s ` / . test ( i . message ) ) ) . toBe ( true ) ;
464464 } ) ;
465465
466+ // `qty` carries `required` + `defaultValue` so it can never be null. That
467+ // is not decoration: it mirrors the real `showcase_invoice_line.quantity`,
468+ // and without it `record.qty >= 100` is a genuine #4811 finding — `>=` on a
469+ // nullable declared field, which faults at runtime and makes the
470+ // `requiredWhen` silently unenforced. This case is about bare-vs-qualified
471+ // references (#1928) and the `parent` namespace, so the fixture is pinned
472+ // to the non-null shape rather than the gate being loosened around it.
466473 it ( 'accepts record-qualified field rules and the master-detail `parent` namespace' , ( ) => {
467474 const issues = validateStackExpressions ( {
468475 objects : [ {
469476 name : 'inv_line' ,
470477 fields : {
471- qty : { type : 'number' , readonlyWhen : "parent.status == 'paid'" } ,
478+ qty : { type : 'number' , required : true , defaultValue : 1 , readonlyWhen : "parent.status == 'paid'" } ,
472479 note : { type : 'text' , requiredWhen : 'record.qty >= 100' } ,
473480 } ,
474481 } ] ,
@@ -919,6 +926,103 @@ describe('null-guard gate (#4763)', () => {
919926 } ) ;
920927 } ) ;
921928
929+ // #4811 — the one surface the coverage review found to MEET the gate's
930+ // totality criterion: `evaluateValidationRules` evaluates a field's
931+ // `requiredWhen` against the same `materializeDeclaredFields`-merged record
932+ // the object's validation rules see. It is also the quietest failure of the
933+ // three covered surfaces: a faulting `requiredWhen` is fail-OPEN (logged and
934+ // skipped), so the field is simply never required and the write sails through.
935+ describe ( 'field `requiredWhen` — covered since #4811' , ( ) => {
936+ const withField = ( requiredWhen : string ) =>
937+ validateStackExpressions ( {
938+ objects : [ { ...project , fields : { ...project . fields , note : { type : 'text' , requiredWhen } } } ] ,
939+ } ) ;
940+
941+ it ( 'REJECTS the `has(a) && has(b) && a < b` shape on a requiredWhen predicate' , ( ) => {
942+ const issues = withField (
943+ 'has(record.start_date) && has(record.end_date) && record.end_date < record.start_date' ,
944+ ) ;
945+ expect ( issues . length ) . toBeGreaterThan ( 0 ) ;
946+ expect ( issues . every ( ( i ) => ( i . severity ?? 'error' ) === 'error' ) ) . toBe ( true ) ;
947+ const joined = issues . map ( ( i ) => i . message ) . join ( '\n' ) ;
948+ // names the slot …
949+ expect ( joined ) . toContain ( "field 'note' requiredWhen" ) ;
950+ // … the operands …
951+ expect ( joined ) . toContain ( 'record.end_date' ) ;
952+ expect ( joined ) . toContain ( 'record.start_date' ) ;
953+ // … and the fix, in the runtime's own words (identical to every other
954+ // surface this gate covers — one voice, #4763).
955+ expect ( joined ) . toContain ( "Guard it with '!= null'" ) ;
956+ expect ( joined ) . toContain ( 'has(x)' ) ;
957+ expect ( issues [ 0 ] . where ) . toContain ( "object 'showcase_project' · field 'note' requiredWhen" ) ;
958+ } ) ;
959+
960+ it ( 'names `has()` explicitly as a non-guard when that is all the author wrote' , ( ) => {
961+ const issues = withField ( 'has(record.budget) && record.budget > 100' ) ;
962+ expect ( issues ) . toHaveLength ( 1 ) ;
963+ expect ( issues [ 0 ] . message ) . toContain ( '`has(record.budget)` does not guard it' ) ;
964+ } ) ;
965+
966+ // The consequence clause is per-surface, and getting it wrong sends the
967+ // author to the wrong place. `requiredWhen` is fail-OPEN — `rule-validator`
968+ // logs and skips — so it must NOT borrow the validation rules' "the write
969+ // is rejected fail-closed" wording.
970+ it ( 'reports the fail-OPEN consequence, not the validation rules’ fail-closed one' , ( ) => {
971+ const [ issue ] = withField ( 'has(record.budget) && record.budget > 100' ) ;
972+ expect ( issue . message ) . toContain ( 'SKIPPED fail-open' ) ;
973+ expect ( issue . message ) . toContain ( 'the field is never actually required' ) ;
974+ expect ( issue . message ) . not . toContain ( 'rejected fail-closed' ) ;
975+ } ) ;
976+
977+ it ( 'leaves the fail-closed wording on the surfaces that really fail closed' , ( ) => {
978+ const [ issue ] = validateStackExpressions ( {
979+ objects : [ { ...project , validations : [ { type : 'script' , name : 'r' , condition : 'record.budget > 1' } ] } ] ,
980+ } ) ;
981+ expect ( issue . message ) . toContain ( 'rejected fail-closed' ) ;
982+ expect ( issue . message ) . not . toContain ( 'SKIPPED fail-open' ) ;
983+ } ) ;
984+
985+ it ( 'ACCEPTS the `!= null` form' , ( ) => {
986+ expect (
987+ withField ( 'record.start_date != null && record.end_date != null && record.end_date < record.start_date' ) ,
988+ ) . toHaveLength ( 0 ) ;
989+ } ) ;
990+
991+ it ( 'never flags a required field or one carrying a default' , ( ) => {
992+ expect ( withField ( 'record.spent > 0' ) ) . toHaveLength ( 0 ) ;
993+ } ) ;
994+
995+ // `has()` over a key the object does not declare is the macro's LEGITIMATE
996+ // use ("was this key in the PATCH?") and must never draw a null-guard
997+ // verdict — a false positive here is worse than a miss. Asserted on the
998+ // null-guard verdict specifically: the independent #1928 field-existence
999+ // pass has its own (pre-existing, correct) opinion about an undeclared
1000+ // name on a record-scoped slot, and that is not what this pins.
1001+ it ( 'leaves `has()` on an UNDECLARED key alone — its legitimate use' , ( ) => {
1002+ const nullGuardIssues = withField ( 'has(record.some_transient_key)' )
1003+ . filter ( ( i ) => i . message . includes ( "Guard it with '!= null'" ) ) ;
1004+ expect ( nullGuardIssues ) . toHaveLength ( 0 ) ;
1005+ } ) ;
1006+
1007+ // Live-metadata pin: `showcase_invoice_line.description` really does carry
1008+ // `requiredWhen: record.quantity >= 100`, and `quantity` is `required: true`
1009+ // WITH `defaultValue: 1`, so it can never be null. This predicate must stay
1010+ // green — flagging it would be the false positive that is worse than a miss.
1011+ it ( 'leaves the real `showcase_invoice_line` requiredWhen alone' , ( ) => {
1012+ expect (
1013+ validateStackExpressions ( {
1014+ objects : [ {
1015+ name : 'showcase_invoice_line' ,
1016+ fields : {
1017+ quantity : { type : 'number' , required : true , defaultValue : 1 } ,
1018+ description : { type : 'text' , requiredWhen : 'record.quantity >= 100' } ,
1019+ } ,
1020+ } ] ,
1021+ } ) ,
1022+ ) . toHaveLength ( 0 ) ;
1023+ } ) ;
1024+ } ) ;
1025+
9221026 describe ( 'surfaces deliberately NOT covered' , ( ) => {
9231027 it ( 'leaves sharing-rule conditions alone (compiled to a SQL filter, never faults)' , ( ) => {
9241028 expect (
@@ -933,7 +1037,15 @@ describe('null-guard gate (#4763)', () => {
9331037 ) . toHaveLength ( 0 ) ;
9341038 } ) ;
9351039
936- it ( 'leaves flattened flow conditions alone (a bare id may be a flow variable)' , ( ) => {
1040+ // #4811 re-measured the reason this one is excluded. It is NOT the
1041+ // flattened scope (this gate never resolves a bare identifier — only
1042+ // `record.<f>`/`previous.<f>`, and the engine binds both roots
1043+ // unconditionally). It is that `record-change-trigger.ts` seeds the flow's
1044+ // record as `{ ...inputDoc, ...after }` with no `materializeDeclaredFields`,
1045+ // so a declared column the write never mentioned is an ABSENT key — and on
1046+ // an absent key the `!= null` this gate prescribes faults exactly like the
1047+ // comparison it was meant to guard.
1048+ it ( 'leaves flow conditions alone (trigger record is not total over declared fields)' , ( ) => {
9371049 expect (
9381050 validateStackExpressions ( {
9391051 objects : [ project ] ,
@@ -943,7 +1055,41 @@ describe('null-guard gate (#4763)', () => {
9431055 { id : 'start' , type : 'start' , config : { objectName : 'showcase_project' } } ,
9441056 { id : 'd' , type : 'decision' , config : { condition : 'record.budget > 100000' } } ,
9451057 ] ,
946- edges : [ ] ,
1058+ edges : [ { id : 'e1' , source : 'd' , target : 'end' , condition : 'record.spent > record.budget' } ] ,
1059+ } ] ,
1060+ } ) ,
1061+ ) . toHaveLength ( 0 ) ;
1062+ } ) ;
1063+
1064+ // Action predicates reach real CEL and fail closed, so the trap bites here
1065+ // too — but the record bound is whatever the client fetched (a list row
1066+ // carries only the view's projected columns) and nothing materializes it,
1067+ // so `!= null` would be the wrong prescription. Excluded until that binding
1068+ // is made total; see the ledger in `validate-null-guards.ts`.
1069+ it ( 'leaves action `visible` / `disabled` alone (client record is not total)' , ( ) => {
1070+ expect (
1071+ validateStackExpressions ( {
1072+ objects : [ {
1073+ ...project ,
1074+ actions : [ {
1075+ name : 'escalate' ,
1076+ visible : 'has(record.budget) && has(record.spent) && record.spent > record.budget' ,
1077+ disabled : 'record.budget < 1000' ,
1078+ } ] ,
1079+ } ] ,
1080+ } ) ,
1081+ ) . toHaveLength ( 0 ) ;
1082+ } ) ;
1083+
1084+ // Same field as the covered `requiredWhen`, opposite verdict — the split is
1085+ // the point. `readonlyWhen` is evaluated by `stripReadonlyWhenFields`, which
1086+ // merges `{ ...previous, ...data }` and never materializes.
1087+ it ( 'leaves field `readonlyWhen` alone (strip path merges without materializing)' , ( ) => {
1088+ expect (
1089+ validateStackExpressions ( {
1090+ objects : [ {
1091+ ...project ,
1092+ fields : { ...project . fields , note : { type : 'text' , readonlyWhen : 'record.budget > 100' } } ,
9471093 } ] ,
9481094 } ) ,
9491095 ) . toHaveLength ( 0 ) ;
0 commit comments