feat(derived): apply the core field scan in the MPE_DERIVED hook #402

2026-10-29T09:44:34Z

hermes commented

2026-10-29 09:44:34 +00:00

What

@sasjs/core 6.1.3 moved the derived-variable field rules out of mp_derivedvars into a macro of its own, mp_derivedvarcheck, so the same scan can run before a definition is ever used - at submit time, or over a table of expressions in a code scanning tool. mp_derivedvars calls it, so the read-time engine and a submit-time caller now measure a row against one rule list.

Data Controller had kept its own copy of that list in mpe_derived_check.sas. This PR bumps @sasjs/core to 6.1.4, deletes the DC copy, and has the MPE_DERIVED post edit hook call the core macro. The rules live in one place from now on: a change upstream reaches the EDIT screen's next read and the next submission without a second edit here.

The hook keeps the two rules that are Data Controller's rather than the expression's:

LIBREF and DSN are required - the control table key, which the loader normalises but does not require
ACTIVE must be 1 or 0 - the picker pair the EDIT screen offers over MPE_SELECTBOX, so a submission that bypasses the grid cannot land a value the screen would never offer

They are reported before the scan's own rules, so which pass found a row does not change the message it gets.

mpe_derived_check was an internal macro of the application, not part of a supported surface: after this change nothing in the repo refers to it - the only other mention was its own test file, deleted with it - so removing it breaks no consumer.

Rules that change for DC

Adopting the core list changes three behaviours, all of them tighter than before except the last:

VAR_NM may not be _N_, _ERROR_ or _IORC_ - automatic variables of the data step
VAR_LOGIC may not exceed 4000 characters - the statement buffer and the control table column, so a longer value would be cut down silently
VAR_LOGIC may not call a function that submits code or reaches the file system, the environment or the macro symbol table (DOSUBL, JAVAOBJ, SYMGET, SYMPUT(X), FOPEN, FILENAME, FDELETE, ...). Blocked characters cannot cover this: BYTE(59) builds a semicolon at run time, while a function name is part of the expression text and cannot be constructed. Quoted literals are stripped first, so a refused word that is only data is not read as a call
VAR_LABEL may carry a line break: it is flattened to a space when the label is used, which is what the engine always did, so only the remaining control characters are refused. The EDIT screen's own HARDREGEX rule still refuses a line break in the grid, so this only widens what an upload can carry

Messages are unchanged for the rules both lists share, apart from the label one above.

Mocks

sas/mocks/sasjs/services/hooks/mpe_derived_postedit.js mirrors the hook by hand, as makedata.js mirrors mpe_makedata, so the field rules there are updated to the core list (and the two passes the hook now runs). A comment in each file says the list is core's and has to be mirrored.

Tests

mpe_derived_check.test.sas is deleted with the macro. The rules it probed are core's, and core's own suite (tests/base/mp_derivedvarcheck.test.sas) covers them at macro level.
stagedata.test.4.sas gains two submit-level cases, which is where the wiring now lives: a VAR_LOGIC that calls a refused function is rejected with the scan's reason, and an ACTIVE value outside the picker pair is rejected by the hook's own rule. The header list of cases is renumbered to match.

Verification

