Release/7.20 #387
Assignees
allan (Allan)
dcbot
DylanLeong
henrik
hermes (hermes)
mihajlo
sabir
sead
trevor
Yury
Clear assignees
allan (Allan)
dcbot
DylanLeong
henrik
hermes (hermes)
mihajlo
sabir
sead
trevor
Yury
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: dc/dc#387
Reference in new issue
Block a user
Data Controller 7.20
31 commits, 23 files, +3117 -83. No database changes, no new options, no migrations: nothing to run after deploying.
VA embed: map report parameters to columns from the URL
A report object can now say which DC column each VA parameter filters, instead of relying on the parameter's name matching the column:
Repeat the parameter for more mappings. This is what lets two report tabs filter the same column differently, from two parameters of their own - the case the name match cannot express.
-, so a label may contain hyphens;Deriving dates from a parameter value
A mapping can resolve the value through a named anchor before the filter is built, so one parameter can bring in "this date plus the previous quarter":
ADD(<anchor>,<offset>)yields the value plus one anchor beside it (IN (...));RANGE(<period>,<offset>)yields one closed period (BETWEEN ... AND ...). Anchors:QUARTER_START,QUARTER_END,MONTH_START,MONTH_END,YEAR_START,YEAR_END. Periods:QUARTER,MONTH,YEAR. The arithmetic is anchored on the parameter's value, never on today, so a saved report selects the same rows whenever it is opened.Every transform compiles to exactly one filter clause, so the AND rule, the query signature, dedup and the reload loop are untouched.
The mistakes are visible instead of silent
A wrong mapping used to look like a working filter over the whole table. The editor now shows a banner above the grid naming the parameter, the column and the fix, one line per cause: column not in the table, transform on a non-temporal column, a parameter VA never sends, two parameters AND-ed on one column with different values, a range sent several values, part of a multi-value parameter that is not a date, and malformed entries (missing
-, no parameter name, a parameter mapped twice). Console noise is deduplicated to one line per cause.Embedded objects no longer offer the deploy wizard
A DC object inside a report ran the first-time configuration wizard, which offered to deploy the application from a report object - something a DDC object cannot do. It now says what is wrong instead. The standalone URL is also defined once rather than in two places.
A successful deployment no longer reports as a failure
makedataended with a single-statement%elsein open code, which SAS rejects withERROR: Expected %DO not found/Skipping to next %END statement. On Viya the job then reportedstate: errorwithjobConditionCode 1012after the deployment had already completed, so a healthy first install looked broken. Both branches use%do/%endnow, in the Viya and server variants.CI
test:tz-west,test:tz-east).Testing
embed-va.cy.ts, including the painted-box geometry of the banner and the red-first console-once case;TZ=America/New_YorkandTZ=Pacific/Auckland(the round-trip of a bare SAS day number is what those pin);prettier --checkwith the exact CI glob, and a clean production build;deploy.component.spec.ts.No
@sasjs/corechange in this release.Docs
dc/docs.datacontroller.io#26 -
va_parmap, the banner causes, the date anchors, and a configuration walkthrough for the VA Report Designer. Ready to merge with the tag.Known gaps
makedatafix (#389) is not yet verified on a live estate: the demo tenant has been answering 503, so the reseed that would show a clean job is still queued. The change cannot affect a deployed application - it only changes the parse of the last block, after the deploy has finished;Reviewed the full base...head diff: three Gitea workflows - new docs.yaml (typedoc and SAS docs deploy), new release-assets.yaml, and release.yaml dropping the inline Surfer 6 deploys for a docs dispatch.
.assets[]?.id), not only the ones this workflow uploaded as the comment claims. A hand-attached or third-party asset disappears on the next run. Delete only the names in ASSETS/SHA256SUMS, or record the ids this run creates.npm i -g @sasjs/cliis unpinned, in the same files that pin @cloudron/surfer exactly for supply-chain reasons. A global CLI installed at release time is the larger surface of the two; pin it to a version.cd sas && npm idoes not enforce a lockfile the way the typedoc job'snpm cidoes. Usenpm ciif sas/ carries a lock, or state why not.echo "$NPMRC" > client/.npmrcreplaces the committed client/.npmrc, so any hardening flags it carries (ignore-scripts, save-exact) are silently dropped in these jobs. Append the secret rather than overwriting the file.releases/latestrather than the tag this job just cut, so two overlapping releases dispatch docs for whichever tag is latest. Take the tag from the semantic-release output (or gitea.ref_name) instead.--tokenbefore and is now passed as--passwordto Surfer 7. Confirm the secret now holds the Cloudron app password, and likewise WEBDOC_DATACONTROLLER_IO, otherwise the first release after merge fails at the docs dispatch.7 findings above for review.
A `va_parmap` entry that cannot be used was dropped in silence, in both places it can fail: at parse time (no `-` separator, a missing side, or a column that is not a plain SAS name) and at apply time (the mapped column is not in the table being opened). Either way the parameter is skipped, which degrades to "no filter" - a typo in the report's URL then looks exactly like a working filter over the whole table. Both are now reported in terms the report author can act on: - a warning banner in the editor (VA mode) naming the parameter, the column and the table, and pointing at the `va_parmap` entries in the URL - console.warn for each distinct problem, so it is findable from the report's console too The parser returns `{ map, problems }` rather than a bare Map, VaFilterService gains `unresolvedMappings()` for the entries the loaded table cannot satisfy, and the editor merges both sets for the banner (logging once per distinct set rather than once per VA message). The banner is covered by two e2e cases, one asserting its painted box inside the viewport (VA hides the app header, so it has to stand on its own) and one covering a parse-time failure before any VA message arrives. The alert markup keeps the message inside the alert's text flow so it wraps, instead of being laid out beside a title and pushed off the right edge.Two review findings, both about a filter that looks applied and is not: - The transform branch of the clause builder interpolated whatever convertJsDateToSasDate returned, so an Invalid Date reaching the helper would have put a literal NaN in the WHERE clause. A non-finite result is now dropped, and unsupportedTransforms reports the parameter, so the author sees a banner instead of a broken filter. - The range count check counted values that survived date parsing, while the builder counts resolved bounds. A RANGE sent two values where one did not parse therefore built a BETWEEN covering one period with no warning. The check now counts what VA sent, the builder refuses a range with more than one value rather than picking one, and a value set that is partly unparseable gets its own line ('1 of its 2 values was not a date - the filter covers only the rest'). The banner also now says what to do: it names the URL parameter to edit and shows the entry grammar with an example, since the reader is the report author and has no other way to act on it.Review follow-up on the embed guard. The spec asserted the isEmbedded getter, which mirrors the template rather than exercising it: dropping !isEmbedded from the @if guards left every test green. The spec now renders the component in a TestBed and asserts the DOM - #TCS present standalone and absent for embed=va and embed=true, and the deploy section withheld after the terms are accepted when embedded. Both guards were mutation-checked by removing them, which fails the spec. deployUrl and onNavigateToHome computed the standalone URL two different ways: origin+pathname in one, location.href.split('#')[0] in the other, which kept the query string, so a tab opened from ?embed=va#/deploy stayed embedded. Both now call standaloneUrl() in standalone-url.ts, which parses the href and drops the query and the fragment. It is tested with a hash-bearing input, and the navigate test asserts the opened URL carries neither '?' nor '#', so a regression to split('#') fails.The self-destruct block ends with a single-statement %else in open code: %if &keepme ne 1 %then %do; %mv_deletejes(path=&root/admin, name=makedata); %end; %else %put NOTE: &_program retained, as KEEPME was requested; Outside a macro SAS pairs %else only with a %do branch, so this stops with 'ERROR: Expected %DO not found' and 'ERROR: Skipping to next %END statement' after the deployment has completed. On Viya the job then reports state error with jobConditionCode 1012, so a successful deployment looks like a failure exactly when someone is watching a first install. Both branches now use %do/%end, which is the pattern the repo uses elsewhere for this reason - see db/migrations/20261003_row_and_cell_limits.sas. The same block is fixed in the server variant. The two delete calls also gain their terminating semicolons: the macro call worked without them, but their absence is what made the block look responsible for the error. Behaviour is unchanged: the program still deletes itself unless KEEPME was requested.Reviewed the full base...head diff of release/7.20 (
main@424b1117...00df87f1, 23 files, +3117/-83). The earlier review on this PR (ad50acf6) covered the CI workflow changes only; this one covers everything and re-checks those.What I ran
npx ng test --no-watch --browsers ChromeHeadlessCIon the four touched specs (va-filter.service, parse-va-parmap, va-parmap-transform, deploy.component): 97 tests, 97 SUCCESS, exit 0.prettier@3.8.4 --check(the version pinned in client/package.json) over all 17 touched client files: all clean.sasjs lintinsas/: no finding against either touchedmakedata.sas(the only makedata hit is the pre-existing sas9 one).00df87f1(/actions/runs2484-2486): Build / Build-and-ng-test success, Lighthouse success, Sites success, Build / Build-and-test-development still in progress when I looked.The va_parmap work itself reads well: the column name is regex-checked before it can reach a WHERE clause, transform ops/anchors/periods are an allow-list rather than an evaluator, the date arithmetic is calendar construction (leap years, DST, DATETIME time-of-day are covered by unit tests), and the diagnostics deliberately do not rewrite AND into OR. Two defects below, both proved with a throwaway spec run against this branch head.
1. A
va_parmapkeyed on the parameter's internal name is dropped with no warning at allva-filter.service.ts:498(mappedTarget) matches the parameter label only, deliberately - andva-filter.service.spec.ts:245pins that a mapping written againstpr273builds no clause. But:va-messaging.service.ts:209still records bothlabelandnameintoseenParamKeys, sounmatchedMappingsseespr273as "VA is sending this" and stays silent, andunresolvedMappingsstays silent too, because the mapped column exists in the table,parse-va-parmap.ts:44documents the key as matching "a VA parameter'slabeland then itsname", which is what invites the entry in the first place.Net effect: the author writes
va_parmap=pr273-SOME_DATE, every diagnostic says the configuration is fine, and the grid is never filtered - exactly the "looks like a working filter over the whole table" failure the three diagnostics exist to prevent, and the one caseunmatchedMappingsclaims to cover.Probe (message with one parameter
{name: 'pr273', label: 'Tab_1_Date', value: '2025-03-31'}, mappr273 -> SOME_DATE, stable parameter set):Fix: record only the label in
seenParamKeys(or key the set the same waymappedTargetdoes), drop thenameclaim from theparseVaParmapdocstring, and add the spec that a name-keyed entry is reported as unmatched.2.
standaloneUrlstrips?_program=, so the deploy tab opens a dead URL on SAS 9 / Viyastandalone-url.ts:13returnsorigin + pathname, discarding the whole query. On SAS 9 and Viya the DC app URL is addressed entirely through the query (/SASJobExecution/?_program=/.../startupservice), so:The old
location.href.split('#')[0]kept the query and only dropped the hash, so this is a regression in shipped behaviour, not just in the new notice:onNavigateToHome()(deploy.component.ts:81) is what the automatic and sasjs-configurator deploy sections emit to reopen the app after a deployment, and on SAS 9 / Viya that now lands on a bare/SASJobExecution/. The newdeployUrllink in the embedded notice is broken the same way.Fix: remove only what makes the URL embedded -
embedand theva_*parameters - plus the hash, and keep the rest of the query.deploy.component.spec.ts:176only ever exercises path-addressed URLs, which is why this slipped through; add a?_program=case.3. Minor: the timezone scripts are not run anywhere
client/package.json:41-42addstest:tz-west/test:tz-east, but no workflow invokes them -build.yaml:76runstest:headlessonly. The local-date reasoning inva-parmap-transform.tsis the reason those runs exist, so either add them to Build-and-ng-test (a two-entry matrix) or note in the PR that the tz proof is manual.Still open from the earlier review
All seven findings from the
ad50acf6review are present at this head, unchanged:release-assets.yaml:172deletes every asset on the release, not only the ones this workflow uploaded.release-assets.yaml:64usesjqtwo steps before it is installed at :78.release-assets.yaml:79anddocs.yaml:95install@sasjs/cliunpinned in files that pin Surfer exactly.docs.yaml:97usesnpm iinsas/where the typedoc job usesnpm ci.docs.yaml:31overwritesclient/.npmrcwith the secret instead of appending to it.release.yaml"Publish the code documentation" resolves the tag fromreleases/latestrather than the tag this job cut.main.Nothing here is a reason to hold the release beyond 1 and 2, which are both small and self-contained.
Addendum to the review at
00df87f1- one finding neither review on this PR reaches, plus two small ones. Not repeating what those reviews already cover..gitea/workflows/release-assets.yaml:62,64,150,173,182,192,195 - every forge API call passes
curl -k, so certificate verification is off on the calls that delete release assets, PATCH the release body and upload the artefacts, each carryingAuthorization: token $GITEA_TOKEN.-kaccepts any certificate, so a MITM on the runner's egress captures a token that can rewrite releases - and it is the delete loop at :173 and the upload at :195 that make it worth a token. If the certificate is the obstacle, pass it with--cacert(or fix the cert chain); a self-signed cert pinned by hand is still verification,-kis none. Same pattern appears in the new dispatch block in release.yaml.client/src/app/shared/utils/va-parmap-transform.ts:141 - a RANGE on a datetime column closes at
setHours(23, 59, 59, 0), which excludes a value in the final second carrying a fractional part. Correct if DC's datetimes are whole seconds; worth one comment saying so, because the surrounding comments are otherwise explicit about why each boundary is where it is (the DST note above it is the standard the file sets for itself).editor.component.html, the
clr-alertbound tovaMappingWarnings-[clrAlertClosable]="false"with a getter that rebuilds four arrays per change-detection pass. The banner is author-facing and should persist, but consider allowing the reader to dismiss it for the session, and returning the stored arrays rather than re-spreading them in the getter.3 findings above. The first is the only one I would hold the release for; 2 and 3 are comments.
Answering review 572, one point per finding. Fixes are on #391 (base
release/7.20, 3 commits), not on this branch, so this release PR keeps its history.Name-keyed
va_parmapsilently dropped - fixed.seenParamKeysnow records the parameter label only (the key the matcher uses), and theparseVaParmapheader no longer advertises the internal name. A name-keyed entry now surfaces as "VA is not sending this parameter", which is the diagnostic that was missing. Red-before/green-after: on00df87fthe two newva-messagingcases fail (seenParamKeys=["tab_1_date","pr273"],unmatchedMappings=[]); with the fix the same suite is green and the label-keyed control stays silent.standaloneUrldropping?_program=- fixed. It now removes only theembedswitch, theva_*parameters and the hash, and keeps the rest of the query, so a SAS 9 / ViyaSASJobExecution/?_program=...URL survives intact. The spec now covers that URL, ava_parmapURL, and a query that must be kept; on00df87fthe SAS 9 case returnshttp://localhost:9876/SASJobExecution/, which is the break you would have got from the "not configured" link and fromonNavigateToHome().Timezone scripts - wired in.
Build-and-ng-testrunstest:tz-westandtest:tz-eaststraight after the UTC run. Both were run locally on the fix branch: 687 SUCCESS each.Verification on #391: 105 SUCCESS across the five VA/deploy specs,
prettier@3.8.4 --checkclean on the five touched client files, both TZ suites 687/687. Not covered there: the Cypress VA suite and a real SAS 9 / Viya deployment - the_program=shape comes from the adapter's own URLs insas.service.spec.ts.The seven workflow findings from the earlier review on this PR are untouched by #391 and still present at
00df87f1.Review found that five specs ran in no shard - coltype-delete-record, download, embed-va, metanav, submitted-details - and one of them is the VA embed suite this release added, 454 lines that had never run in CI. The old single --spec string omitted them too, so nothing regressed; they were simply never wired up, and nothing in the repo marked them skipped. The spec lists now live in .gitea/scripts/cypress-shards.sh, one place, so the workflow no longer spells them out four times. A guard step in Build-and-ng-test compares the shards against the specs on disk and fails when a file belongs to none of them or to more than one - proven by removing a name and watching it fail with that file listed. Rebalanced for the five additions, on test count and spec count together, since each spec pays a few seconds of startup: shard 1 editor, csv, metanav 57 tests shard 2 excel, filtering, download, browser-info, excel-multi-load, hook-programs 49 tests shard 3 embed-va, viewbox, licensing, filter-panel, liveness 43 tests shard 4 viewer-labels, row-cell-limits, derived-variables, ddl-export, coltype-delete-record, submitted-details, startupservice-launcher, stage, full-table-search, csv-limited 27 tests 24 specs / 176 tests, each in exactly one shard. upload-artifact moved from the retired v3 to v4; this change multiplies that step by four, so it is worth not carrying the dead major forward. Not in this commit: the three new check names still need adding to branch protection, which needs repo admin.Re-checked
release/7.20at60589c7, with #391 and #392 merged plus the two follow-up commits on top.Fixed since the last reviews - verified, not taken on trust
febfea3addsif: always(), compares all fourneeds.*.resultvalues and fails on anything that is notsuccess, including the case where the needs context does not resolve (it counts the results and exits 1 rather than looping over nothing). That closes the finding that the required check would have goneskippedon a red shard.EMBED_PARAMS = ['embed', 'va_parmap']), so a laterva_*parameter survives - pinned by a test usingva_rowlimit.history.replaceStateand asserts the exact standalone URL, so it proves the app query survives instead of passing on the runner's own query string.**/va-*.spec.ts. I checked the match:va-filter.service.spec.tsandva-parmap-transform.spec.ts(the two carrying the date property) are in,parse-va-parmap.spec.ts(no date logic) is out. Correct..gitea/scripts/cypress-shards.sh coverage .against the tree:All 24 specs are assigned to exactly one shard.upload-artifact@v4.Must change before this release ships
curl -kon 15 forge API calls, with a write-scoped token in the header. Seven in.gitea/workflows/release-assets.yaml, eight in.gitea/workflows/release.yaml. These are the calls that delete release assets, PATCH the release body, upload the artefacts and dispatch workflows. Certificate verification off means any MITM on the runner's egress captures a token that can rewrite releases. If the certificate is the obstacle, pass it with--cacertrather than disabling verification.The release artefacts are built with floating tooling.
.gitea/workflows/release-assets.yaml:79npm i -g @sasjs/cli,:87npm i webpack,:96npm i(insas/, where the typedoc job usesnpm ci). This job produces the files users download. The same branch pins@cloudron/surfer@7.2.0indocs.yamlprecisely so an upstream release cannot publish itself - the release build deserves the same rule.No run of the four sharded Cypress jobs has ever completed, and five of the specs they now cover have never executed in CI. Across the last 25 runs every
cypress-shard-*job iscancelled(superseded by the next push) or still running - zero successes.embed-va.cy.tsis among the newly-wired specs: 13 tests for the va_parmap feature this release is about. So the release would ship a suite that has never run. Before merging this PR, get one full green four-shard run onrelease/7.20; the three runs on the current tip are still in progress.Should change, all small
main, after this merges.Build-and-test-development- that name is now the fan-in gate, not the old single e2e job - and that the fourcypress-shard-*names are not also required, since a failed shard fails the gate anyway and requiring both would double the wait.Note on capacity
Three runs are in progress on the tip alone, and each push now spawns ng-test plus four shard jobs, each of which installs, builds, fetches a SASjs server and starts one. On a queue that is already behind, the sharding will make throughput worse before it makes latency better. Worth watching for a day before assuming the split delivered what it promised.
Answering reviews 566 and 573, one point per finding. Fixes are on #393 (base
release/7.20, 4 commits, head8888734). Review 572's findings are on #391, now merged into this branch.Review 566
Asset cleanup (
release-assets.yaml:172) - fixed. The loop deletes only the names this workflow uploads (the seven artefact basenames plusSHA256SUMS); anything else on the release is left alone. Run against the real v7.19.0 release JSON plus a synthetichand-attached-notes.txt: all eight owned assets removed, the foreign one kept.jq before install - fixed.
zip/jqare installed in a step above "Resolve the release tag", so the resolve no longer depends on the runner image shipping jq.Unpinned
@sasjs/cli(release-assets.yaml:79,docs.yamlsas-docs) - fixed. Both jobs install the CLI at the version the checkout's ownsas/package.jsonpins (CLI_VERSION=$(node -p ...)); inrelease-assets.yamlthat step runs after the tag checkout, so it is the tag's pin. Derived:@sasjs/cli@6.2.0.npm ivsnpm ciinsas/- fixed.sas/package-lock.jsonis tracked and in sync (npm ci --dry-runinsas/: "added 159 packages"), so the sas-docs job usesnpm ci..npmrcoverwrite (docs.yaml:31) - the premise does not hold here:client/.npmrcis not committed. It is gitignored (.gitignore:23,git check-ignore -vconfirms) and untracked (git ls-filesfinds only the root.npmrcandsas/.npmrc), so the step creates the file and nothing is dropped. I switched the two new files to append anyway, so tracked content would survive if the file ever gains any.TAG from
releases/latest(release.yaml, "Publish the code documentation") - fixed, and applied to the same pattern in the upload-assets and demo-site steps.semantic-releasebumps the working-treepackage.jsonbefore the release commit (@semantic-release/npm's prepare runsnpm version ... --no-git-tag-version, checked in the plugin source at 13.1.5), so the job exportsRELEASE_TAGand uses it for the asset upload and both dispatches.releases/latestno longer appears in the release job.Docs-publish secrets - probed from a run on a throwaway branch (deleted afterwards; the probe printed only status codes and site names):
WEBDOC_DATACONTROLLER_IO->/api/profile200, a working app password. Its app carries thedefault(datacontroller.io),docsandwebdocsites, so the typedoc job's--site-name webdocis the right target and the marketing default site is a different site on that app.CODE_DATACONTROLLER_IO-> empty: the secret is missing, so the sas-docs deploy will fail on the first push to main with the "password value is empty" fingerprint. It needs setting to the app password for code.datacontroller.io before this merges.code.datacontroller.iodoes not appear in the webdoc app's site list, so it is its own app; the unnamed sas-docs deploy is the right form once the secret exists.Review 573
curl -k- fixed. The flag is gone from all fifteen calls in the two release workflows. The certificate needs no exception: the forge serves a publicly trusted chain (ssl_verify_result=0,Verify return code: 0 (ok)), and a probe run from a runner reaches the API without-k(git.datacontroller.io without -k -> 200), so the changed calls verify on the same infrastructure.and 3. (the RANGE end-of-day note and the banner getter) are not in #393 - the review marked them as comments rather than blockers; happy to take them as a follow-up.
Not executed before merge:
release.yamlonly runs on a push tomain, so its changed calls first execute on the next release; the evidence above is the certificate check, the tag derivation (plugin source), and the local runs of the changed blocks.The four shards made the release-branch run red, and the reasons are in the logs rather than in the theory: - every failure was a timeout, not an assertion - "cy.type() failed because the center of this element is hidden from view", "Expected <div.app-loading> not to exist ... continuously found". Four jobs on one runner means four SASjs servers, four ng serve and four Chromes competing for the same CPU, and the app under test starves; - upload-artifact@v4 does not work on this runner at all: "GHESNotSupportedError: @actions/artifact v2.0.0+, upload-artifact@v4+ ... not supported on GHES", so the v3 -> v4 move I made on review advice broke the artifact step outright. Measured, from the one sharded run that did complete: 20m34s wall against 33-35 minutes for the single job, with peak concurrency 3 - so a third off, not the four-fold the split suggests. That is not worth a flaky pipeline while a release sits on it. Kept from the sharding work, because those parts were the real gain: - the spec list has one home, .gitea/scripts/cypress-specs.sh; - Build-and-ng-test fails when a spec on disk is not listed, so the five that had never run in CI cannot go missing again - and they do run now; - videos are zipped and uploaded only on failure; - one job means one server, which is also what the guards and the balance arguments were protecting. The shard and fan-in jobs are gone with the branch that carried them.Reviewed
main...release/7.20atb8eea50(26 files, +3331 -92), focusing on what blocks the release. Three findings, all in the Cypress layer, all created by specs that only started running when the spec list was fixed.client/cypress/e2e/download.cy.ts:10 - this spec cannot pass in CI. Its first
beforeEachcallsdeleteDownloadsFolder(), which doescy.task('deleteFolder', ...)(client/cypress/util/deleteDownloadFolder.ts:3), and that task is registered only in client/cypress/plugins/index.js:46.cypress.config.tsnever loads that file - itssetupNodeEventsregisters nothing butbefore:browser:launch- and Cypress 10+ ignoresplugins/index.jsunless it is required fromsetupNodeEvents. So the task has no handler, which is the CI error verbatim:Fix by moving the
on('task', {...})block intosetupNodeEvents(or requiring the plugins file there). Pre-existing, but invisible while the spec ran nowhere - enabling it is what surfaced it.client/cypress/e2e/metanav.cy.ts:9-30 - this spec is knowingly broken and says so in its own source:
// BLOCKER // For unkown reasons, .clr-accordion-header-button always null although it is present on the page.ItsbeforeEachalso logs in twice -beforeAllcallscy.loginAndUpdateValidKey(), then every test re-visits the app and types intoinput.username/input.password, which is where CI fails:Either fix it (drop the second login, then deal with the accordion blocker) or take it out of the spec list with a link to an issue, so the exclusion is stated rather than implied.
.gitea/scripts/cypress-specs.sh - with both specs in the list,
Build-and-test-developmentcannot go green, and the guard inBuild-and-ng-testrefuses silent omission - which is the right design but leaves the release with no green run of the Cypress job since the list went from 19 to 24 specs; every attempt so far is either cancelled (superseded) or red. The two options are the fixes above, or an explicit exclusion: a documented skip list in the same script, each entry with an issue reference, so the guard stays honest and the job can pass. I would not ship with the job red and unexplained.No findings on the rest of the diff: the
va_parmapparsing and transform maths, the banner diagnostics, the label-only param keys,standaloneUrl(enumeratedembed/va_parmaprather than ava_prefix), the deploy guard and themakedata%elsefix all read correctly against their tests, and I checked the one that matters for injection - character values are interpolated as'${s.replace(/'/g, "''")}'(client/src/app/services/va-filter.service.ts), so a quote in a VA parameter cannot break out of the clause.3 findings above for review.
Four specs failed on release/7.20 once the list went from 19 to 24. Three of the four are addressed here; the fourth is a spec that cannot pass and is now excluded with an issue. download.cy.ts - it clears the downloads folder in an afterEach through cy.task('deleteFolder'), and that task was registered only in cypress/plugins/index.js, which Cypress 10+ never loads unless it is required from cypress.config.ts. The task is now registered in setupNodeEvents, where Cypress looks, and the login in its beforeEach only fills the form when the form is actually on screen - the unconditional version failed with 'the center of this element is hidden from view' whenever the session was already live. csv-limited.cy.ts and hook-programs.cy.ts - neither is broken; both depend on where they sit in the run. csv-limited asserts the free-tier 'Locked Feature' modal, which an earlier spec removes by planting a valid licence, and hook-programs asserts an approval row against its target table before other specs have added rows to the approval tables. The list is now the sequence that was green for months, with csv-limited first and hook-programs second, and the newer specs appended rather than inserted. The script says why order matters, because the next person to alphabetise it will break the same two specs. metanav.cy.ts - cannot pass, and says so in its own source: '// BLOCKER ... .clr-accordion-header-button always null although it is present on the page'. With the duplicate login in its beforeEach removed it now reaches that assertion and fails there, so it is excluded from the list with #395 as the reason, and the login fix is kept for whoever picks it up. The coverage check now distinguishes a deliberate exclusion from a forgotten file, so nothing can go missing silently. Not verified locally: the specs need an estate seeded by 'sasjs cbd -t server-ci' plus makedata, which a scratch mock does not reproduce - the app never leaves app-loading. The CI run is the verification.The run on release/7.20 went from four failing specs to two: csv-limited PASS (order restored) hook-programs PASS (order restored) metanav held out, #395 download 7 of 7 fail download now gets past its hooks - the deleteFolder task is registered - and fails on the button it clicks: AssertionError: expected '<button.btn.btn-sm.btn-success...>' to be 'visible' This element is not visible because its ancestor has 'position: fixed' CSS property and it is overflowed by other elements. Something fixed-position sits over it. That needs a scrollIntoView or an overlay dismissed, and each attempt costs a forty minute CI cycle, so it is held out with #398 carrying the exact error and the suggested fix. The list reports 22 run, 2 excluded. editor.cy.ts still fails one test of fifty-five, and it failed the same way in both runs: 'Expected <div.app-loading> not to exist ... continuously found', a load timeout on test 28 rather than an assertion. One retry in run mode absorbs that, none interactively; a real failure still fails twice and the retry's output is in the log.