fix(deps): bump @sasjs/core to 7.0.1 so special missings can be restored #409

Merged

allan merged 3 commits from fix/restore-special-missings into release/7.22 2026-10-30 17:23:32 +00:00

Conversation 3 Commits 3 Files Changed 6

+193 -7

hermes commented 2026-10-30 16:07:45 +00:00

Collaborator

Copy link

Copy source

What

Bumps @sasjs/core from 6.1.4 to 7.0.1 so that restoring a load which touched a special missing numeric works, adds a test that reproduces the failure (issue #368), and fixes two defects found in the test path while proving it.

The bug this fixes

mp_stripdiffs reverts a load by generating proc sql; update ... set <col> = <value> statements. PUT writes a special missing numeric as its letter only, so a value of ._ reached the statement as the bare token _:

update work.stage set SOME_BESTNUM =_  where ... ;
ERROR: The following columns were not found in the contributing tables: _.
ERROR: SOME_BESTNUM, a numeric column, can only be updated with a numeric expression.

PROC SQL then put the whole block into NOEXEC, so the revert ran against unchanged data and the service's syscc check is what surfaced it. Any table holding ._ or .A-.Z in a column that was changed, or in a row that was deleted, was affected - the restore simply could not complete.

Fixed upstream in sasjs/core#499, released as @sasjs/core 7.0.1: the period is written before the value, and .A-.Z/._ are covered (the guard is missing(v) and v ne ., because .A sorts above .).

Breaking changes between 6.1.4 and 7.0.1

v7.0.0 changed mf_existvarlist to warn and return 0 instead of aborting when libds/varlist is blank. DC does not call it - there are no direct references in sas/sasjs - and the core macros that do (mp_assertcols, mp_stackdiffs, mv_jobflow) are covered by the suite run below. v6 is already absorbed by the 6.1.4 pin.

Test

sas/sasjs/services/editors/restore.test.2.sas:

1. seeds some_bestnum=._ on mpe_x_test (the audit stores whatever the base holds, so this is the value the revert has to write back);
2. changes that cell to 42 through stagedata and approves it through postdata;
3. reverts that load through services/editors/restore and approves the reversion;
4. asserts the cell is ._ again - not blank, not 42;
5. puts the fixture back as makedata left it.

Steps 1-3 are the issue #368 path: the diff's old value is the special missing, so it is the value the generated UPDATE has to carry.

Two defects found while proving it

Both were in the way of the test, not in the bump, and both are fixed here:

• mpe_execute lost a child's abort message. Its failfast abort was built with ,msg=%str(&program did not complete: state=&state &msg). %str masks the literal text, not the resolved value, so commas in the child's own message stayed delimiters: the call split into extra positional parameters and the caller got ERROR: All positional parameters must precede keyword parameters instead of the child's failure. Now %bquote, which masks the resolved value. The same shape in mpe_security_postedit (a submitted ACCESS_LEVEL value interpolated into an abort) is fixed the same way.
• stagedata.test.4 cancelled itself on an expected abort. Test 8 submits an expression the derived-variable scan refuses, so the child is meant to abort - but its %mpe_execute call omitted failfast=0, which the file's other abort-expecting calls pass. Added.

Evidence

Run on a scratch appLoc with makedata for a fresh control library, sasjs cbd before each run.

Red - core 6.1.4, the version being replaced:

aad23eaf-7720-4c9c-b08d-a66c8fe0aaa1  mpe_checkrestore  PASS
abb9c448-67aa-4b7c-84b8-dab209b6bd0f  restore           FAIL
a312e6d5-0a96-40aa-bb12-defdad03883e  restore           PASS

with Checking the revert was submitted, Checking the reversion was approved and Checking the special missing was restored all FAIL. The pre-existing restore.test passes in both runs - its fixture has no special missings, which is why this went unnoticed.

Green - core 7.0.1:

0bd004f1-52d8-41c6-82b4-99e8feda3972  mpe_checkrestore  PASS
0a95b4b3-b73f-4d6e-a3dd-19732594ef70  restore           PASS
01fc6c68-7bfa-45e6-a344-86b28ee90977  restore           PASS

Full SAS suite on 7.0.1: 63 suites PASS, 1 FAIL - stagedata.test.4. That failure is pre-existing and independent of the bump: it fails identically on 6.1.4, and it is not caused by anything in this PR's earlier state either. With the two fixes above it is reduced to one specific scenario - the child process aborts inside the hook and never writes a response, so the assertion has nothing to read - which is now tracked as #411. The other abort-expecting cases in that file pass.

Not run

SAS 9 and Viya lanes - no estate for them from this session. The change is a macro-library bump plus SAS-only regression tests, and the affected macro is base (all platforms).

Closes #368

## What Bumps `@sasjs/core` from 6.1.4 to **7.0.1** so that restoring a load which touched a special missing numeric works, adds a test that reproduces the failure (issue #368), and fixes two defects found in the test path while proving it. ## The bug this fixes `mp_stripdiffs` reverts a load by generating `proc sql; update ... set <col> = <value>` statements. `PUT` writes a special missing numeric as its letter only, so a value of `._` reached the statement as the bare token `_`: ``` update work.stage set SOME_BESTNUM =_ where ... ; ERROR: The following columns were not found in the contributing tables: _. ERROR: SOME_BESTNUM, a numeric column, can only be updated with a numeric expression. ``` PROC SQL then put the whole block into NOEXEC, so the revert ran against unchanged data and the service's `syscc` check is what surfaced it. Any table holding `._` or `.A`-`.Z` in a column that was changed, or in a row that was deleted, was affected - the restore simply could not complete. Fixed upstream in [sasjs/core#499](https://github.com/sasjs/core/pull/499), released as `@sasjs/core` 7.0.1: the period is written before the value, and `.A`-`.Z`/`._` are covered (the guard is `missing(v) and v ne .`, because `.A` sorts above `.`). ## Breaking changes between 6.1.4 and 7.0.1 v7.0.0 changed `mf_existvarlist` to warn and return 0 instead of aborting when `libds`/`varlist` is blank. DC does not call it - there are no direct references in `sas/sasjs` - and the core macros that do (`mp_assertcols`, `mp_stackdiffs`, `mv_jobflow`) are covered by the suite run below. v6 is already absorbed by the 6.1.4 pin. ## Test `sas/sasjs/services/editors/restore.test.2.sas`: 1. seeds `some_bestnum=._` on `mpe_x_test` (the audit stores whatever the base holds, so this is the value the revert has to write back); 2. changes that cell to `42` through `stagedata` and approves it through `postdata`; 3. reverts that load through `services/editors/restore` and approves the reversion; 4. asserts the cell is `._` again - not blank, not `42`; 5. puts the fixture back as `makedata` left it. Steps 1-3 are the issue #368 path: the diff's old value is the special missing, so it is the value the generated UPDATE has to carry. ## Two defects found while proving it Both were in the way of the test, not in the bump, and both are fixed here: - **`mpe_execute` lost a child's abort message.** Its failfast abort was built with `,msg=%str(&program did not complete: state=&state &msg)`. `%str` masks the literal text, not the resolved value, so commas in the child's own message stayed delimiters: the call split into extra positional parameters and the caller got `ERROR: All positional parameters must precede keyword parameters` instead of the child's failure. Now `%bquote`, which masks the resolved value. The same shape in `mpe_security_postedit` (a submitted `ACCESS_LEVEL` value interpolated into an abort) is fixed the same way. - **`stagedata.test.4` cancelled itself on an expected abort.** Test 8 submits an expression the derived-variable scan refuses, so the child is meant to abort - but its `%mpe_execute` call omitted `failfast=0`, which the file's other abort-expecting calls pass. Added. ## Evidence Run on a scratch appLoc with `makedata` for a fresh control library, `sasjs cbd` before each run. **Red** - core 6.1.4, the version being replaced: ``` aad23eaf-7720-4c9c-b08d-a66c8fe0aaa1 mpe_checkrestore PASS abb9c448-67aa-4b7c-84b8-dab209b6bd0f restore FAIL a312e6d5-0a96-40aa-bb12-defdad03883e restore PASS ``` with `Checking the revert was submitted`, `Checking the reversion was approved` and `Checking the special missing was restored` all FAIL. The pre-existing `restore.test` passes in both runs - its fixture has no special missings, which is why this went unnoticed. **Green** - core 7.0.1: ``` 0bd004f1-52d8-41c6-82b4-99e8feda3972 mpe_checkrestore PASS 0a95b4b3-b73f-4d6e-a3dd-19732594ef70 restore PASS 01fc6c68-7bfa-45e6-a344-86b28ee90977 restore PASS ``` **Full SAS suite** on 7.0.1: 63 suites PASS, 1 FAIL - `stagedata.test.4`. That failure is **pre-existing and independent of the bump**: it fails identically on 6.1.4, and it is not caused by anything in this PR's earlier state either. With the two fixes above it is reduced to one specific scenario - the child process aborts inside the hook and never writes a response, so the assertion has nothing to read - which is now tracked as #411. The other abort-expecting cases in that file pass. ## Not run SAS 9 and Viya lanes - no estate for them from this session. The change is a macro-library bump plus SAS-only regression tests, and the affected macro is base (all platforms). Closes #368

hermes added 1 commit 2026-10-30 16:07:45 +00:00

fix(deps): bump @sasjs/core to 7.0.1 so special missings can be restored ... 3d2eefdd7b

Restoring any load that touched a special missing numeric (._ or .A-.Z) failed
in mp_stripdiffs, which writes the old value into a generated UPDATE statement.
PUT writes a special missing as its letter only, so the statement carried
"set SOME_BESTNUM =_" and PROC SQL read the bare underscore as a column name:

  ERROR: The following columns were not found in the contributing tables: _.
  ERROR: SOME_BESTNUM, a numeric column, can only be updated with a numeric
    expression.

Every statement in the generated block then failed NOEXEC, so the revert
produced the wrong data; the service's syscc check is what surfaced it.

Fixed upstream in sasjs/core#499 (the period is now written first) and
released as 7.0.1. v7.0.0's breaking change - mf_existvarlist warns and returns
0 instead of aborting on blank input - does not affect DC: there are no direct
callers, and the full SAS suite covers the core macros that do use it.

Adds restore.test.2.sas, which seeds some_bestnum=._ on mpe_x_test, changes it
to a number through stagedata and approves it, reverts that load, approves the
reversion, and asserts the cell is back to ._ rather than blank. It fails on
6.1.4 and passes on 7.0.1.

hermes reviewed 2026-10-30 16:27:19 +00:00

hermes left a comment

Author

Collaborator

Copy link

Copy source

REVIEWED ON 2026-10-30 16:27 UTC

Reviewed the full diff: @sasjs/core 6.1.4 -> 7.0.1 pinned in sas/package.json with the lockfile updated, plus the new special-missing restore test.

• sas/package-lock.json - the bump leaves @sasjs/core 6.1.4 nested at node_modules/@sasjs/cli/node_modules/@sasjs/core. If the compile resolves core from the CLI's own dependency tree rather than the project root, 6.1.4 is the macro that gets compiled and the fix never reaches the deployed makedata service. Confirm the compiled artifact carries the 7.0.1 mp_stripdiffs before merging.

1 finding above for review.

REVIEWED ON 2026-10-30 16:27 UTC Reviewed the full diff: @sasjs/core 6.1.4 -> 7.0.1 pinned in sas/package.json with the lockfile updated, plus the new special-missing restore test. - sas/package-lock.json - the bump leaves @sasjs/core 6.1.4 nested at node_modules/@sasjs/cli/node_modules/@sasjs/core. If the compile resolves core from the CLI's own dependency tree rather than the project root, 6.1.4 is the macro that gets compiled and the fix never reaches the deployed makedata service. Confirm the compiled artifact carries the 7.0.1 mp_stripdiffs before merging. 1 finding above for review.

hermes commented 2026-10-30 17:18:10 +00:00

Author

Collaborator

Copy link

Copy source

Checked, and the concern does not apply here - the compiler takes core from the project root, not from the CLI's tree.

node_modules/@sasjs/core                                  -> 7.0.1
node_modules/@sasjs/cli/node_modules/@sasjs/core          -> 6.1.4   (as you saw)

The artifact built by sasjs cbd -t server at this head is sasjsbuild/server.sas (single file, what gets deployed), plus the per-service copies. Both carry the 7.0.1 guard - the period-prefixed special-missing branch that does not exist in 6.1.4:

$ grep -c "missing(oldval_num) and oldval_num ne\|missing(newval_num) and newval_num ne" \
    sasjsbuild/services/editors/restore.sas
5           # the five sites in mp_stripdiffs: 4 oldval, 1 newval

node_modules/@sasjs/cli/node_modules/@sasjs/core is 6.1.4 because the CLI declares @sasjs/core: ^6.0.0 as its own dependency - that copy is the CLI's macro set, and no macro from it is inlined into a project build.

The behavioural proof is the pair of runs already in the PR body: with 6.1.4 installed at the root, restore.test.2 fails on "revert was submitted", "reversion was approved" and "special missing was restored"; with 7.0.1 at the root it passes. Both runs were rebuilt with sasjs cbd immediately beforehand, so what the deployed services executed changed with the root version and nothing else.

If you would rather the nested copy were not there at all, that is a CLI change, not this one: it needs either an npm overrides pin forcing @sasjs/cli onto a core it does not declare, or a CLI release on the 7.x line. Happy to raise it against @sasjs/cli as a follow-up if you want the tree flattened.

Checked, and the concern does not apply here - the compiler takes core from the project root, not from the CLI's tree. ```text node_modules/@sasjs/core -> 7.0.1 node_modules/@sasjs/cli/node_modules/@sasjs/core -> 6.1.4 (as you saw) ``` The artifact built by `sasjs cbd -t server` at this head is `sasjsbuild/server.sas` (single file, what gets deployed), plus the per-service copies. Both carry the 7.0.1 guard - the period-prefixed special-missing branch that does not exist in 6.1.4: ```text $ grep -c "missing(oldval_num) and oldval_num ne\|missing(newval_num) and newval_num ne" \ sasjsbuild/services/editors/restore.sas 5 # the five sites in mp_stripdiffs: 4 oldval, 1 newval ``` `node_modules/@sasjs/cli/node_modules/@sasjs/core` is 6.1.4 because the CLI declares `@sasjs/core: ^6.0.0` as its own dependency - that copy is the CLI's macro set, and no macro from it is inlined into a project build. The behavioural proof is the pair of runs already in the PR body: with 6.1.4 installed at the root, `restore.test.2` fails on "revert was submitted", "reversion was approved" and "special missing was restored"; with 7.0.1 at the root it passes. Both runs were rebuilt with `sasjs cbd` immediately beforehand, so what the deployed services executed changed with the root version and nothing else. If you would rather the nested copy were not there at all, that is a CLI change, not this one: it needs either an npm `overrides` pin forcing `@sasjs/cli` onto a core it does not declare, or a CLI release on the 7.x line. Happy to raise it against `@sasjs/cli` as a follow-up if you want the tree flattened.

hermes added 2 commits 2026-10-30 17:20:06 +00:00

fix(mpe_execute): mask a child abort message before it aborts the caller ... 20dbf00fb0

mpe_execute reports a child that did not complete by aborting the caller with
"&program did not complete: state=&state &msg". The message was passed through
%str, which masks the literal text before resolution - so the commas in a
child's own abort message (mp_abort writes them as part of the reason) were
still delimiters when the call was parsed. The call split into extra positional
parameters and SAS raised

  ERROR: All positional parameters must precede keyword parameters.

replacing the real failure with a parse error at the call site. The caller then
lost the child's message entirely.

%bquote masks the resolved value, so the message arrives as one argument and the
abort reports what the child actually said.

The same shape in mpe_security_postedit, where the submitted ACCESS_LEVEL value
is interpolated into the abort message, is fixed the same way.

test(stagedata): do not let an expected child abort cancel the test ... dfc5c6ebb8

Test 8 submits a VAR_LOGIC expression that the derived-variable scan refuses, so
the child is expected to abort and the assertion reads its message from the
response. Without failfast=0 mpe_execute aborted this program instead, so the
case could never reach its assertion. The file's other abort-expecting calls
already pass failfast=0.

This does not make the case pass on its own - the child still cancels without
writing a response, which is tracked separately - but it removes the wrong
failure in front of it.

hermes commented 2026-10-30 17:20:23 +00:00

Author

Collaborator

Copy link

Copy source

Two further commits on the branch, both found while proving the regression test, plus an issue for the one thing they do not fix.

• 20dbf00 - mpe_execute passed a child's abort message through %str, so a comma in that message split its own abort call and the caller saw All positional parameters must precede keyword parameters instead of the child's failure. %bquote masks the resolved value; the same shape in mpe_security_postedit is fixed too.
• dfc5c6e - stagedata.test.4 test 8 called %mpe_execute without failfast=0 although the child is expected to abort, so the test cancelled itself before its assertion. The file's other abort-expecting calls already passed it.

With those, stagedata.test.4 fails for exactly one reason: the child aborts inside the hook and writes no response, so the assertion has nothing to read. That is independent of the core bump - identical on 6.1.4 and 7.0.1 - and is now #411, with the log chain and the ruled-out causes.

PR body updated with the two commits and the corrected suite statement (63 pass, 1 fail, tracked).

Two further commits on the branch, both found while proving the regression test, plus an issue for the one thing they do not fix. - `20dbf00` - `mpe_execute` passed a child's abort message through `%str`, so a comma in that message split its own abort call and the caller saw `All positional parameters must precede keyword parameters` instead of the child's failure. `%bquote` masks the resolved value; the same shape in `mpe_security_postedit` is fixed too. - `dfc5c6e` - `stagedata.test.4` test 8 called `%mpe_execute` without `failfast=0` although the child is expected to abort, so the test cancelled itself before its assertion. The file's other abort-expecting calls already passed it. With those, `stagedata.test.4` fails for exactly one reason: the child aborts inside the hook and writes no response, so the assertion has nothing to read. That is independent of the core bump - identical on 6.1.4 and 7.0.1 - and is now #411, with the log chain and the ruled-out causes. PR body updated with the two commits and the corrected suite statement (63 pass, 1 fail, tracked).

allan merged commit 8862798d65 into release/7.22 2026-10-30 17:23:32 +00:00

allan deleted branch fix/restore-special-missings 2026-10-30 17:23:33 +00:00

hermes referenced this pull request 2026-10-30 17:52:34 +00:00

fix(mpe_execute): keep a child abort message intact when reporting it #412

Sign in to join this conversation.

Reviewers

No Reviewers

hermes

Labels

Clear labels

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

wontfix

This won't be fixed

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

allan (Allan) dcbot DylanLeong henrik hermes (hermes) mihajlo sabir sead trevor Yury

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dc/dc#409