test(embed): read the stored filter, by this run's hash, instead of wiping the tables #454

Merged

allan merged 14 commits from fix/449-va-ddc-host-order-free into release/7.22 2026-11-04 11:19:47 +00:00

Conversation 8 Commits 14 Files Changed 2

+192 -48

hermes commented 2026-11-03 20:53:56 +00:00

Collaborator

Copy link

Copy source

Why

The spec located its evidence as "the last stored row for this variable", which cannot tell this run's row from one an earlier case or run left behind. The answer was to delete the DC_JSLIB filter tables before each case - and that bought a scheduling contract: va-ddc-host had to run after every other spec on the shared mock estate, and it left the estate falsified for anything that ran after it. The sequencing was not a property of the thing under test; it was a workaround for how the evidence was found.

What

• The stored clause is now selected by hash, not by position. The mock public/validatefilter already returns the FILTER_HASH it computed for the clause, so cy.intercept captures it from this case's own response and the rows are read back by that hash. A clause that changed hashes differently, so a regression cannot be satisfied by a row an earlier run wrote; a store that already holds this clause holds the same rows, which is what makes a repeat run and a reused estate safe.
• resetFilterStore is gone, with the three DELETEs it issued. The spec now writes nothing shared and removes nothing.
• va-ddc-host moves back in front of metanav in cypress-specs.sh, restoring metanav as last - its licence key is the one real ordering constraint, because it must not change another spec. The header bullet that required va-ddc-host to be last is removed with it.

Verification

Against a local mock estate: @sasjs/server 3.12.2 with DDC_HOST_ENABLED=true, mock services deployed with sasjs cbd -t server-ci, makedata seeding the drive, and the client built and streamed as clickme.

Four consecutive npx cypress run --spec cypress/e2e/va-ddc-host.cy.ts runs, no wipe between them:

✓ 1 | stores a month-name date as a date literal
✓ 2 | stores a DATE9 date as a date literal (control)
✓ 3 | stores a character parameter as a quoted string (control)
3 passing (56s)     <- and 3 passing (5s) on each of the three repeats

The store after those runs holds three rows, two of them for SOME_DATE:

E05C5E72 SOME_DATE     '02AUG1971'd
2FC1F9D2 SOME_DATE     '01DEC2021'd
2A6DF30F SOME_DROPDOWN 'Option 2'

That is the state the wipe existed to prevent, and it is also what proves the selection: the last SOME_DATE row in the store is the DATE9 one, yet case 1 asserts '02AUG1971'd and passes. A variable-only read would have taken the DATE9 row and failed.

bash .gitea/scripts/cypress-specs.sh coverage . -> All 25 specs are accounted for: 25 run, 0 excluded., and the all list now ends download, va-ddc-host, metanav. npx prettier --check on the spec is clean.

Not changed here: the mock service itself. It already returns FILTER_HASH, so nothing needed adding to it.

Note on the CI pin: release/7.22 carries 3.12.2 for the mocked estate, so this branch is cut from there; timing=early needs 3.12.0 or later either way.

## Why The spec located its evidence as "the last stored row for this variable", which cannot tell this run's row from one an earlier case or run left behind. The answer was to delete the `DC_JSLIB` filter tables before each case - and that bought a scheduling contract: `va-ddc-host` had to run after every other spec on the shared mock estate, and it left the estate falsified for anything that ran after it. The sequencing was not a property of the thing under test; it was a workaround for how the evidence was found. ## What - **The stored clause is now selected by hash, not by position.** The mock `public/validatefilter` already returns the `FILTER_HASH` it computed for the clause, so `cy.intercept` captures it from this case's own response and the rows are read back by that hash. A clause that changed hashes differently, so a regression cannot be satisfied by a row an earlier run wrote; a store that already holds this clause holds the same rows, which is what makes a repeat run and a reused estate safe. - **`resetFilterStore` is gone**, with the three `DELETE`s it issued. The spec now writes nothing shared and removes nothing. - **`va-ddc-host` moves back in front of `metanav`** in `cypress-specs.sh`, restoring `metanav` as last - its licence key is the one real ordering constraint, because it must not change another spec. The header bullet that required `va-ddc-host` to be last is removed with it. ## Verification Against a local mock estate: `@sasjs/server` 3.12.2 with `DDC_HOST_ENABLED=true`, mock services deployed with `sasjs cbd -t server-ci`, `makedata` seeding the drive, and the client built and streamed as `clickme`. Four consecutive `npx cypress run --spec cypress/e2e/va-ddc-host.cy.ts` runs, **no wipe between them**: ``` ✓ 1 | stores a month-name date as a date literal ✓ 2 | stores a DATE9 date as a date literal (control) ✓ 3 | stores a character parameter as a quoted string (control) 3 passing (56s) <- and 3 passing (5s) on each of the three repeats ``` The store after those runs holds three rows, two of them for `SOME_DATE`: ``` E05C5E72 SOME_DATE '02AUG1971'd 2FC1F9D2 SOME_DATE '01DEC2021'd 2A6DF30F SOME_DROPDOWN 'Option 2' ``` That is the state the wipe existed to prevent, and it is also what proves the selection: the last `SOME_DATE` row in the store is the DATE9 one, yet case 1 asserts `'02AUG1971'd` and passes. A variable-only read would have taken the DATE9 row and failed. `bash .gitea/scripts/cypress-specs.sh coverage .` -> `All 25 specs are accounted for: 25 run, 0 excluded.`, and the `all` list now ends `download, va-ddc-host, metanav`. `npx prettier --check` on the spec is clean. Not changed here: the mock service itself. It already returns `FILTER_HASH`, so nothing needed adding to it. Note on the CI pin: `release/7.22` carries 3.12.2 for the mocked estate, so this branch is cut from there; `timing=early` needs 3.12.0 or later either way.

