ci: run workflows on main instead of dormant master/develop

[chrip]

Summary

The project moved from master to main between v10 and v11 (v10.0.0 is on master/stable10; v11.0.0 and v11.0.1 are on main), but the workflow branch filters were never updated. As a result Artifact, ESLint, Lint-php and Lint-phpcs had not run since 2026-05-21 and never ran for a main-targeted PR — leaving DCO as the only check on every PR.

This re-points them at main (plus stable* for maintenance lines) and fixes the two things that stopped them passing once enabled.

Found while reviewing #117, whose CI verification turned out to be unverifiable for exactly this reason.

Commits

1. ci: run workflows on main instead of dormant master/develop — branch filters
   [master, develop] / [master] → [main, 'stable*'] in artifact.yml,
   lint-eslint.yml, lint-php.yml, lint-phpcs.yml.
2. ci(eslint): build on node 24 to match the release pipeline — NODE_VERSION: 20 → 24.
3. fix(lint): clear eslint errors blocking CI on main — the 25 pre-existing eslint errors.

Why node 24 (commit 2)

With the filters fixed, ESLint failed at npm ci:

npm error `npm ci` can only install packages when your package.json and
npm-shrinkwrap.json are in sync.
npm error Missing: picomatch@4.0.5 from lock file

The lockfile is not corrupt. fdir@6.5.0 declares picomatch: ^3 || ^4 as an optional peer (peerDependenciesMeta.picomatch.optional: true). npm 10 — bundled with node 20 — tries to materialise that optional peer, resolves it to the newest match (picomatch@4.0.5, published recently), finds no lockfile entry and aborts. npm 11 honours the optionality and installs cleanly.

Nothing in this repo changed to cause it; it started failing when picomatch shipped 4.0.5.

Node 20 was the wrong pin regardless:

• @nextcloud/files@4.0.0 — a direct dependency — declares engines.node: ^24.0.0. Node 20 violates it (the EBADENGINE line in the failing log).
• appstore-build-publish.yml already builds releases on node ^24 / npm ^11.3, so release and lint CI were on different major toolchains.

Why the lint cleanup (commit 3)

Two mechanical, pre-existing problems, both surfaced only because CI finally ran:

• 18 × semi, all in src/listener.js. The shared config sets semi: ['error', 'never']; that one file was written with semicolons. Fixed by eslint --fix.
• 7 × no-unused-vars — desktop.js, directeditor.js, editor.js, listener.js, main.js, share.js, template.js each declared _ in a /* global */ comment for Nextcloud's retired Underscore.js. _( appears nowhere in any of them, so the declaration is dropped (and the comment removed entirely where _ was its only entry).

No behavioural change: 6 files are comment-only, and listener.js is 18 trailing-semicolon removals plus its comment — verified line by line.

Workflow audit

Since these workflows were inherited, I checked whether each still makes sense here rather than only renaming branches:

Workflow	Action	Rationale
lint-php	✅ filter fixed	php -l clean across 51 files
lint-phpcs	✅ filter fixed	48 files, 0 violations against the repo's own ruleset.xml
lint-eslint	✅ filter + node 24	eslint 0 errors, stylelint 0 problems
artifact	✅ filter fixed	full build + tarball; green in CI
create-tag	⛔ deliberately left on master	see below
release	untouched	works — tag-triggered, ran for v11.0.1
appstore-build-publish	untouched	correct as-is

create-tag.yml is intentionally not migrated. It has never run (0 runs ever — the v11 tags were created by hand). Pointing it at main is not a rename but a behaviour change: it would auto-tag every non-docs push to main, cascading into release.yml → a published GitHub release. That is a release-process decision and belongs in its own PR. Either delete it as vestigial or enable it deliberately.

appstore-build-publish.yml showing "skipped" is correct, not a bug: it is guarded by if: github.repository_owner == 'nextcloud-releases' and only runs in the mirror. release.yml handles the Euro-Office side, as its inline comment explains.

The two remaining master references in appstore-build-publish.yml point at nextcloud/server's branch and the app-certificate-requests repo — deliberately untouched.

Verification

All 8 checks green: Artifact, ESLint, Lint-php (8.1–8.4), Lint-phpcs, DCO.

Confirmed locally against the exact CI commands before pushing, with results matching CI character-for-character (✖ 90 problems (25 errors, 65 warnings) before the cleanup, 0 errors after).

Known follow-ups (not in this PR)