npx sasjs lint - no warnings on the touched files (the repo's existing warnings elsewhere are untouched)
npx sasjs cb -t nextviya and -t local - the project builds, and the compiled services/hooks/mpe_derived_postedit.sas carries the mp_derivedvarcheck definition and its call, so the core macro resolves inside the service rather than relying on a runtime include
npx sasjs cbd -t local against a local SASjs Server - the pack deploys, which proves the service tree (hook included) is well formed

No SAS engine was reachable from the machine this branch was prepared on, so sasjs test has not been run: the local SASjs Server instance runs the JS runtime only (runTimes: ['js']), the Viya compute jobs submitted for the deploy stayed pending, and deploying to sas.4gl.io needs client credentials that are not present there. The PR pipeline runs lint, the Angular tests, the production build and the Cypress suite against the mocks - it does not run the SAS suite - so the two new submit-level cases are unproven until the SAS tests are run on an estate.

Follow ups

docs.datacontroller.io/docs/derived-variables.md states that a line break in VAR_LABEL is refused; with this change the server flattens it, so that sentence needs a docs PR. The page should also mention the refused-function rule, which is the security-relevant half of VAR_LOGIC.

## What `@sasjs/core` 6.1.3 moved the derived-variable field rules out of `mp_derivedvars` into a macro of its own, `mp_derivedvarcheck`, so the same scan can run before a definition is ever used - at submit time, or over a table of expressions in a code scanning tool. `mp_derivedvars` calls it, so the read-time engine and a submit-time caller now measure a row against one rule list. Data Controller had kept its own copy of that list in `mpe_derived_check.sas`. This PR bumps `@sasjs/core` to 6.1.4, deletes the DC copy, and has the `MPE_DERIVED` post edit hook call the core macro. The rules live in one place from now on: a change upstream reaches the EDIT screen's next read and the next submission without a second edit here. The hook keeps the two rules that are Data Controller's rather than the expression's: - `LIBREF` and `DSN` are required - the control table key, which the loader normalises but does not require - `ACTIVE` must be 1 or 0 - the picker pair the EDIT screen offers over `MPE_SELECTBOX`, so a submission that bypasses the grid cannot land a value the screen would never offer They are reported before the scan's own rules, so which pass found a row does not change the message it gets. `mpe_derived_check` was an internal macro of the application, not part of a supported surface: after this change nothing in the repo refers to it - the only other mention was its own test file, deleted with it - so removing it breaks no consumer. ## Rules that change for DC Adopting the core list changes three behaviours, all of them tighter than before except the last: - `VAR_NM` may not be `_N_`, `_ERROR_` or `_IORC_` - automatic variables of the data step - `VAR_LOGIC` may not exceed 4000 characters - the statement buffer and the control table column, so a longer value would be cut down silently - `VAR_LOGIC` may not call a function that submits code or reaches the file system, the environment or the macro symbol table (`DOSUBL`, `JAVAOBJ`, `SYMGET`, `SYMPUT(X)`, `FOPEN`, `FILENAME`, `FDELETE`, ...). Blocked characters cannot cover this: `BYTE(59)` builds a semicolon at run time, while a function name is part of the expression text and cannot be constructed. Quoted literals are stripped first, so a refused word that is only data is not read as a call - `VAR_LABEL` may carry a line break: it is flattened to a space when the label is used, which is what the engine always did, so only the remaining control characters are refused. The EDIT screen's own `HARDREGEX` rule still refuses a line break in the grid, so this only widens what an upload can carry Messages are unchanged for the rules both lists share, apart from the label one above. ## Mocks `sas/mocks/sasjs/services/hooks/mpe_derived_postedit.js` mirrors the hook by hand, as `makedata.js` mirrors `mpe_makedata`, so the field rules there are updated to the core list (and the two passes the hook now runs). A comment in each file says the list is core's and has to be mirrored. ## Tests - `mpe_derived_check.test.sas` is deleted with the macro. The rules it probed are core's, and core's own suite (`tests/base/mp_derivedvarcheck.test.sas`) covers them at macro level. - `stagedata.test.4.sas` gains two submit-level cases, which is where the wiring now lives: a `VAR_LOGIC` that calls a refused function is rejected with the scan's reason, and an `ACTIVE` value outside the picker pair is rejected by the hook's own rule. The header list of cases is renumbered to match. ## Verification - `npx sasjs lint` - no warnings on the touched files (the repo's existing warnings elsewhere are untouched) - `npx sasjs cb -t nextviya` and `-t local` - the project builds, and the compiled `services/hooks/mpe_derived_postedit.sas` carries the `mp_derivedvarcheck` definition and its call, so the core macro resolves inside the service rather than relying on a runtime include - `npx sasjs cbd -t local` against a local SASjs Server - the pack deploys, which proves the service tree (hook included) is well formed No SAS engine was reachable from the machine this branch was prepared on, so `sasjs test` has not been run: the local SASjs Server instance runs the JS runtime only (`runTimes: ['js']`), the Viya compute jobs submitted for the deploy stayed `pending`, and deploying to `sas.4gl.io` needs client credentials that are not present there. The PR pipeline runs lint, the Angular tests, the production build and the Cypress suite against the mocks - it does not run the SAS suite - so the two new submit-level cases are unproven until the SAS tests are run on an estate. ## Follow ups - `docs.datacontroller.io/docs/derived-variables.md` states that a line break in `VAR_LABEL` is refused; with this change the server flattens it, so that sentence needs a docs PR. The page should also mention the refused-function rule, which is the security-relevant half of `VAR_LOGIC`.

hermes reviewed

2026-10-29 09:48:17 +00:00

hermes left a comment

Reviewed the full base...head diff: the SAS hook now delegates field rules to core's mp_derivedvarcheck, mpe_derived_check.sas and its test are deleted, the JS mock is updated to match, @sasjs/core goes 6.1.0 -> 6.1.4 (pinned, lockfile updated), and stagedata.test.4 gains two cases.

sas/sasjs/services/hooks/mpe_derived_postedit.sas:~135: the two scan outputs are stacked with set work.derived_dcfield work.derived_field; and the error is then built from a name column: call symputx('der_errmsg',cats('MPE_DERIVED: ',name,': ',reason));. This only works if mp_derivedvarcheck emits name (and a reason of at least the same width). If it emits var_nm, the message degrades to "MPE_DERIVED: : " with no variable named. Confirm the output contract against core 6.1.4 and, if it differs, rename explicitly on the way in rather than relying on the stacked PDV.
sas/sasjs/macros/mpe_derived_check.sas (deleted): removing a documented macro is a breaking change for anything that includes it by name (@li mpe_derived_check.sas appears in other headers, docs and the test harness). Confirm nothing else in the repo, the sasjsconfig compile list or the docs still references it; if it is dead, say so in the PR body so consumers know.
sas/sasjs/services/editors/stagedata.test.4.sas:~620 (test 8): the expected message is written inside %str(...) across two source lines, so the literal carries a newline and the continuation indentation, while the hook emits a single-line reason. Wrap it (%sysfunc(compbl(...)), or one source line) or the assert compares a multi-line string against a single-line one and can never match.
sas/mocks/sasjs/services/hooks/mpe_derived_postedit.js:~110: the mock now re-spells core's whole rule list - AUTOMATIC_VARS, the 4000-character limit, the label flatten-then-scan, and a deny list of function names. The file comment ("a change there has to be mirrored here") is the only guard against drift, and the deny list is the part where drift is a security regression, not a UX one. Add a parity test that feeds the same fixtures through the mock and the SAS paths, or at least assert the deny list against the macro's list, rather than leaving it to a comment.
Commit subject is refactor(derived): ..., but the diff changes behaviour in both directions: new refusals (automatic variable names, deny-listed functions, 4000-character expressions) and a relaxation (a label containing a line break is now accepted, since it is flattened before the control-character scan). A refactor cuts no release, so those land silently. Use feat(derived) for the new refusals and call out the label relaxation as a behaviour change.

5 findings above for review.

Reviewed the full base...head diff: the SAS hook now delegates field rules to core's mp_derivedvarcheck, mpe_derived_check.sas and its test are deleted, the JS mock is updated to match, @sasjs/core goes 6.1.0 -> 6.1.4 (pinned, lockfile updated), and stagedata.test.4 gains two cases. - sas/sasjs/services/hooks/mpe_derived_postedit.sas:~135: the two scan outputs are stacked with `set work.derived_dcfield work.derived_field;` and the error is then built from a `name` column: `call symputx('der_errmsg',cats('MPE_DERIVED: ',name,': ',reason));`. This only works if mp_derivedvarcheck emits `name` (and a `reason` of at least the same width). If it emits `var_nm`, the message degrades to "MPE_DERIVED: : <reason>" with no variable named. Confirm the output contract against core 6.1.4 and, if it differs, rename explicitly on the way in rather than relying on the stacked PDV. - sas/sasjs/macros/mpe_derived_check.sas (deleted): removing a documented macro is a breaking change for anything that includes it by name (`@li mpe_derived_check.sas` appears in other headers, docs and the test harness). Confirm nothing else in the repo, the sasjsconfig compile list or the docs still references it; if it is dead, say so in the PR body so consumers know. - sas/sasjs/services/editors/stagedata.test.4.sas:~620 (test 8): the expected message is written inside `%str(...)` across two source lines, so the literal carries a newline and the continuation indentation, while the hook emits a single-line reason. Wrap it (`%sysfunc(compbl(...))`, or one source line) or the assert compares a multi-line string against a single-line one and can never match. - sas/mocks/sasjs/services/hooks/mpe_derived_postedit.js:~110: the mock now re-spells core's whole rule list - AUTOMATIC_VARS, the 4000-character limit, the label flatten-then-scan, and a deny list of function names. The file comment ("a change there has to be mirrored here") is the only guard against drift, and the deny list is the part where drift is a security regression, not a UX one. Add a parity test that feeds the same fixtures through the mock and the SAS paths, or at least assert the deny list against the macro's list, rather than leaving it to a comment. - Commit subject is `refactor(derived): ...`, but the diff changes behaviour in both directions: new refusals (automatic variable names, deny-listed functions, 4000-character expressions) and a relaxation (a label containing a line break is now accepted, since it is flattened before the control-character scan). A `refactor` cuts no release, so those land silently. Use `feat(derived)` for the new refusals and call out the label relaxation as a behaviour change. 5 findings above for review.

hermes force-pushed refactor/derivedvarcheck from 5830db7444 to 49b6aa3999

2026-10-29 10:01:26 +00:00

Compare

hermes changed title from ~~refactor(derived): apply the core field scan in the MPE_DERIVED hook~~ to feat(derived): apply the core field scan in the MPE_DERIVED hook

2026-10-29 10:01:32 +00:00

hermes commented

2026-10-29 10:01:42 +00:00

Thanks - all five read, and four are acted on. Responses in order:

1. name column and the stacked PDV - claim checked against core 6.1.4, it holds. mp_derivedvarcheck writes data &outds(keep=name reason); with length name $32 reason $200; and name=upcase(strip(var_nm));, so both stacked datasets carry the same two columns with the same widths, and the hook's cats('MPE_DERIVED: ',name,': ',reason) cannot lose the variable name. The name is upcased by the macro, which is what the DC copy did with strip(var_nm) on already-normalised rows, so messages read the same. Nothing renamed on the way in - a rename would only make the two shapes differ.

2. mpe_derived_check removal - it is dead after this change. It was an internal macro of the application, not a supported surface: grep -rn mpe_derived_check over the repo now returns nothing but its own (deleted) test and untracked build results, and it is not in the sasjsconfig macro lists or the test flow. The PR body says so now.

3. Test 8's expected message - valid, fixed. The value is now assembled on one line:

%let exp8=MPE_DERIVED: CALLER: VAR_LOGIC may not call a function that;
%let exp8=&exp8 submits code or reaches the file system,;
%let exp8=&exp8 the environment or the macro symbol table;

mpe_assert_abort compares "%superq(msg)" to the abort's MSG exactly, so a wrapped literal could never have matched - it would have failed the moment the suite ran. Pushed.

4. Mock parity for the deny list - accepted as a real risk, but I do not think a CI guard has a cheap home here, so I would rather decide the shape with you than bolt one on. Two constraints: the gating job (Build-and-ng-test) installs client dependencies only, so core's mp_derivedvarcheck.sas is not on disk there; and the list lives in node_modules, so a check would have to install sas/ dependencies just to read it, or keep a second copy of the list in DC - which is the drift the check is meant to prevent. What is true regardless is that the mock never affects production: drift there means an E2E case stops mirroring a server rejection, not that a bad row gets through. The existing guard for makedata.js is the same comment-only one. Options I can take in this PR: (a) leave the comment and treat the mock as a test estate, (b) add a Cypress case that submits a refuse-listed function through the mock and expects the abort, which covers the behaviour rather than the text, (c) add a .gitea/scripts check that reads the macro from sas/node_modules and add the sas install to that job - largest, and it slows every PR. Say which and I will do it here.

5. Commit type - changed to feat(derived). The new refusals are user-visible, and the label relaxation is called out in both the commit body and the PR body rather than landing under a refactor.

Thanks - all five read, and four are acted on. Responses in order: **1. `name` column and the stacked PDV - claim checked against core 6.1.4, it holds.** `mp_derivedvarcheck` writes `data &outds(keep=name reason);` with `length name $32 reason $200;` and `name=upcase(strip(var_nm));`, so both stacked datasets carry the same two columns with the same widths, and the hook's `cats('MPE_DERIVED: ',name,': ',reason)` cannot lose the variable name. The `name` is upcased by the macro, which is what the DC copy did with `strip(var_nm)` on already-normalised rows, so messages read the same. Nothing renamed on the way in - a rename would only make the two shapes differ. **2. `mpe_derived_check` removal - it is dead after this change.** It was an internal macro of the application, not a supported surface: `grep -rn mpe_derived_check` over the repo now returns nothing but its own (deleted) test and untracked build results, and it is not in the `sasjsconfig` macro lists or the test flow. The PR body says so now. **3. Test 8's expected message - valid, fixed.** The value is now assembled on one line: ``` %let exp8=MPE_DERIVED: CALLER: VAR_LOGIC may not call a function that; %let exp8=&exp8 submits code or reaches the file system,; %let exp8=&exp8 the environment or the macro symbol table; ``` `mpe_assert_abort` compares `"%superq(msg)"` to the abort's MSG exactly, so a wrapped literal could never have matched - it would have failed the moment the suite ran. Pushed. **4. Mock parity for the deny list - accepted as a real risk, but I do not think a CI guard has a cheap home here, so I would rather decide the shape with you than bolt one on.** Two constraints: the gating job (`Build-and-ng-test`) installs client dependencies only, so core's `mp_derivedvarcheck.sas` is not on disk there; and the list lives in `node_modules`, so a check would have to install `sas/` dependencies just to read it, or keep a second copy of the list in DC - which is the drift the check is meant to prevent. What is true regardless is that the mock never affects production: drift there means an E2E case stops mirroring a server rejection, not that a bad row gets through. The existing guard for `makedata.js` is the same comment-only one. Options I can take in this PR: (a) leave the comment and treat the mock as a test estate, (b) add a Cypress case that submits a refuse-listed function through the mock and expects the abort, which covers the *behaviour* rather than the text, (c) add a `.gitea/scripts` check that reads the macro from `sas/node_modules` and add the `sas` install to that job - largest, and it slows every PR. Say which and I will do it here. **5. Commit type - changed to `feat(derived)`.** The new refusals are user-visible, and the label relaxation is called out in both the commit body and the PR body rather than landing under a `refactor`.

hermes reviewed

2026-10-29 10:11:42 +00:00

hermes left a comment

Reviewed the full base...head diff: the hook now delegates the derived-variable rules to core's mp_derivedvarcheck and keeps only a local pass for LIBREF/DSN and ACTIVE, mpe_derived_check and its test are deleted, the editor mock is reordered to mirror the two passes, and tests 8 and 9 pin the new behaviour.

sas/sasjs/services/editors/stagedata.test.4.sas (Test 8): VAR_LOGIC='dosubl("data null; run;")' carries semicolons inside the quoted literal, and mp_derivedvarcheck tests index(var_logic,';') on the raw value before it ever reaches the function-name check - the abort reason is therefore "VAR_LOGIC must be a single expression - remove the ;", not the refused-function message exp8 asserts, so %mpe_assert_abort cannot pass. Use a semicolon-free payload that still names a refused function (e.g. dosubl("data null")) so the case actually exercises the deny rule.

1 findings above for review.

Reviewed the full base...head diff: the hook now delegates the derived-variable rules to core's mp_derivedvarcheck and keeps only a local pass for LIBREF/DSN and ACTIVE, mpe_derived_check and its test are deleted, the editor mock is reordered to mirror the two passes, and tests 8 and 9 pin the new behaviour. - sas/sasjs/services/editors/stagedata.test.4.sas (Test 8): VAR_LOGIC='dosubl("data _null_; run;")' carries semicolons inside the quoted literal, and mp_derivedvarcheck tests index(var_logic,';') on the raw value before it ever reaches the function-name check - the abort reason is therefore "VAR_LOGIC must be a single expression - remove the ;", not the refused-function message exp8 asserts, so %mpe_assert_abort cannot pass. Use a semicolon-free payload that still names a refused function (e.g. dosubl("data _null_")) so the case actually exercises the deny rule. 1 findings above for review.

hermes added 1 commit 2026-10-29 10:51:03 +00:00

feat(derived): apply the core field scan in the MPE_DERIVED hook

Build / Build-and-ng-test (pull_request) Successful in 6m6s

Details

Lighthouse Checks / lighthouse (pull_request) Successful in 20m15s

Details

Build / Build-and-test-development (pull_request) Successful in 32m40s

Details

a4a0f9cb2f

@sasjs/core 6.1.3 moved the derived variable field rules out of
mp_derivedvars into mp_derivedvarcheck, so the same scan can run before a
definition is used.  DC carried its own copy of that list in
mpe_derived_check.sas, which is now deleted: the post edit hook calls the
core macro instead, and the hook keeps only the rules that belong to the
control table rather than the expression - LIBREF/DSN required, and ACTIVE
within the picker pair.

The scan is stricter than the copy it replaces, in three ways:

- VAR_NM may not be _N_, _ERROR_ or _IORC_ (automatic variables of the data
  step)
- VAR_LOGIC may not exceed 4000 characters
- VAR_LOGIC may not call a function that submits code or reaches the file
  system, the environment or the macro symbol table - a function name cannot
  be assembled at run time, so this is a control the expression cannot evade

and it relaxes one: VAR_LABEL may carry a line break, which is flattened to a
space when the label is used, so only the remaining control characters are
refused.

The mock hook mirror is updated to the same rules, and stagedata.test.4
gains the two submit-time cases that pin the new wiring.

hermes force-pushed refactor/derivedvarcheck from 49b6aa3999 to a4a0f9cb2f

2026-10-29 10:51:03 +00:00

Compare

hermes commented

2026-10-29 10:51:10 +00:00

Both test-side findings are fixed and pushed (a4a0f9c):

Test 8 payload: correct - the quoted semicolon tripped the single-expression rule first, so the case could never have reached the deny list. It is now VAR_LOGIC='dosubl("data _null_")', with a comment saying why the payload carries no semicolon.
Test 8 expected message: already assembled on one line via successive %lets, so the exact comparison in mpe_assert_abort gets a single-line value.

Also from the first pass: the name/reason output contract of mp_derivedvarcheck holds (data &outds(keep=name reason) with length name $32 reason $200, name=upcase(strip(var_nm))), the deleted macro is dead after this change, and the commit is now feat(derived).

CI on 49b6aa3 was green (gitleaks, audit, lint, licence, Angular tests in three timezones, the spec-list guard, production build, the Cypress suite against the mocks, Lighthouse). Re-running on a4a0f9c.

Both test-side findings are fixed and pushed (`a4a0f9c`): - **Test 8 payload**: correct - the quoted semicolon tripped the single-expression rule first, so the case could never have reached the deny list. It is now `VAR_LOGIC='dosubl("data _null_")'`, with a comment saying why the payload carries no semicolon. - **Test 8 expected message**: already assembled on one line via successive `%let`s, so the exact comparison in `mpe_assert_abort` gets a single-line value. Also from the first pass: the `name`/`reason` output contract of `mp_derivedvarcheck` holds (`data &outds(keep=name reason)` with `length name $32 reason $200`, `name=upcase(strip(var_nm))`), the deleted macro is dead after this change, and the commit is now `feat(derived)`. CI on `49b6aa3` was green (gitleaks, audit, lint, licence, Angular tests in three timezones, the spec-list guard, production build, the Cypress suite against the mocks, Lighthouse). Re-running on `a4a0f9c`.

hermes reviewed

2026-10-29 10:57:08 +00:00

hermes left a comment

Reviewed the full base...head diff against core 6.1.4 (verified the mp_derivedvarcheck output contract - it emits NAME/REASON - and that mp_derivedvars calls it, so the "one rule list" claim in mpe_derived.sas holds), the rebuilt two-pass hook, the removal of mpe_derived_check and its test (no remaining references in the tree), the editor mock mirroring both passes in the same order, and tests 8/9. Pinning and lockfile for @sasjs/core 6.1.4 are correct.

sas/sasjs/macros/mpe_derived_check.sas (deleted) + commit subject "feat(derived): ...": the macro ships inside the sas9.sas / viya.sas bundles that are deployed to the SAS server, so customer programs that include it by name stop compiling after the upgrade. The commit carries no ! or BREAKING CHANGE: footer, so semantic-release cuts a minor and the CHANGELOG says nothing about the removal. Either mark it breaking (feat! / BREAKING CHANGE footer) or keep a deprecated stub for one release that calls mp_derivedvarcheck and returns the old outds shape.
sas/mocks/sasjs/services/hooks/mpe_derived_postedit.js:175: else if (logic.length > 4000) counts UTF-16 code units, while core applies klength(var_logic) and the label check nine lines earlier (line 172) uses [...label].length for the same character semantics. Use [...logic].length so the mock does not reject a multi-byte expression core accepts.

2 findings above for review.

Reviewed the full base...head diff against core 6.1.4 (verified the mp_derivedvarcheck output contract - it emits NAME/REASON - and that mp_derivedvars calls it, so the "one rule list" claim in mpe_derived.sas holds), the rebuilt two-pass hook, the removal of mpe_derived_check and its test (no remaining references in the tree), the editor mock mirroring both passes in the same order, and tests 8/9. Pinning and lockfile for @sasjs/core 6.1.4 are correct. - sas/sasjs/macros/mpe_derived_check.sas (deleted) + commit subject "feat(derived): ...": the macro ships inside the sas9.sas / viya.sas bundles that are deployed to the SAS server, so customer programs that include it by name stop compiling after the upgrade. The commit carries no `!` or BREAKING CHANGE: footer, so semantic-release cuts a minor and the CHANGELOG says nothing about the removal. Either mark it breaking (feat! / BREAKING CHANGE footer) or keep a deprecated stub for one release that calls mp_derivedvarcheck and returns the old outds shape. - sas/mocks/sasjs/services/hooks/mpe_derived_postedit.js:175: `else if (logic.length > 4000)` counts UTF-16 code units, while core applies klength(var_logic) and the label check nine lines earlier (line 172) uses `[...label].length` for the same character semantics. Use `[...logic].length` so the mock does not reject a multi-byte expression core accepts. 2 findings above for review.

allan merged commit f74100a818 into release/7.21

2026-10-29 11:25:27 +00:00

allan deleted branch refactor/derivedvarcheck

2026-10-29 11:25:28 +00:00

hermes referenced this pull request

2026-10-29 15:11:42 +00:00

feat: Release/7.21 #401

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dc/dc#402