hermes added 1 commit 2026-11-03 20:53:56 +00:00

test(embed): find the stored clause by this run's filter hash, without wiping ... e6881a2900

The spec located its evidence as "the last stored row for this variable", which
cannot tell this run's row from one an earlier case or run left behind - so it
deleted the DC_JSLIB filter tables before each case instead, and that bought a
scheduling contract: it had to run after every other spec on the shared mock
estate, and it left the estate falsified for anything that ran after it.

The mock validatefilter already returns the FILTER_HASH it computed for the
clause, so the rows are now selected by the hash this case's own response names.
Nothing is cleared, and no spec has to run before or after it. A store that
already holds this clause holds the same rows; a clause that changed hashes
differently, so a regression cannot be satisfied by a stale row.

va-ddc-host moves back in front of metanav, which is the one spec with a real
ordering constraint (its licence key must not change another spec).

hermes added 1 commit 2026-11-03 22:24:56 +00:00

test(embed): scope the captured hash to this case, and fail on its own terms ... 8ec4d4e61d

The intercept kept one hash from any validatefilter response in the case, so a
second call - another table, a revalidate on load - would have replaced it and
the rows read back would belong to a different clause. The response is now only
taken when the request body carries this case's variable, which is what tells one
clause from another, and a case that sees more than one call for that variable
fails on the count rather than silently keeping the last.

A hash that cannot be read is captured and rethrown in the command chain: thrown
inside the route handler, Cypress reports it as an uncaught error and the case
fails later as a bare "no clause under" timeout instead of naming the response.

hermes added 1 commit 2026-11-03 22:27:34 +00:00

chore: merge release/7.22 into fix/449-va-ddc-host-order-free 28458539f9

hermes commented 2026-11-03 22:27:43 +00:00

Author

Collaborator

Copy link

Copy source

Both are right, fixed in 8ec4d4e.

Which response is this case's (line 190). The matcher now decides per request, not per URL: the app posts the plugin tables as multipart, so the body carries the filterquery CSV and the variable name is in it.

Content-Disposition: form-data; name="filterquery"; filename="filterquery.csv"
GROUP_LOGIC:$char3. ... VARIABLE_NM:$char13. OPERATOR_NM:$char1. RAW_VALUE:$char10.
AND,AND,0,SOME_DROPDOWN,=,"'Option 2'"

A response is only taken when the body names this case's variable, so a second call for another table or another clause cannot overwrite the hash. A second call carrying the same variable would be the same clause - but it would also mean the harness delivered twice, which the case should not paper over, so the calls for that variable are counted and the case asserts there is exactly one:

expect(capture.calls, `validatefilter calls carrying ${variable} in this case`).to.equal(1)

A hash that cannot be read (line 120). The parse/hash failure is now stored in capture.error and rethrown from the command chain, so it fails the case with its own message. Verified by forcing it - with the real wiring in place but a body that is not JSON, the case reports:

Error: the validatefilter response for SOME_DATE was not JSON: not json

rather than an uncaught Cypress error followed by a 15-second "no clause under" timeout.

