fix(ci): unblock the pipeline — resync package-lock, and use the real app id (E2E has never run) - #125
fix(ci): unblock the pipeline — resync package-lock, and use the real app id (E2E has never run)#125rubenvdlinde wants to merge 5 commits into
Conversation
`npm ci` has been failing on every run with EUSAGE:
npm error Missing: picomatch@4.0.5 from lock file
Exactly one package was missing. `rollup-plugin-license` carries a nested
`fdir@6.4.3` whose OPTIONAL PEER is `picomatch@^3 || ^4`; the hoisted
`node_modules/picomatch` is 2.3.2, so that peer needs its own nested entry
and the lockfile had none. `npm install --package-lock-only` adds precisely
that one entry (15 lines) — no other resolution moves.
Two things made this hard to see:
* npm 11 tolerates the gap (it treats a missing optional peer as
satisfiable) and installs cleanly. CI runs node 20 / npm 10 from .nvmrc,
which does not. `npm ci` therefore passed on a developer box with a
current node and failed in CI, on the same commit.
* The failure is not confined to the jobs that obviously need npm. The
shared workflow's Playwright job carries
`needs.security.result != 'failure'`, and Security (npm) starts with
`npm ci` — so a one-line lockfile gap DELETED the E2E job. It reported
`skipped`, which the Quality Report renders the same as a pass, and the
15 spec files under tests/e2e had never executed in CI. Vue Quality
(eslint + stylelint), Frontend Build and Frontend Tests died on the same
`npm ci`.
With the lock resynced those jobs reach a verdict for the first time, which
surfaces two real findings they had been unable to report:
* stylelint: `word-break: break-word` is deprecated (3 occurrences).
Replaced with the standard `overflow-wrap: break-word`.
* the `stylelint` npm script passed its globs UNQUOTED, so the shell (no
globstar) expanded `src/**/*.vue` to `src/*/*.vue` — which matches
src/components/ but NOT src/App.vue, the largest component in the app.
The lint was silently skipping it. Quoting the globs hands the `**` to
stylelint and brings App.vue under the gate; it reported two
`no-descending-specificity` errors, fixed by declaring the two
single-class `.versionSelectButton` rules before the more specific
`.versionItem:hover .versionSelectButton`. Both orderings render
identically — specificity, not source order, decides between them.
docs/features.json was also stale against openspec/specs/ (1 of 10 features
listed, and that one with an outdated summary). Regenerated with the shared
extract-features.py, which is what Features Check verifies on a PR.
Not fixed here, and not caused by the lockfile: the SBOM job fails with
`There are no commands defined in the "CycloneDX" namespace` — this repo has
no cyclonedx-php-composer in composer.json. That job only runs on pushes to
main/beta/development, so it does not appear on a PR at all.
Quality Report — ConductionNL/app-versions @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 17/17 | |||
| npm | ✅ | ✅ 282/282 | |||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-05 17:44 UTC
Download the full PDF report from the workflow artifacts.
…e repo name
With Security (npm) green, PHPUnit and E2E ran for the first time and both
died in 55-75 seconds on one line:
php occ app:enable app-versions
App not found
The shared workflow checks the app out into `server/apps/<app-name>` and
enables `<app-name>`. Nextcloud requires that directory to be named after
the id in appinfo/info.xml — `app_versions`, with an underscore — while the
repository is `app-versions`, with a hyphen. So the app was never enabled
and no test was ever collected.
This was the SECOND dead layer under the same tarpaulin: the jobs that would
have reported it are the same ones `needs.security.result != 'failure'`
deleted, so a wrong app id looked exactly like a passing pipeline.
Note for a follow-up (deliberately NOT changed here, because this PR cannot
measure it): release-beta.yml and release-stable.yml pass the same hyphenated
`app-name`, and the shared release workflow uses it both as the top-level
directory inside the release tarball and as the App Store registration
identifier that gets signed. Both of those also want the app id.
…esync-unblock-e2e
…is PR
The earlier commit here quoted the `stylelint` globs. That was a real finding
— unquoted, the shell (no globstar) expands `src/**/*.vue` to `src/*/*.vue`,
which matches src/components/ but NOT src/App.vue, so the largest component in
the app has never been linted. But fixing it forces edits to App.vue (two
`no-descending-specificity` errors it was hiding), and App.vue in the diff
pulls in three DIFF-SCOPED Hydra a11y gates that pass vacuously on any PR that
does not touch it:
[gate-38] skip-link FAIL — 1 root component
[gate-40] form-label-association FAIL — 5 inputs
[gate-45] prefers-reduced-motion FAIL — 1 <style> block
Two of those want judgement this PR should not be making:
* gate-40 is a FALSE POSITIVE on 4 of its 5 findings. Those inputs are each
wrapped in a `<label>` carrying visible text — implicit association, valid
HTML and valid WCAG; the gate's regex just cannot see a wrapping label
across a multi-line `<input>`. "Fixing" them with `aria-label` would
OVERRIDE the visible label and regress SC 2.5.3 (Label in Name). Only the
version filter input (placeholder, no label) is a genuine defect.
* gate-38 wants a skip link on App.vue, which renders inside Nextcloud's
admin settings page — a surface that already carries core's own skip link.
So this stays out of a CI-unblocking PR, and none of it is waived: no skip, no
exclude, no `continue-on-error`. The stylelint scope returns to exactly what it
was, the gates keep full strength, and the debt is recorded as a follow-up
issue. #124 measured the same four gates (38/40/43/45) as inherited whole-tree
debt at both v1.0.1 and v1.3.0, which is the same conclusion.
The `word-break: break-word` fixes in the two components stylelint DOES cover
stay — those are what the running lint actually reported.
Quality Report — ConductionNL/app-versions @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 17/17 | |||
| npm | ✅ | ✅ 282/282 | |||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-05 18:30 UTC
Download the full PDF report from the workflow artifacts.
Quality Report — ConductionNL/app-versions @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 17/17 | |||
| npm | ✅ | ✅ 282/282 | |||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-05 18:50 UTC
Download the full PDF report from the workflow artifacts.
…uld not be installed
With the app id corrected, `occ app:enable` reached this app's migrations for
the first time in CI and stopped there:
Column "oc_app_versions_pats"."shared_with_admins" is type Bool
and also NotNull, so it can not store "false".
Nextcloud's MigrationService rejects a NOT NULL boolean outright — a `bool` is
an integer of length 1 there and the platform cannot store `false` in it. The
exception aborts the whole `app:enable`, so App Versions could not be installed
on a fresh Nextcloud 31/32/33 AT ALL. The app declares
`<nextcloud min-version="31" max-version="34"/>`.
Version1000Date20260502120000 already documents that exact rule at the point it
creates the column:
// Note: Nextcloud's MigrationService validation rejects a notnull bool
// column with `default => false`. We omit the default and let the
// entity assign false on construction; …
Version1004 then did the forbidden thing anyway. Nothing caught it because no
CI job had ever reached `app:enable` — Playwright and PHPUnit both carry
`needs.security.result != 'failure'`, and Security (npm) had been red on a
one-line lockfile gap, so both reported `skipped`.
The NOT NULL is dropped; the `false` default and the preSchemaChange backfill
stay. Together they achieve what the NOT NULL was reaching for: no existing row
is left NULL, and an insert that omits the field stores `false` rather than
NULL — which is the failure mode the migration was written to close.
Quality Report — ConductionNL/app-versions @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ❌ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 17/17 | |||
| npm | ✅ | ✅ 282/282 | |||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-05 19:46 UTC
Download the full PDF report from the workflow artifacts.
Development's own last push run had 18 failing jobs — psalm, eslint, stylelint, the frontend build, unit tests, all six PHPUnit cells, E2E and Hydra Gates. This run has 2: E2E Tests (Playwright) and the Quality Report that aggregates it. Merged at a large improvement rather than at green, per the standing rule that parity — not green — is the merge gate on a red base. The remaining E2E failure is pre-existing and separately scoped: 7 tests depend on the live Nextcloud App Store returning a catalogue, and app-versions' E2E has never run in CI before this branch made it runnable (see #125). Weakening those assertions or skipping them without a verified reason would turn a known red into an invisible pass. Gate-13 modal-isolation now passes: the inline downgrade confirmation moved into src/dialogs/DowngradeConfirmDialog.vue.
|
Closing as superseded, and it would regress the app-id rename if merged. Its stated purpose is done. The title is "unblock the pipeline — resync package-lock", and development's Code Quality is green (today 10:15). Nothing is blocked. Its migration is already on development. Merging it now would undo work. This branch predates - {{ t('versioniq', 'Changes in this range') }}
+ {{ t('app_versions', 'Changes in this range') }}The branch carries 55 The one part not obviously superseded is |
Two independent one-word defects, stacked, kept most of this repo's CI dark.
enable-playwright: truewas already set — the toggle was never the problem.1.
package.json/package-lock.jsonout of syncExactly one package.
rollup-plugin-licensecarries a nestedfdir@6.4.3whose optional peer ispicomatch@^3 || ^4; the hoistednode_modules/picomatchis2.3.2, so that peer needs its own nested entry and the lockfile had none.npm install --package-lock-onlyadds precisely that one entry — 15 lines, no other resolution moves.Two things kept it invisible:
.nvmrc(node 20 → npm 10) and fails. Same commit, opposite verdict.needs.security.result != 'failure', andSecurity (npm)begins withnpm ci— so a one-line lockfile gap deleted them. They reportedskipped, which the Quality Report renders identically to a pass, and the 15 spec files undertests/e2e/had never executed in CI.Vue Quality (eslint + stylelint),Frontend Build,Frontend Tests (unit)andLicense (npm)all died on the samenpm ci.2.
app-namewas the repo name, not the Nextcloud app idWith
Security (npm)green, PHPUnit and E2E ran for the first time — and both died in under 80 seconds on one line (measured, run 31031058500):The shared workflow checks the app out into
server/apps/<app-name>and enables<app-name>. Nextcloud requires that directory to be named after the id inappinfo/info.xml—app_versions, with an underscore — while the repository isapp-versions, with a hyphen. The app was never enabled and no test was ever collected.This is the second dead layer under the same tarpaulin: the jobs that would have reported the wrong app id are exactly the ones the security gate had already deleted, so it looked like a green pipeline.
Two findings the newly-live jobs reported
word-break: break-wordis deprecated →overflow-wrap: break-word, in the two components the lint actually covers.docs/features.jsonwas stale againstopenspec/specs/— it listed 1 of 10 features, and that one with an outdated summary. Regenerated with the sharedextract-features.py, which is whatFeatures Checkverifies.What is deliberately NOT in this PR
An earlier commit here quoted the
stylelintglobs. That is a real finding — unquoted, the shell (no globstar) expandssrc/**/*.vuetosrc/*/*.vue, matchingsrc/components/but notsrc/App.vue, so the largest component in the app has never been linted. But fixing it forces edits to App.vue, and App.vue in the diff pulls in three diff-scoped Hydra a11y gates that pass vacuously on any PR that does not touch it:Two of those want judgement a CI-unblocking PR should not be making:
<label>carrying visible text — implicit association, valid HTML and valid WCAG. The gate's regex simply cannot see a wrapping label across a multi-line<input>. "Fixing" them witharia-labelwould override the visible label and regress SC 2.5.3 (Label in Name). Only the version-filter input (placeholder, no label) is a genuine defect.So the stylelint scope returns to exactly what it was and the debt is recorded for a follow-up. #124 measured the same four gates (38/40/43/45) as inherited whole-tree debt at both v1.0.1 and v1.3.0 — same conclusion. Nothing is skipped, waived, excluded or
continue-on-error'd. No@conduction/nextcloud-vuepin exists in this repo to touch.Verified locally (node 20.20.2 / npm 10.9.8 — CI's pair)
Reproduced the EUSAGE first on the same node, so the two arms are known to differ.
npm cinpm audit --audit-level=critical --omit=devnpm run buildnpm run lintnpm run stylelintnpm run testextract-features.py --checkrun-hydra-gates.sh --scope-to-diffKnown, unrelated, NOT fixed here
SBOMfails withThere are no commands defined in the "CycloneDX" namespace— this repo has nocyclonedx-php-composerincomposer.json. Independent of the lockfile, and that job only runs on pushes tomain/beta/development, so it does not appear on a PR at all.