• 65 @nextcloud/no-deprecations warnings remain (OC.generateUrl, OCP.Toast, OC.filePath, OC.linkToOCS). Non-blocking, pre-existing. Worth a separate cleanup — and note that if --max-warnings 0 is ever added, they become failures.
• artifact.yml runs npm install, not npm ci, so it re-resolves and silently ignores the lockfile. It stayed green through the npm-10 breakage that ESLint caught. Switching it to npm ci would make it a real lockfile gate.
• No engines in package.json. Declaring node/npm would make the release pipeline's ^24 / ^11.3 fallback explicit instead of accidental, and give Renovate a constraint for lockfile regeneration.
• create-tag.yml needs the decision described above.

Assisted-by: ClaudeCode:claude-opus-5

[moodyjmz]

@chrip — reviewed, approve. Every factual claim in the description checks out against source; details at the bottom. Four things worth knowing before/after merge, none of them blockers.

TL;DR

1. main has no branch protection, so these checks run but gate nothing — a red ESLint still merges. This is the other half of the bug.
2. The 'stable*' filter is currently inert, and backporting commit 1 alone will make stable CI red.
3. release/stable11 doesn't match stable* — uncovered if release PRs target it.
4. Merge conflict incoming with change smart picker behavior #70 — same listener.js block. Trivial to resolve.

1 — main has no branch protection: the checks run but gate nothing

$ gh api repos/Euro-Office/eurooffice-nextcloud/branches/main/protection
404 Branch not protected

This PR fixes visibility, not enforcement. "Leaving DCO as the only check" was half the problem; the other half is that no check is required, so a red ESLint merges just as happily as a green one. Worth a follow-up to add required status checks (Lint, artifact, php-cs, php-lint (8.1–8.4), DCO) — arguably the higher-value change of the two.

2 — The 'stable*' filter is inert today, and backporting commit 1 alone will make stable red

GitHub resolves the workflow file from the pushed ref (push) or the merge ref (pull_request), not from main. So stable10/stable11 keep using their own copy:

$ git show origin/stable11:.github/workflows/lint-eslint.yml | head -12
on:
  push:
    branches: [master, develop]
  pull_request:
    branches: [master, develop]
env:
  NODE_VERSION: 20

Nothing changes on those branches until this is backported. And when it is, commit 3 has to come with it — both branches still carry the stale /* global _ */ declarations that commit 3 removes here:

branch	files declaring an unused _
stable10	7
stable11	8

Backporting the filter without the cleanup turns stable CI red on arrival. The workflow-audit table currently reads as though stable is covered — it will be, just not yet. Might be worth a line in the description saying so.

3 — release/stable11 doesn't match stable*

The glob matches stable10 and stable11 but not release/stable11, which also exists as a remote branch. If that's a branch release PRs actually target, it stays uncovered. Flagging with low confidence since I don't know whether it's live — either widen the glob or confirm it's vestigial.

4 — Merge conflict incoming with #70

Commit 3 de-semicolons exactly the getLinkWithPicker block that #70 rewrites (src/listener.js ~120–140). Whichever lands second needs a rebase. Trivial resolution — #70 already writes those lines semicolon-free — just don't be surprised by it.

Silver lining: once this merges, #70 gets real ESLint/artifact coverage for the first time, which is rather the point.

Verification — what I re-derived rather than took on trust

The description is AI-assisted (Assisted-by:), so I checked the claims against source instead of the prose. All of them hold:

Claim	Result
Not run since 2026-05-21	✅ last ESLint run was push → master, 2026-05-21
create-tag.yml never ran	✅ 0 runs ever
picomatch/fdir optional-peer diagnosis	✅ npm-shrinkwrap.json:9990-10006 — rollup-plugin-license/node_modules/fdir@6.5.0 declares picomatch: ^3 || ^4 with optional: true and no nested entry. Run history confirms it empirically: two failures on node 20, success on 24
listener.js is semicolons only	✅ strip trailing ; from both revisions and the sole remaining diff is the /* global */ line
_ unused in all 7 files	✅ no bare _ token remains in any of them; /* global */ is eslint-only, zero runtime effect
All 8 checks green	✅

settings.js:162 correctly keeps /* global _ */ — it genuinely uses _.debounce. Though that does undercut the "Underscore is retired" framing: either _ is still provided by the server, or that path is already broken. It sits behind if (typeof window.$ !== 'function') return (settings.js:154), so it's jQuery/select2-era dead code on NC 33+ anyway (info.xml min-version 33). Pre-existing, out of scope here, but a decent companion to the deprecation-warning cleanup you've already listed.

And of your stated follow-ups, npm install vs npm ci in artifact.yml is the one that matters — it sailed green straight through the exact breakage ESLint caught, which is the textbook definition of a check that isn't a gate.

_{Reviewed with Claude Code (claude-opus-5).}

[moodyjmz]

Follow-up for finding 1 filed as #121 — main has no branch protection, so the eight checks this PR revives are all advisory. Repo-settings change, needs admin rights; nothing in this PR can address it. Doesn't block merge here.

[moodyjmz]

@chrip — corrections to my review above. I put it through an adversarial re-check and finding 2 had real errors in it. Most consequential: the stable-branch table is wrong, and the "stable CI goes red" warning does not apply to stable10 at all. Corrected guidance is in #125; details below.

What was wrong

1. stable10 has zero unused _, not 7. Every /* global _ */ on that branch is legitimate — _.extend(...) is used in all 8 files, plus _.debounce in settings.js:

$ git grep -cE '_\.(extend|debounce|each|map)' origin/stable10 -- 'src/*.js'
desktop.js:1  directeditor.js:1  editor.js:1  listener.js:1
main.js:1     settings.js:2      share.js:1   template.js:1

stable10 also has no fdir entries at all in its lockfile, so the npm-10 breakage can't occur there either. Commit 1 backports cleanly and the branch stays green. Commit 3 would actively break it.

My error: I counted /* global */ declarations and labelled the number "unused" without checking usage — projecting main's state onto branches with different code.

2. stable11 is 7, not 8 (only settings.js uses _), and commit 3 alone isn't enough. stable11 still pins NODE_VERSION: 20 and carries the same three fdir@6.5.0 entries (npm-shrinkwrap.json:9768, :10890, :12066), so a commit-1-only backport dies at npm ci before eslint runs. It needs commits 1 + 2 + 3 together.

3. "Run history confirms it: two failures on node 20, success on 24" — wrong, there was one. Per-SHA:

commit	node	result
f1fe67a4 (1)	20	failure — npm ci
11f700e7 (2)	24	failure — eslint, 25 errors
9ce52403 (3)	24	success

The conclusion stands; the corroboration was half what I said.

4. release/stable11 — my "if release PRs target it" premise was wrong: no PR has ever targeted it as a base. It's a head branch (#86, #88 → main), so those changes already get coverage. And "widen the glob" wouldn't help — it doesn't begin with stable, so stable** misses it too; you'd need '**/stable*'. Real gap is push-event coverage only, i.e. much narrower than I implied.

5. settings.js:162 — I hedged correctly and then dropped the hedge with "dead code on NC 33+ anyway". Whether NC 33 still exposes _ isn't determinable from this repo. The guard structure is right; the conclusion was overstated.

What stands

Findings 1, 3 and 4 as posted. Plus, verified more rigorously than I originally managed:

• The stable*-is-inert mechanism — confirmed from this PR's own runs. Its ESLint log shows node: v24.18.0, a value existing only in the PR's copy of the workflow, while main still filtered on [master, develop]. So the file came from the merge ref and the filter matched the base. stable11 and release/stable11 have 0 runs ever.
• The picomatch diagnosis — tighter than I credited. rollup-plugin-license/node_modules/fdir is the only unsatisfied picomatch edge; tinyglobby's and vite's each have a satisfying sibling.
• listener.js zero behavioural change — my method was weak (stripping ;$ masks exactly the ASI hazard that matters). Proper proof: both revisions parsed with the repo's espree and compared as position-stripped ASTs — byte-identical, 72788 each. ASI included. Your "verified line by line" was right.
• The change smart picker behavior #70 conflict has materialised — now mergeable: CONFLICTING. git merge-tree isolates it to commit 3 specifically: clean against 2c0b4baf and f1fe67a4, conflicting from 9ce52403 onward.

Follow-ups filed

#125 (backport, with the corrected per-branch detail) · #126 (npm install → npm ci) · #127 (engines) · #128 (deprecation warnings) · #129 (create-tag.yml) · #121 (branch protection)

Worth noting the merge already earned its keep: Renovate rebased and several dependency PRs got their first real CI run, three of them red.

_{Reviewed with Claude Code (claude-opus-5); corrections after an adversarial second pass.}