Re-verified after merging release/7.22 for #453 (the mock services' webout() change): two runs, 3 passing each, and the store holding both SOME_DATE hashes at the same time - '02AUG1971'd and '01DEC2021'd - which is the state the old variable-only read would have confused. npx prettier --check clean.

Both are right, fixed in `8ec4d4e`. **Which response is this case's (line 190).** The matcher now decides per request, not per URL: the app posts the plugin tables as multipart, so the body carries the `filterquery` CSV and the variable name is in it. ``` Content-Disposition: form-data; name="filterquery"; filename="filterquery.csv" GROUP_LOGIC:$char3. ... VARIABLE_NM:$char13. OPERATOR_NM:$char1. RAW_VALUE:$char10. AND,AND,0,SOME_DROPDOWN,=,"'Option 2'" ``` A response is only taken when the body names this case's variable, so a second call for another table or another clause cannot overwrite the hash. A second call *carrying the same variable* would be the same clause - but it would also mean the harness delivered twice, which the case should not paper over, so the calls for that variable are counted and the case asserts there is exactly one: ``` expect(capture.calls, `validatefilter calls carrying ${variable} in this case`).to.equal(1) ``` **A hash that cannot be read (line 120).** The parse/hash failure is now stored in `capture.error` and rethrown from the command chain, so it fails the case with its own message. Verified by forcing it - with the real wiring in place but a body that is not JSON, the case reports: ``` Error: the validatefilter response for SOME_DATE was not JSON: not json ``` rather than an uncaught Cypress error followed by a 15-second "no clause under" timeout. Re-verified after merging `release/7.22` for #453 (the mock services' `webout()` change): two runs, 3 passing each, and the store holding both `SOME_DATE` hashes at the same time - `'02AUG1971'd` and `'01DEC2021'd` - which is the state the old variable-only read would have confused. `npx prettier --check` clean.

allan added 1 commit 2026-11-03 23:07:16 +00:00

Merge branch 'release/7.22' into fix/449-va-ddc-host-order-free 74d78ffe57

hermes added 1 commit 2026-11-03 23:15:57 +00:00

test(embed): match a parsed request body, and count duplicates after the read ... 93af2dabe6

Two gaps in the guard that decides which validatefilter response belongs to the
case.

A body Cypress parses into an object stringified to "[object Object]", so no
call matched and the case failed on the count with nothing naming the body shape.
The body is now serialised the way the response already was - a string is used as
it is, anything else is JSON.stringify'd.

The count was asserted immediately after the first response, which is before a
second POST from the same apply could reach its own callback, so a duplicate read
as one call. The count is now asserted after the stored row has been read back,
with a settle that covers a second POST still in flight when the read returned.

hermes commented 2026-11-03 23:16:07 +00:00

Author

Collaborator

Copy link

Copy source

Both are right, fixed in 93af2da.

A parsed body read as "[object Object]". The request body now goes through the same defensive serialisation the response already used - a string is taken as it is, anything else is JSON.stringify'd:

const requestText = (body: any): string =>
  typeof body === 'string' ? body : JSON.stringify(body ?? '')

The duplicate guard could not be relied on. The count is now asserted after the stored row has been read back rather than straight after the first response, with a settle covering a second POST that was still in flight when the read returned:

.then((clause: StoredClause) =>
  cy.wait(SETTLE_MS).then(() => {
    expect(capture.calls, `validatefilter calls carrying ${variable} in this case`).to.equal(1)
    return clause
  })
)

Both fixes proven together, because one probe exercises both: it fires a duplicate validatefilter POST from the page with a JSON object body rather than the app's multipart, after the first response has landed.

win.fetch(`${serverUrl}/SASjsApi/stp/validatefilter`, {
  method: 'POST',
  headers: { 'Content-Type': 'application/json' },
  body: JSON.stringify({ tables: { filterquery: { VARIABLE_NM: variable } } })
})

Result, on all three cases:

validatefilter calls carrying SOME_DATE in this case
+ expected - actual

-2
+1

Two calls counted, so the object body matched the variable (finding 1) and the duplicate that arrived after the first response was seen (finding 2). Before this commit the same probe would have left calls at 1 and passed.

Unchanged spec, two runs after the change: 3 passing each. npx prettier --check clean. The branch also re-merges release/7.22 at 74d78ff, which now carries #455 (cancelling superseded runs), so this PR's own older runs no longer queue behind it.

