Skip to content

ci: fail when the generated config pages drift from core - #143

Merged
phil-davis merged 2 commits into
mainfrom
ci/config-docs-in-sync
Sep 22, 2026
Merged

phil-davis merged 2 commits into
mainfrom
ci/config-docs-in-sync

Conversation

@oc-tmueller

Copy link
Copy Markdown
Contributor

Closes the gap that #137, #138 and #142 exposed: these pages are generated, nothing enforced it, and both directions of failure had already happened.

This regenerates every version from its core branch and fails on any difference.

Why it does not trust exit codes

convert.php exits 0 on both of its own failure paths — unreadable input and unwritable output. So a sample renamed in core would leave the pages untouched, the diff would find nothing, and the job would pass having checked nothing. That is the same class of bug this workflow exists to catch, one level up, so every file is checked before use and the pages actually regenerated are counted and asserted (Regenerated 6 pages across 3 versions.).

Design notes

Loop-driven, not list-driven. The loop iterates content/server/*/, so a new version cannot be added and silently left unchecked. The core branch is resolved by name, falling back to master only for the in-development version — and that fallback is validated against master's own OC_VersionString, so a directory matching no core branch fails loudly rather than being regenerated from the wrong source.

git ls-remote exit 2 ("no such branch") is distinguished from every other failure. Conflating them would let a transient network error resolve 10.16 to master and then fail the diff for a baffling reason.

Not path-filtered. A required check that is skipped reports as perpetually pending and blocks merges — the trap documented at length in lint-pr-title.yml. The cost is that core-side drift surfaces on an unrelated author's PR, so there is also a nightly run to find it first.

Actions policy. PHP comes from the ownCloud-owned owncloudci/php:7.4 container rather than a third-party setup action. Both actions are actions/*, SHA-pinned; checkout uses the same pin already in ci.yml.

One deliberate non-pin: the converter checkout floats on master, because the question being asked is whether the committed pages match what a maintainer running ctd.sh today would produce. Its composer deps are pinned, since the lock file is the generator's own declared contract.

On failure the explanation and full diff go to the job summary, and the regenerated pages upload as an artifact — downloadable rather than copy-pasted out of a log.

Verified by running the job's own script

Extracted the run: blocks and executed them in owncloudci/php:7.4 against four cases:

case result
in-sync tree passes, Regenerated 6 pages across 3 versions.
committed hand edit below the marker fails, with the diff
edit above the marker passes — the generator preserves that region by design
version directory with no matching core branch fails: core has no 10.14 branch and master is 11.0, so there is no source for content/server/10.14

Runtime is ~1.5 MB per core clone (--filter=blob:none matters here — a plain --depth 1 --sparse clone still transfers ~30 MB of blobs), three clones per run.

Two things for you to decide

  1. Whether to make this a required check. That is a repo-settings call, not something this PR can do. It is only meaningful as a gate if required.
  2. Stacking. Based on docs: sync config.sample.php parameters for 10.15 #142, because the check is red on main today and green once 10.15 is in sync. Merge docs: sync config.sample.php parameters for 10.15 #142 first; GitHub will retarget this to main.

🤖 Generated with Claude Code

The same staleness just fixed for 10.16 in #138: core added
'web-updater.enabled' to config/config.sample.php in owncloud/core#41385,
which is present on the 10.15 branch too, and the generated page was never
regenerated.

Regenerated with config-to-docs from core's 10.15 branch. The diff is the one
missing section, byte-identical to the block #138 added for 10.16, and the
hand-written header above the "header end" marker is untouched.

With this, all three published server versions regenerate to an empty diff
against their core branch, which is the precondition for checking that
automatically rather than discovering drift years later.

The same caveat as #138 applies: the pre-existing 'upgrade.disable-web'
comment a few lines above still claims the web updater is enabled by default,
which owncloud/core#41385 invalidated. That wants fixing in core, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
@oc-tmueller
oc-tmueller requested a review from a team as a code owner September 21, 2026 15:47
The two config parameter pages per server version are generated by
config-to-docs from core's config/config.sample.php and
config.apps.sample.php. Everything below the "header end do not delete or edit
this line" marker is machine output. Nothing enforced that, so a hand edit
there looked correct, rendered correctly, passed CI, and would have been
destroyed the next time anyone regenerated - and drift in the other direction
went unnoticed for two releases, which is what #138 and #142 just repaired.

This regenerates every version from its core branch and fails on any
difference, so the pages are provably generator output.

Nothing here infers success from an exit status, because convert.php exits 0 on
both of its own failure paths - unreadable input and unwritable output. Every
file is checked before use and the pages actually regenerated are counted and
asserted. Without that, a sample renamed in core would leave the pages
untouched, the comparison would find nothing, and the job would pass having
checked nothing at all - which is precisely the failure this workflow exists to
catch, one level up.

The comparison snapshots each page before regenerating and then plain-diffs the
two, rather than asking git. The pages are just files, and git inside this
container cannot see the workspace repository even though actions/checkout
populated it with git - which is worth knowing, but not worth depending on.

Other choices worth stating:

The loop is driven by the content/server/* directories themselves rather than a
version list, so a new server version cannot be added and silently left
unchecked. The core branch is resolved by name, falling back to master only for
the version still in development - and that fallback is verified against
master's own OC_VersionString, so a directory that matches no core branch fails
loudly instead of being regenerated from the wrong source.

git ls-remote's "no such branch" (exit 2) is distinguished from every other
failure. Conflating them would let a transient network error resolve 10.16 to
master and then fail the comparison for a baffling reason.

It is deliberately not path-filtered: a required check that is skipped reports
as perpetually pending and blocks merges, the trap already documented at length
in lint-pr-title.yml. The cost is that drift caused by a core commit surfaces
on an unrelated author's PR, so there is also a nightly run to find it first.

PHP comes from owncloudci/php:7.4 as a job container rather than a third-party
setup action, per the Actions policy in AGENTS.md. Inside a container the
default shell is sh, so bash is requested explicitly. The converter is
installed with `composer install`, not `update`: its comment parsing lives in
phpdocumentor/reflection-docblock, accepted as ^4.3.0 but pinned in the lock,
and floating it can change the generated output. The converter checkout itself
is deliberately unpinned - the question being asked is whether the pages match
what a maintainer running ctd.sh today would produce.

On failure the explanation and the full diff go to the job summary, and the
regenerated pages are uploaded as an artifact, so the fix can be downloaded
rather than copy-pasted out of a log.

Verified by running the job's own script in that container against four cases:
an in-sync tree passes and reports 6 pages across 3 versions; a hand edit below
the marker fails with the diff; an edit above the marker still passes, because
the generator preserves that region by design; and a version directory with no
corresponding core branch fails naming the mismatch.

Stacked on #142 - the check only goes green once 10.15 is in sync.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
@oc-tmueller
oc-tmueller force-pushed the ci/config-docs-in-sync branch from 7ff3fb2 to 801e1c2 Compare September 21, 2026 15:55
LukasHirt
LukasHirt previously approved these changes Sep 21, 2026
Base automatically changed from docs/sync-config-sample-10.15 to main September 22, 2026 03:07
@phil-davis
phil-davis dismissed LukasHirt’s stale review September 22, 2026 03:07

The base branch was changed.

@phil-davis
phil-davis merged commit 3bffbe4 into main Sep 22, 2026
6 checks passed
@phil-davis
phil-davis deleted the ci/config-docs-in-sync branch September 22, 2026 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants