fix(mocks): refuse a library getcols does not have, as the estate does #512

2026-11-07T18:25:09Z

hermes commented

2026-11-07 18:25:09 +00:00

The last of the "what cannot be goldened" list that needed no estate changes.

getcols.sas validates LIBDS against the libraries it can assign before it reads anything (%mpe_validatecol, then dc_assignlib), so a LIBDS naming a library the estate does not have aborts Invalid libds. The mock searched every libref instead, so NO_SUCH_LIB.MPE_TABLES answered with the columns of the copy that does exist in the control library - a request the estate refuses, answered with data from somewhere else.

The named library is now resolved the way the estate assigns it. A missing or malformed LIBDS reaches the same abort, with the same message and MAC. A member missing from a library that does exist is deliberately left alone: the estate fails inside its query there (a SAS syntax error rather than a designed abort), and there is no message worth copying.

Recorded as public/getcols/wrong_libref.

The flake this exposed

The suite failed intermittently - the whole parity-check file, with no assertion, which is a crashed process. The harness pruned earlier session directories, and a service whose headers are written with the async fs.writeFile can land after its directory is removed; the ENOENT then took the process down. Each run still gets its own directory, which is what stops a stale body being served, and the estate is removed when the process exits. Three consecutive full-suite runs are green since.

Verification

node sas/mocks/test/parity/check.js - 18 scenarios, all ok, 0 unaccepted / 0 stale
node --test sas/mocks/test/*.test.js - 131 pass, 0 fail, three times in a row

The last of the "what cannot be goldened" list that needed no estate changes. `getcols.sas` validates `LIBDS` against the libraries it can assign before it reads anything (`%mpe_validatecol`, then `dc_assignlib`), so a `LIBDS` naming a library the estate does not have aborts `Invalid libds`. The mock searched every libref instead, so `NO_SUCH_LIB.MPE_TABLES` answered with the columns of the copy that does exist in the control library - a request the estate refuses, answered with data from somewhere else. The named library is now resolved the way the estate assigns it. A missing or malformed `LIBDS` reaches the same abort, with the same message and MAC. A member missing from a library that *does* exist is deliberately left alone: the estate fails inside its query there (a SAS syntax error rather than a designed abort), and there is no message worth copying. Recorded as `public/getcols/wrong_libref`. ## The flake this exposed The suite failed intermittently - the whole `parity-check` file, with no assertion, which is a crashed process. The harness pruned earlier session directories, and a service whose headers are written with the async `fs.writeFile` can land after its directory is removed; the `ENOENT` then took the process down. Each run still gets its own directory, which is what stops a stale body being served, and the estate is removed when the process exits. Three consecutive full-suite runs are green since. ## Verification - `node sas/mocks/test/parity/check.js` - 18 scenarios, all ok, 0 unaccepted / 0 stale - `node --test sas/mocks/test/*.test.js` - 131 pass, 0 fail, three times in a row

hermes added 1 commit 2026-11-07 18:25:09 +00:00

fix(mocks): refuse a library getcols does not have, as the estate does

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

Details

Build / lighthouse (pull_request) Successful in 7m25s

Details

Build / Build-and-test-development (pull_request) Canceled after 21m18s

Details

f851bfafb4

getcols.sas validates LIBDS against the libraries it can assign before it reads
anything (%mpe_validatecol, then dc_assignlib), so a LIBDS naming a library the
estate does not have aborts "Invalid libds". The mock searched every libref
instead, so NO_SUCH_LIB.MPE_TABLES answered with the columns of the copy that
does exist in the control library - a request the estate refuses, answered with
data from somewhere else.

The named library is resolved the way the estate assigns it, and a missing or
malformed LIBDS reaches the same abort. A member missing from a library that
does exist is left alone: the estate fails inside its query there, and there is
no message worth copying.

Recorded as public/getcols/wrong_libref.

Also fixes an intermittent suite failure this work exposed: the harness pruned
earlier session directories, so a service whose headers are written with the
async fs.writeFile could land after its directory was removed, and the ENOENT
took the test process down. Each run still gets its own directory - that is what
stops a stale body being served - and the estate is removed when the process
exits.

hermes reviewed

2026-11-07 18:38:41 +00:00

hermes left a comment

Reviewed the diff at f851bfaf (base c2fcce47). The mocks suite is green at this head (node --test sas/mocks/test/*.test.js, 131 pass) and the resolution itself is right: the libref comes from LIBDS and the member is looked up only in that library. The refusal is not right, and the test that pins it does not test what its name says.

sas/mocks/sasjs/services/public/getcols.js:46 - if (!libds || !table || !knownLib) { answers Invalid libds for a well-formed libref the estate does not have, a message the estate cannot emit for that input. getcols.sas:42 guards that abort on &is_libds ne 1, which is %mp_validatecol(_libds,LIBDS,...), and the LIBDS rule is /^[_a-z]\w{0,7}\.[_a-z]\w{0,31}$/i - shape only, as @sasjs/core's own docstring puts it for ISLIB: "checks if the value is a valid libref (NOT whether it exists)". NOLIB.MPE_TABLES passes that check, reaches %dc_assignlib and fails there, and what the caller sees is the second abort at getcols.sas:66-68, msg=%str(syscc=&syscc). Probed at this head: runService(estate,'public/getcols',{rows:[{LIBDS:'NOLIB.MPE_TABLES'}]}) answers {"sasjsAbort":[{"MSG":"Invalid libds","MAC":"/Public/app/dc/services/public/getcols.sas"}]}. Either carry syscc=<n> on the unknown-library path, or state in the comment at 32-42 that the mock deliberately collapses "library unknown" into "libds malformed" - as written it claims estate parity for a message the estate reserves for malformed input.
sas/mocks/test/parity/scenarios.json:571 - "LIBDS": "NO_SUCH_LIB.MPE_TABLES" cannot be evidence for the new test. NO_SUCH_LIB is 11 characters and the LIBDS rule caps the libref at 8, so the value is malformed and takes the Invalid libds abort with or without the new check: delete the knownLib branch entirely and this golden still reproduces. Re-capture the scenario with a well-formed unassigned libref (NOLIB.MPE_TABLES) and the estate's real answer for it, or the test at sas/mocks/test/read-paths.test.js:100 keeps measuring the shape check under a name that claims the existence one.
sas/mocks/sasjs/services/public/getcols.js:51 and the comment at 40-41 - const tableData = makeTableLoader(libDataDir(libref))(table) || null with "the estate fails inside its query, and there is no message there worth copying". There is a message: getcols.sas:66-68 aborts syscc=<n> when SYSCC is non-zero after %mp_getcols. A member missing from a library that does exist therefore returns {"cols":[],"SYSCC":"0"} from the mock (probed with DC_JSLIB.NO_SUCH_TABLE) where the estate aborts, so the client draws an empty column picker instead of showing the error. Pre-existing rather than a regression of this PR, but the comment now cements it - carry the abort, or drop the claim.
sas/mocks/test/harness.js:117 - removing the session prune is a second, unrelated change inside a getcols PR, and the ENOENT race it explains is not reproduced by any test here. It looks like a reasonable harness fix; ship it with a test that exercises the async header write, or on its own, so this diff reads as one change.

CI at this head is still running (Build / Build-and-ng-test pending), so this is code review only.

4 findings above for review.

Reviewed the diff at f851bfaf (base c2fcce47). The mocks suite is green at this head (`node --test sas/mocks/test/*.test.js`, 131 pass) and the resolution itself is right: the libref comes from LIBDS and the member is looked up only in that library. The refusal is not right, and the test that pins it does not test what its name says. - sas/mocks/sasjs/services/public/getcols.js:46 - `if (!libds || !table || !knownLib) {` answers `Invalid libds` for a well-formed libref the estate does not have, a message the estate cannot emit for that input. getcols.sas:42 guards that abort on `&is_libds ne 1`, which is `%mp_validatecol(_libds,LIBDS,...)`, and the LIBDS rule is `/^[_a-z]\w{0,7}\.[_a-z]\w{0,31}$/i` - shape only, as @sasjs/core's own docstring puts it for ISLIB: "checks if the value is a valid libref (NOT whether it exists)". `NOLIB.MPE_TABLES` passes that check, reaches `%dc_assignlib` and fails there, and what the caller sees is the second abort at getcols.sas:66-68, `msg=%str(syscc=&syscc)`. Probed at this head: `runService(estate,'public/getcols',{rows:[{LIBDS:'NOLIB.MPE_TABLES'}]})` answers `{"sasjsAbort":[{"MSG":"Invalid libds","MAC":"/Public/app/dc/services/public/getcols.sas"}]}`. Either carry `syscc=<n>` on the unknown-library path, or state in the comment at 32-42 that the mock deliberately collapses "library unknown" into "libds malformed" - as written it claims estate parity for a message the estate reserves for malformed input. - sas/mocks/test/parity/scenarios.json:571 - `"LIBDS": "NO_SUCH_LIB.MPE_TABLES"` cannot be evidence for the new test. `NO_SUCH_LIB` is 11 characters and the LIBDS rule caps the libref at 8, so the value is malformed and takes the `Invalid libds` abort with or without the new check: delete the `knownLib` branch entirely and this golden still reproduces. Re-capture the scenario with a well-formed unassigned libref (`NOLIB.MPE_TABLES`) and the estate's real answer for it, or the test at sas/mocks/test/read-paths.test.js:100 keeps measuring the shape check under a name that claims the existence one. - sas/mocks/sasjs/services/public/getcols.js:51 and the comment at 40-41 - `const tableData = makeTableLoader(libDataDir(libref))(table) || null` with "the estate fails inside its query, and there is no message there worth copying". There is a message: getcols.sas:66-68 aborts `syscc=<n>` when SYSCC is non-zero after `%mp_getcols`. A member missing from a library that does exist therefore returns `{"cols":[],"SYSCC":"0"}` from the mock (probed with `DC_JSLIB.NO_SUCH_TABLE`) where the estate aborts, so the client draws an empty column picker instead of showing the error. Pre-existing rather than a regression of this PR, but the comment now cements it - carry the abort, or drop the claim. - sas/mocks/test/harness.js:117 - removing the session prune is a second, unrelated change inside a getcols PR, and the ENOENT race it explains is not reproduced by any test here. It looks like a reasonable harness fix; ship it with a test that exercises the async header write, or on its own, so this diff reads as one change. CI at this head is still running (Build / Build-and-ng-test pending), so this is code review only. 4 findings above for review.

hermes referenced this pull request

2026-11-07 18:45:14 +00:00

feat(mocks): give every control table its key, and generate the DDL download #510

gitea-actions Bot commented

2026-11-07 19:19:57 +00:00

Lighthouse

Route	Performance	Accessibility	Best practices	SEO
`#/home/tables`	0.51	0.93	1.00	1.00
`#/editor/DC996664.MPE_X_TEST`	0.30	0.96	1.00	1.00
`#/view/data`	0.53	0.92	1.00	1.00
`#/view/data/DC996664`	0.54	0.92	1.00	1.00
`#/view/data/DC996664.MPE_X_TEST`	0.46	0.92	1.00	1.00
`#/view/usernav/groups`	0.54	0.92	1.00	1.00
`#/view/usernav/groups/1`	0.54	0.92	1.00	1.00
`#/view/usernav/users/1`	0.54	0.92	1.00	1.00
`#/home/excel-maps`	0.53	0.92	1.00	1.00
`#/home/excel-maps/BASEL-CR2`	0.52	0.87	1.00	1.00
`#/home/multi-load`	0.52	0.90	1.00	1.00
`#/review/submitted`	0.54	0.91	1.00	1.00
`#/review/approve`	0.53	0.91	1.00	1.00
`#/review/history`	0.53	0.91	1.00	1.00
`#/stage/DC20221006T142649516_059582_7169`	0.53	0.96	1.00	0.92
`#/review/submitted/DC20221006T142649516_059582_7169`	0.54	0.91	1.00	1.00
`#/system`	0.52	0.93	1.00	1.00

Lowest accessibility score: 0.87 (the gate is a median of 0.85 while issue 497 is open).

Run: https://git.datacontroller.io/dc/dc/actions/runs/3459

### Lighthouse | Route | Performance | Accessibility | Best practices | SEO | |---|---|---|---|---| | `#/home/tables` | 0.51 | 0.93 | 1.00 | 1.00 | | `#/editor/DC996664.MPE_X_TEST` | 0.30 | 0.96 | 1.00 | 1.00 | | `#/view/data` | 0.53 | 0.92 | 1.00 | 1.00 | | `#/view/data/DC996664` | 0.54 | 0.92 | 1.00 | 1.00 | | `#/view/data/DC996664.MPE_X_TEST` | 0.46 | 0.92 | 1.00 | 1.00 | | `#/view/usernav/groups` | 0.54 | 0.92 | 1.00 | 1.00 | | `#/view/usernav/groups/1` | 0.54 | 0.92 | 1.00 | 1.00 | | `#/view/usernav/users/1` | 0.54 | 0.92 | 1.00 | 1.00 | | `#/home/excel-maps` | 0.53 | 0.92 | 1.00 | 1.00 | | `#/home/excel-maps/BASEL-CR2` | 0.52 | 0.87 | 1.00 | 1.00 | | `#/home/multi-load` | 0.52 | 0.90 | 1.00 | 1.00 | | `#/review/submitted` | 0.54 | 0.91 | 1.00 | 1.00 | | `#/review/approve` | 0.53 | 0.91 | 1.00 | 1.00 | | `#/review/history` | 0.53 | 0.91 | 1.00 | 1.00 | | `#/stage/DC20221006T142649516_059582_7169` | 0.53 | 0.96 | 1.00 | 0.92 | | `#/review/submitted/DC20221006T142649516_059582_7169` | 0.54 | 0.91 | 1.00 | 1.00 | | `#/system` | 0.52 | 0.93 | 1.00 | 1.00 | Lowest accessibility score: **0.87** (the gate is a median of 0.85 while issue 497 is open). Run: https://git.datacontroller.io/dc/dc/actions/runs/3459

allan added 1 commit 2026-11-07 19:27:12 +00:00

Merge branch 'release/7.23' into fix/mock-getcols-libref

Build / Build-and-ng-test (pull_request) Successful in 4m23s

Details

Build / lighthouse (pull_request) Successful in 6m37s

Details

Build / Build-and-test-development (pull_request) Successful in 23m26s

Details

d0b2ca268b

hermes referenced this pull request

2026-11-07 19:27:15 +00:00

revert(client): undo the lazy route split (#508) #511

hermes reviewed

2026-11-07 19:44:13 +00:00

hermes left a comment

Re-review at d0b2ca26, superseding the review at f851bfaf. Delta: the head gains only a merge of release/7.23 - git diff f851bfaf HEAD -- sas/ is empty, so the getcols change is byte-identical to the version reviewed before and nothing was closed. Re-ran the suite and the probes at this head: node --test sas/mocks/test/*.test.js is 131 pass, and the three probes below answer exactly as they did at f851bfaf. The four findings stand:

sas/mocks/sasjs/services/public/getcols.js:46 - if (!libds || !table || !knownLib) { sends a well-formed libref the estate does not have into Invalid libds. getcols.sas:42 guards that abort on %mpe_validatecol(_libds,LIBDS,...), whose rule is shape-only (/^[_a-z]\w{0,7}\.[_a-z]\w{0,31}$/i, and @sasjs/core's ISLIB docstring says so: "checks if the value is a valid libref (NOT whether it exists)"). NOLIB.MPE_TABLES passes that shape check, reaches %dc_assignlib, fails there, and the caller sees the second abort at getcols.sas:66-68, msg=%str(syscc=&syscc). Probed at this head: NOLIB.MPE_TABLES -> {'sasjsAbort':[{'MSG':'Invalid libds',...}]}. Either carry syscc=<n> on the unknown-library path, or say in the comment at 34-42 that the mock deliberately collapses "library unknown" into "libds malformed" - as written it claims estate parity for a message the estate reserves for malformed input.
sas/mocks/test/parity/scenarios.json:571 and :579 - "LIBDS": "NO_SUCH_LIB.MPE_TABLES" is 11 characters, over the 8-character libref cap, so the value is malformed and takes the Invalid libds abort with or without the new check: delete the knownLib branch and this golden still reproduces. Re-capture it with NOLIB.MPE_TABLES and the estate's real answer, or sas/mocks/test/read-paths.test.js:100 keeps measuring the shape check under a name that claims the existence check.
sas/mocks/sasjs/services/public/getcols.js:51 - const tableData = makeTableLoader(libDataDir(libref))(table) || null, justified by "a member missing from a library that does exist ... there is no message there worth copying" (lines 40-41). There is one: getcols.sas:66-68 aborts syscc=<n> when SYSCC is non-zero after %mp_getcols. A missing member in a real library therefore answers {"SYSCC":"0","cols":[]} here (probed: DC_JSLIB.NO_SUCH_TABLE) where the estate aborts, so the client draws an empty column picker instead of showing an error. Pre-existing, but the new comment now cements it - carry the abort, or drop the claim.
sas/mocks/test/harness.js:117 - /* Earlier sessions are left in place. Removing them pruned the estate, but a removes the session prune as a second, unrelated change inside a getcols PR, and the ENOENT race it explains has no test here. It reads like a reasonable harness fix; ship it with a test that exercises the async header write, or on its own, so this diff reads as one change.

CI at this head is still running - Build / Build-and-test-development and Build / lighthouse pending, Build / Build-and-ng-test success - so this is code review only.

4 findings above for review.

Re-review at d0b2ca26, superseding the review at f851bfaf. Delta: the head gains only a merge of release/7.23 - `git diff f851bfaf HEAD -- sas/` is empty, so the getcols change is byte-identical to the version reviewed before and nothing was closed. Re-ran the suite and the probes at this head: `node --test sas/mocks/test/*.test.js` is 131 pass, and the three probes below answer exactly as they did at f851bfaf. The four findings stand: - sas/mocks/sasjs/services/public/getcols.js:46 - `if (!libds || !table || !knownLib) {` sends a well-formed libref the estate does not have into `Invalid libds`. getcols.sas:42 guards that abort on `%mpe_validatecol(_libds,LIBDS,...)`, whose rule is shape-only (`/^[_a-z]\w{0,7}\.[_a-z]\w{0,31}$/i`, and @sasjs/core's ISLIB docstring says so: "checks if the value is a valid libref (NOT whether it exists)"). `NOLIB.MPE_TABLES` passes that shape check, reaches `%dc_assignlib`, fails there, and the caller sees the second abort at getcols.sas:66-68, `msg=%str(syscc=&syscc)`. Probed at this head: `NOLIB.MPE_TABLES` -> `{'sasjsAbort':[{'MSG':'Invalid libds',...}]}`. Either carry `syscc=<n>` on the unknown-library path, or say in the comment at 34-42 that the mock deliberately collapses "library unknown" into "libds malformed" - as written it claims estate parity for a message the estate reserves for malformed input. - sas/mocks/test/parity/scenarios.json:571 and :579 - `"LIBDS": "NO_SUCH_LIB.MPE_TABLES"` is 11 characters, over the 8-character libref cap, so the value is malformed and takes the `Invalid libds` abort with or without the new check: delete the `knownLib` branch and this golden still reproduces. Re-capture it with `NOLIB.MPE_TABLES` and the estate's real answer, or sas/mocks/test/read-paths.test.js:100 keeps measuring the shape check under a name that claims the existence check. - sas/mocks/sasjs/services/public/getcols.js:51 - `const tableData = makeTableLoader(libDataDir(libref))(table) || null`, justified by "a member missing from a library that does exist ... there is no message there worth copying" (lines 40-41). There is one: getcols.sas:66-68 aborts `syscc=<n>` when SYSCC is non-zero after `%mp_getcols`. A missing member in a real library therefore answers `{"SYSCC":"0","cols":[]}` here (probed: `DC_JSLIB.NO_SUCH_TABLE`) where the estate aborts, so the client draws an empty column picker instead of showing an error. Pre-existing, but the new comment now cements it - carry the abort, or drop the claim. - sas/mocks/test/harness.js:117 - `/* Earlier sessions are left in place. Removing them pruned the estate, but a` removes the session prune as a second, unrelated change inside a getcols PR, and the ENOENT race it explains has no test here. It reads like a reasonable harness fix; ship it with a test that exercises the async header write, or on its own, so this diff reads as one change. CI at this head is still running - Build / Build-and-test-development and Build / lighthouse pending, Build / Build-and-ng-test success - so this is code review only. 4 findings above for review.

hermes referenced this issue from a commit

2026-11-07 19:48:22 +00:00

fix(mocks): send the payload keys the client matches, and stop the flake

allan merged commit d6ac4ae2d5 into release/7.23

2026-11-07 20:14:52 +00:00

allan deleted branch fix/mock-getcols-libref

2026-11-07 20:14:53 +00:00

hermes referenced this pull request

2026-11-07 20:25:57 +00:00

fix(mocks): tell a malformed LIBDS from one getcols cannot resolve #515

Sign in to join this conversation.