Both are right, fixed in `93af2da`. **A parsed body read as "[object Object]".** The request body now goes through the same defensive serialisation the response already used - a string is taken as it is, anything else is `JSON.stringify`'d: ```ts const requestText = (body: any): string => typeof body === 'string' ? body : JSON.stringify(body ?? '') ``` **The duplicate guard could not be relied on.** The count is now asserted after the stored row has been read back rather than straight after the first response, with a settle covering a second POST that was still in flight when the read returned: ```ts .then((clause: StoredClause) => cy.wait(SETTLE_MS).then(() => { expect(capture.calls, `validatefilter calls carrying ${variable} in this case`).to.equal(1) return clause }) ) ``` Both fixes proven together, because one probe exercises both: it fires a duplicate `validatefilter` POST from the page with a **JSON object** body rather than the app's multipart, after the first response has landed. ```js win.fetch(`${serverUrl}/SASjsApi/stp/validatefilter`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ tables: { filterquery: { VARIABLE_NM: variable } } }) }) ``` Result, on all three cases: ``` validatefilter calls carrying SOME_DATE in this case + expected - actual -2 +1 ``` Two calls counted, so the object body matched the variable (finding 1) and the duplicate that arrived after the first response was seen (finding 2). Before this commit the same probe would have left `calls` at 1 and passed. Unchanged spec, two runs after the change: 3 passing each. `npx prettier --check` clean. The branch also re-merges `release/7.22` at `74d78ff`, which now carries #455 (cancelling superseded runs), so this PR's own older runs no longer queue behind it.

hermes added 1 commit 2026-11-03 23:16:43 +00:00

chore: merge release/7.22 into fix/449-va-ddc-host-order-free 6508ac7f5c

hermes added 1 commit 2026-11-03 23:54:51 +00:00

test(embed): match the clause's field, and wait for the response as long as the page takes ... 9c53e5b90d

The matcher tested the variable as a substring of the whole request body, so a
RAW_VALUE, an SQL fragment or another variable whose name contains it was counted
as another delivery of this clause and failed the count. It now requires the name
between its CSV delimiters, which is the filterquery VARIABLE_NM field; a quoted
literal cannot match, because its delimiters are quotes.

cy.wait('@validatefilter') used the 5s default, where the polling loop it replaced
tolerated 15s, and the message only arrives once the framed page has rendered.
It now waits for longerCommandTimeout, as the rest of this spec does.

The capture assertion also carries the tail of the host page's log in its message:
the failure reads "expected -1 to be above -1" otherwise, and the log is what says
whether the app never exposed its buffer or the frame never loaded.

hermes commented 2026-11-03 23:54:57 +00:00

Author

Collaborator

Copy link

Copy source

Both fixed in 9c53e5b.

Substring match. The variable is now matched between its CSV delimiters, which is where VARIABLE_NM sits in the filterquery row, rather than as a substring of the whole payload:

const escaped = variable.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
return new RegExp(`(^|[,\\r\\n])${escaped}(,|$)`, 'm').test(requestText(body))

A quoted literal cannot match, because its delimiters are quotes, not commas.

Checked on the function text extracted from the spec (not a re-typed copy), seven cases:

ok   multipart body, real clause row -> true
ok   parsed object carrying the CSV row -> true
ok   literal that only mentions the name -> false
ok   a longer variable name (SOME_DATE_EXTRA) -> false
ok   the name inside a quoted value -> false
ok   another variable entirely -> false
ok   row at the very start of the text -> true

Wait timeout. cy.wait('@validatefilter', { timeout: longerCommandTimeout }), so it tolerates the same 50 s the rest of the spec does instead of the 5 s default.

End to end after both changes: three consecutive runs of the spec against the local estate, 3 passing each.

One thing worth recording, since it will otherwise show up as an unexplained red in CI: while testing this I hit an intermittent failure of the capture assertion - the app captured the message: expected -1 to be above -1 - on runs where the box was busy, and it did not reproduce on the three runs above. It is the assertion that waits for the host page's early replay: the app captured the message line, and that page gives up after 30 s, so a slow first render can produce it. It predates this PR (the original spec fails the same way on a loaded box). Rather than leave it opaque, that assertion now carries the tail of the page's log in its message, so a failure says whether the app never exposed its buffer or the frame never loaded. I have not otherwise changed the capture path.

Both fixed in `9c53e5b`. **Substring match.** The variable is now matched between its CSV delimiters, which is where `VARIABLE_NM` sits in the `filterquery` row, rather than as a substring of the whole payload: ```ts const escaped = variable.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') return new RegExp(`(^|[,\\r\\n])${escaped}(,|$)`, 'm').test(requestText(body)) ``` A quoted literal cannot match, because its delimiters are quotes, not commas. Checked on the function text extracted from the spec (not a re-typed copy), seven cases: ``` ok multipart body, real clause row -> true ok parsed object carrying the CSV row -> true ok literal that only mentions the name -> false ok a longer variable name (SOME_DATE_EXTRA) -> false ok the name inside a quoted value -> false ok another variable entirely -> false ok row at the very start of the text -> true ``` **Wait timeout.** `cy.wait('@validatefilter', { timeout: longerCommandTimeout })`, so it tolerates the same 50 s the rest of the spec does instead of the 5 s default. End to end after both changes: three consecutive runs of the spec against the local estate, 3 passing each. One thing worth recording, since it will otherwise show up as an unexplained red in CI: while testing this I hit an **intermittent** failure of the capture assertion - `the app captured the message: expected -1 to be above -1` - on runs where the box was busy, and it did not reproduce on the three runs above. It is the assertion that waits for the host page's `early replay: the app captured the message` line, and that page gives up after 30 s, so a slow first render can produce it. It predates this PR (the original spec fails the same way on a loaded box). Rather than leave it opaque, that assertion now carries the tail of the page's log in its message, so a failure says whether the app never exposed its buffer or the frame never loaded. I have not otherwise changed the capture path.

hermes added 1 commit 2026-11-04 00:40:50 +00:00

test(embed): match the clause's field wherever the body wraps it ... 1c4d2f65cc

Review finding: the matcher only accepted the variable name at the start of the
body or after a comma or newline, so a delivery wrapped any other way was not
counted - and the case then failed at the call-count assertion, reading as
"the app never called validatefilter". Reproduced against the previous
implementation: a urlencoded form value, a form value that starts with the
name, a JSON object keyed on the column and a JSON string value holding the
row all matched nothing.

The body is now expanded into the candidate strings it can carry - the raw
text, the percent-decoded text, each form field's value, and every string leaf
of a JSON body - and the name has to be a whole CSV field of one of them.
Field equality rather than delimiters means a quoted literal, a longer name,
an SQL fragment or another variable's name is still refused, and there is no
regular expression over user text to get wrong.

Also restores the ordering note in cypress-specs.sh: va-ddc-host asserts
nothing about licensing, so it runs before metanav, the one spec that plants a
licence key.

hermes commented 2026-11-04 00:40:57 +00:00

Author

Collaborator

Copy link

Copy source

Both in 1c4d2f6.

1. carriesClause and wrapped bodies - agreed, and reproduced against the previous implementation before changing it. Four shapes matched nothing, which is the false failure you describe:

FAIL urlencoded form value carrying the row -> false (want true)
FAIL form value that starts with the name   -> false (want true)
FAIL JSON object keyed on the column        -> false (want true)
FAIL JSON string value that is the CSV row  -> false (want true)
ok   multipart part carrying the CSV row    -> true

I did not widen the delimiter class, because the quotes and spaces it would have to accept are exactly what lets a quoted literal through - the property the matcher was written for. Instead the body is expanded into the strings it can carry (the raw text, the percent-decoded text, each form field's value after =, and every string leaf of a JSON body), and the name has to be a whole CSV field of one of them:

const csvFields = (text: string): string[] => text.split(/[,\r\n]/)
const carriesClause = (body, variable) =>
  bodyTexts(body).some((text) => csvFields(text).some((field) => field.trim() === variable))

That is field equality rather than a pattern over user text, so there is no escaping to get wrong. Ten cases against the function text extracted from the spec (only the type annotations stripped), all as intended: multipart, urlencoded, a form value that starts with the name, a JSON object keyed on the column, a JSON string value holding the row, and a row at the very start - true; a quoted literal, the name inside a quoted value, a longer name (SOME_DROPDOWN_EXTRA) and another variable - false.

2. The ordering note - agreed, restored. cypress-specs.sh now says: va-ddc-host asserts nothing about licensing and depends on the estate being unlicensed, so it runs before metanav, the one spec that plants a key.

npx prettier --check clean on the spec, npx esbuild transpiles it, bash -n on the script. I have not re-run the Cypress spec itself against an estate for this change - the assertion it fixes is about the body shapes, which the extracted-function cases cover directly, and the three-run evidence from 9c53e5b still applies to the rest of the case.

• agent001

Both in 1c4d2f6. 1. `carriesClause` and wrapped bodies - agreed, and reproduced against the previous implementation before changing it. Four shapes matched nothing, which is the false failure you describe: ``` FAIL urlencoded form value carrying the row -> false (want true) FAIL form value that starts with the name -> false (want true) FAIL JSON object keyed on the column -> false (want true) FAIL JSON string value that is the CSV row -> false (want true) ok multipart part carrying the CSV row -> true ``` I did not widen the delimiter class, because the quotes and spaces it would have to accept are exactly what lets a quoted literal through - the property the matcher was written for. Instead the body is expanded into the strings it can carry (the raw text, the percent-decoded text, each form field's value after `=`, and every string leaf of a JSON body), and the name has to be a whole CSV field of one of them: ```ts const csvFields = (text: string): string[] => text.split(/[,\r\n]/) const carriesClause = (body, variable) => bodyTexts(body).some((text) => csvFields(text).some((field) => field.trim() === variable)) ``` That is field equality rather than a pattern over user text, so there is no escaping to get wrong. Ten cases against the function text extracted from the spec (only the type annotations stripped), all as intended: multipart, urlencoded, a form value that starts with the name, a JSON object keyed on the column, a JSON string value holding the row, and a row at the very start - true; a quoted literal, the name inside a quoted value, a longer name (`SOME_DROPDOWN_EXTRA`) and another variable - false. 2. The ordering note - agreed, restored. `cypress-specs.sh` now says: `va-ddc-host` asserts nothing about licensing and depends on the estate being unlicensed, so it runs before `metanav`, the one spec that plants a key. `npx prettier --check` clean on the spec, `npx esbuild` transpiles it, `bash -n` on the script. I have not re-run the Cypress spec itself against an estate for this change - the assertion it fixes is about the body shapes, which the extracted-function cases cover directly, and the three-run evidence from 9c53e5b still applies to the rest of the case. - agent001

hermes added 2 commits 2026-11-04 01:20:18 +00:00

test(embed): count deliveries on the app's own signal, not on a clock ... 4b42d1aaf1

The duplicate check waited a fixed 250ms after reading the stored row, which is
wrong in both directions: on a slow runner the window can elapse before the first
response callback has incremented the counter, and a second POST landing after it
passes with two deliveries.

The app's sequence for an apply is validatefilter, then the data load for the
filtered route, so the load is the completion signal: the count is asserted after
it, and nothing depends on how fast the machine is. A delivery issued later than
that is a second apply, not a duplicate of this one.

Also corrects the ordering paragraph in cypress-specs.sh: it still said specs are
appended, in a file that now places va-ddc-host ahead of metanav. va-ddc-host does
not depend on the estate being unlicensed either - it passed 3/3 immediately after
metanav planted a key in the full CI run - so the bullet says what is true and
metanav stays the only spec with a fixed place.

chore: merge release/7.22 into fix/449-va-ddc-host-order-free 4a87f8f6ce

hermes commented 2026-11-04 01:20:27 +00:00

Author

Collaborator

Copy link

Copy source

Both in 4b42d1a.

The clock. Right in both directions - a fixed 250 ms can expire before the first callback on a slow runner, and can be outlived by a second POST. The count is now asserted on the app's own completion signal instead. An apply's request sequence is validatefilter, then the data load for the filtered route, so getdata is awaited and the count is checked after it:

cy.intercept({ method: 'POST', url: '**/*getdata*' }).as('getdata')
...
.then((clause: StoredClause) =>
  cy.wait('@getdata', { timeout: longerCommandTimeout }).then(() => {
    expect(capture.calls, `validatefilter calls carrying ${variable} in this case`).to.equal(1)
    return clause
  })
)

Nothing depends on elapsed time, and the boundary is now a fact about the app rather than a number: a POST issued before the load is a duplicate of this apply and is counted; one issued after it is a second apply. SETTLE_MS is gone.

Two runs of the spec on the local estate after the change - 3 passing each, so getdata is being observed inside the timeout rather than the assertion passing by luck.

The stale paragraph (cypress-specs.sh:17). Corrected - and the bullet above it was wrong in the same way, so both moved: va-ddc-host does not depend on the estate being unlicensed. In the full CI run at 17:38 it ran immediately after metanav planted its key and passed 3/3, which is the evidence that it works on either side. It now sits before metanav so that metanav keeps the only fixed place, and the paragraph says that rather than claiming an append-only order the list no longer follows:

#   - va-ddc-host asserts nothing about licensing, so it works on either side of
#     metanav; it sits before it, which leaves metanav last.
#
# Specs added later are appended rather than inserted, so the sequence that was
# known green keeps its shape. metanav is the exception - it has to stay last for
# the key it plants, so a new spec goes in front of it.

npx prettier --check clean on the spec, bash -n clean on the script. The branch also re-merges release/7.22 at 514e326.

Both in `4b42d1a`. **The clock.** Right in both directions - a fixed 250 ms can expire before the first callback on a slow runner, and can be outlived by a second POST. The count is now asserted on the app's own completion signal instead. An apply's request sequence is `validatefilter`, then the data load for the filtered route, so `getdata` is awaited and the count is checked after it: ```ts cy.intercept({ method: 'POST', url: '**/*getdata*' }).as('getdata') ... .then((clause: StoredClause) => cy.wait('@getdata', { timeout: longerCommandTimeout }).then(() => { expect(capture.calls, `validatefilter calls carrying ${variable} in this case`).to.equal(1) return clause }) ) ``` Nothing depends on elapsed time, and the boundary is now a fact about the app rather than a number: a POST issued before the load is a duplicate of this apply and is counted; one issued after it is a second apply. `SETTLE_MS` is gone. Two runs of the spec on the local estate after the change - 3 passing each, so `getdata` is being observed inside the timeout rather than the assertion passing by luck. **The stale paragraph (cypress-specs.sh:17).** Corrected - and the bullet above it was wrong in the same way, so both moved: `va-ddc-host` does not depend on the estate being unlicensed. In the full CI run at 17:38 it ran immediately after `metanav` planted its key and passed 3/3, which is the evidence that it works on either side. It now sits before `metanav` so that `metanav` keeps the only fixed place, and the paragraph says that rather than claiming an append-only order the list no longer follows: ``` # - va-ddc-host asserts nothing about licensing, so it works on either side of # metanav; it sits before it, which leaves metanav last. # # Specs added later are appended rather than inserted, so the sequence that was # known green keeps its shape. metanav is the exception - it has to stay last for # the key it plants, so a new spec goes in front of it. ``` `npx prettier --check` clean on the spec, `bash -n` clean on the script. The branch also re-merges `release/7.22` at `514e326`.

hermes added 1 commit 2026-11-04 09:08:59 +00:00

test(embed): fail on a missing FILTER_HASH before reading the store back ... 1225eb122a

Review finding: when every body is refused by carriesClause, or the response
carries no FILTER_HASH, capture.hash stays empty - and storedClauseFor('')
then spends its 60 retries before reporting "no clause for <var> under",
which names neither the refused body nor the missing hash.

The hash is now asserted non-empty before the read, with the case's own
message, so a broken delivery fails where it happened.

The call count stays where it was - after the data load - because moving it
ahead of the read would check it before the app has issued the second POST
that the count exists to catch.

hermes commented 2026-11-04 09:50:29 +00:00

Author

Collaborator

Copy link

Copy source

Both points taken.

1. A missing FILTER_HASH - fixed in 1225eb1. The hash is now asserted non-empty (validatefilter FILTER_HASH for this case) before storedClauseFor, so a body refused by carriesClause, or a response with no FILTER_HASH, fails at the delivery instead of as a missing row after 60 retries. I left the calls === 1 count where it is, after the @getdata wait: moving it ahead of the read would check it before the app has issued the second POST the count exists to catch. Happy to move it if you would rather have the earlier report.

2. The pipeline failure is not from this PR. licensing.cy.ts fails 3 of its 10 tests (3 expired, 5 wrong organisation, 6 paste HTTP key) on release/7.22 too - runs 3038, 3057 and 3059 all carry the same three, and I reproduced the same three locally on a freshly seeded mock estate under Chromium.

Root cause: startupservice.SYSSITE is now empty. The JS stored programs take their envelope from the server's webout() prelude, which writes "SYSSITE":""; %webout writes &syssite. Before the mocks moved onto webout() the hand-rolled envelope in dcMockUtils.js supplied '123'.

client/src/app/services/licence.service.ts routes an expired key only if (variables && startup_site_id) (line 254), and the site-id mismatch check needs the same value. With SYSSITE empty an expired key is still decoded - the page shows "valid until 2020-10-03" - but is never routed, so the banner stays on "missing". That is test 3, and it drags 5 and 6 with it.

Proof: patching only the prelude's SYSSITE literal to '123', with nothing else changed, makes all three pass under Chromium; with '' they fail 3/10, retries included.

The fix belongs in its own change, not in this PR.

Both points taken. **1. A missing FILTER_HASH** - fixed in 1225eb1. The hash is now asserted non-empty (`validatefilter FILTER_HASH for this case`) before `storedClauseFor`, so a body refused by `carriesClause`, or a response with no FILTER_HASH, fails at the delivery instead of as a missing row after 60 retries. I left the `calls === 1` count where it is, after the `@getdata` wait: moving it ahead of the read would check it before the app has issued the second POST the count exists to catch. Happy to move it if you would rather have the earlier report. **2. The pipeline failure is not from this PR.** `licensing.cy.ts` fails 3 of its 10 tests (3 expired, 5 wrong organisation, 6 paste HTTP key) on release/7.22 too - runs 3038, 3057 and 3059 all carry the same three, and I reproduced the same three locally on a freshly seeded mock estate under Chromium. Root cause: `startupservice.SYSSITE` is now empty. The JS stored programs take their envelope from the server's `webout()` prelude, which writes `"SYSSITE":""`; `%webout` writes `&syssite`. Before the mocks moved onto `webout()` the hand-rolled envelope in `dcMockUtils.js` supplied `'123'`. `client/src/app/services/licence.service.ts` routes an expired key only `if (variables && startup_site_id)` (line 254), and the site-id mismatch check needs the same value. With `SYSSITE` empty an expired key is still decoded - the page shows "valid until 2020-10-03" - but is never routed, so the banner stays on "missing". That is test 3, and it drags 5 and 6 with it. Proof: patching only the prelude's `SYSSITE` literal to `'123'`, with nothing else changed, makes all three pass under Chromium; with `''` they fail 3/10, retries included. The fix belongs in its own change, not in this PR.

hermes added 1 commit 2026-11-04 10:20:26 +00:00

Merge remote-tracking branch 'origin/release/7.22' into fix/449-va-ddc-host-order-free aa295b384a

hermes commented 2026-11-04 10:20:37 +00:00

Author

Collaborator

Copy link

Copy source

Follow-up on the red pipeline: the fix is the server's SYSSITE support, now in @sasjs/server 3.13.0 (@sasjs/utils 4.7 reports it, defaulting to 123).

The pin is bumped to 3.13.0 in ba32d1f (also on release/7.22, so #407 carries it), and this branch has merged it in aa295b3.

Verified locally before pushing: with 3.13.0 and no other change, startupservice returns SYSSITE "123" and licensing.cy.ts passes 10/10 under Chromium. With 3.12.3 it fails the same three cases CI reported (3, 5, 6).

Follow-up on the red pipeline: the fix is the server's SYSSITE support, now in @sasjs/server 3.13.0 (@sasjs/utils 4.7 reports it, defaulting to 123). The pin is bumped to 3.13.0 in ba32d1f (also on release/7.22, so #407 carries it), and this branch has merged it in aa295b3. Verified locally before pushing: with 3.13.0 and no other change, `startupservice` returns `SYSSITE` "123" and licensing.cy.ts passes 10/10 under Chromium. With 3.12.3 it fails the same three cases CI reported (3, 5, 6).

allan added 1 commit 2026-11-04 10:39:55 +00:00

Merge branch 'release/7.22' into fix/449-va-ddc-host-order-free 1a0d371fb0

allan added 1 commit 2026-11-04 10:40:16 +00:00

Merge branch 'release/7.22' into fix/449-va-ddc-host-order-free 3d94c499d9

hermes reviewed 2026-11-04 10:52:26 +00:00

hermes left a comment

Author

Collaborator

Copy link

Copy source

REVIEWED ON 2026-11-04 10:52 UTC

Re-reviewed the full current diff (test harness and spec-order changes).

The store is now read by the FILTER_HASH the service returns for this run's own clause instead of being emptied first, and the duplicate-delivery check is taken after the data load rather than on a timer, so the spec no longer carries a position dependency. The cypress-specs.sh ordering comment and the spec order were updated consistently with that.

No blocking issues found.

REVIEWED ON 2026-11-04 10:52 UTC Re-reviewed the full current diff (test harness and spec-order changes). The store is now read by the FILTER_HASH the service returns for this run's own clause instead of being emptied first, and the duplicate-delivery check is taken after the data load rather than on a timer, so the spec no longer carries a position dependency. The cypress-specs.sh ordering comment and the spec order were updated consistently with that. No blocking issues found.

allan merged commit 2b85341845 into release/7.22 2026-11-04 11:19:47 +00:00

allan deleted branch fix/449-va-ddc-host-order-free 2026-11-04 11:19:47 +00:00

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

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dc/dc#454