Skip to content

build: maintenance on the build and release path - #66

Merged
feanil merged 11 commits into
mainfrom
feanil/more_maintenance
Sep 1, 2026
Merged

build: maintenance on the build and release path#66
feanil merged 11 commits into
mainfrom
feanil/more_maintenance

Conversation

@feanil

@feanil feanil commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Build and release maintenance, found by auditing the publish path end to end. Each commit stands alone.

Publishing

  • PyPI trusted publishing — both publish jobs already requested id-token: write, so dropping the user/password pair lets gh-action-pypi-publish exchange the GitHub OIDC token instead. This also un-breaks attestations, which the action was silently ignoring: The workflow was run with the 'attestations: true' input, but an explicit password was also set, disabling Trusted Publishing.
  • npm ci when publishing — every dependency in frontend-plugin-sample/package.json is "*", so the lockfile is the only thing pinning anything. npm install reconciles drift and carries on; npm ci installs the lock exactly and fails if it disagrees with the manifest. Worth having on the one build that becomes a published artifact.
  • npm version --no-git-tag-version — a no-op today, added as intent documentation. npm gates git work behind a stat for .git in the package directory and does not walk up to the repo root, so a subdirectory package never commits or tags. The flag stops that resting on repository layout.
  • Correct repository metadata — npm rewrote this field on every publish, so the registry metadata did not match the committed manifest. Adding directory also points the npmjs.com repository link at the subfolder.

The release could publish untested code

release.yml required only the backend test matrix. The frontend package had no CI at all: its first and only compile was inside publish_to_npm, which runs after the GitHub release is published and after both PyPI uploads. A syntax error in plugin.jsx would surface once two immutable PyPI versions and an immutable GitHub release already existed for a version with no npm package. The tutor plugin was likewise first built inside the release job.

This adds frontend-ci.yml and tutor-ci.yml (both pull_request + workflow_call) and makes release need all three, so nothing is tagged until every published package is known to build.

Two notes on those workflows:

  • The tutor checkout uses fetch-depth: 0. setuptools-scm reads the version from git tags and tutor-contrib-sample declares no fallback_version, unlike the backend, so a shallow clone breaks it. This also exercises a path the release never takes, since the release passes SETUPTOOLS_SCM_PRETEND_VERSION.
  • The frontend has no lint or test script, so compiling is the only check available. Adding those is a reasonable follow-up.

Correctness

  • The tutor plugin shipped no license file. license-files = ["LICENSE*"] was declared but that directory had no LICENSE, so the glob matched nothing and setuptools built a wheel with no License-File: in METADATA and no dist-info/licenses/. The package claimed Apache-2.0 without carrying the text. Verified fixed by rebuilding.
  • Python classifiers — the tutor plugin sets requires-python = ">=3.11" but only advertised 3.12.

Housekeeping

  • CI concurrency — without a concurrency group GitHub never cancels superseded runs, so every push to a PR started another five-job matrix while the previous five ran to completion. The release path is unaffected by construction: in a called workflow github.workflow resolves to the caller's name, and with cancel-in-progress: false GitHub keeps only one pending run per group, so non-pull_request events key on github.run_id to give each release run a group of its own.
  • Dead [wheel] universal = 1 — a Python 2 era flag on a package requiring >=3.12, and inert anyway since setuptools reads that option from [bdist_wheel].

Verification

actionlint passes on all workflows. Both new builds were run locally first: npm ci && npm run build compiles, and the tutor plugin builds sdist + wheel with twine check PASSED on both.

Follow-ups not in this PR

  • The weekly Upgrade Python Requirements job has failed every run (4/4 in August) — the shared openedx/.github workflow runs make upgrade from the repo root and there is no root Makefile. Deferred pending a larger change.
  • Dependabot only watches github-actions, so frontend-plugin-sample/package-lock.json gets no updates.
  • codecov.yml sits in backend-plugin-sample/, but Codecov reads it from the repo root, .github/, or dev/ — the 100% patch target may not be applied. Unverified.
  • Broken i18n make targets: Translation make targets point at a directory that does not exist #65.
  • Within a release the frontend is still compiled after PyPI is written. The new gate makes that near-harmless since the same SHA already built green, but restructuring so everything builds before anything publishes would close it properly.

feanil and others added 11 commits September 1, 2026 09:14
Both PyPI publish jobs already request the `id-token: write` permission,
so gh-action-pypi-publish can mint its upload credential from the
GitHub OIDC token instead of a long-lived API token stored in
PYPI_UPLOAD_TOKEN. Dropping the explicit user/password lets the action
take that path.

This requires a trusted publisher to be configured on PyPI for each
project (openedx-plugin-sample and tutor-contrib-sample), pointing at
this repository and the release.yml workflow:
https://docs.pypi.org/trusted-publishers/adding-a-publisher/

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tutor-contrib-sample/pyproject.toml declares
`license-files = ["LICENSE*"]`, but that directory had no LICENSE, so the
glob matched nothing and setuptools silently built a wheel with no
`License-File:` in METADATA and no `dist-info/licenses/` directory. The
published package claimed Apache-2.0 without carrying the text.

Copied verbatim from backend-plugin-sample/LICENSE.txt so both
distributions carry identical terms.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tutor plugin sets `requires-python = ">=3.11"` but only advertised a
3.12 classifier, so PyPI under-reported what it runs on. It works on both
3.11 and 3.12, so list both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`[wheel] universal = 1` is a Python 2 era setting that marks a build as
py2.py3 -- meaningless for a package that requires Python >=3.12. It was
inert anyway: setuptools reads this option from `[bdist_wheel]`, not
`[wheel]`, so nothing had been consuming it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Without a concurrency group GitHub never cancels superseded runs, so every
push to a PR branch starts another five-job matrix while the previous five
run to completion. Three pushes in a row means fifteen jobs, ten of them
for commits nobody will read.

The release calls this workflow via workflow_call, and in a called workflow
`github.workflow` resolves to the caller's name -- so a naive
`cancel-in-progress: true` would let a push to main cancel the tests of an
in-flight release, and a shared group key would make concurrent releases
queue behind one another. Keying non-pull_request events on `github.run_id`
gives every release run a group of its own, leaving that path untouched.

https://docs.github.com/en/actions/using-jobs/using-concurrency

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
npm rewrote this field on every publish, so the metadata on the registry
did not match the committed manifest:

    npm warn publish npm auto-corrected some errors in your package.json
    npm warn publish "repository" was changed from a string to an object
    npm warn publish "repository.url" was normalized to
      "git+https://github.com/openedx/sample-plugin.git"

Write it in the shape npm normalizes to, and add `directory` while we are
here -- the package lives in a subfolder, and that field is what makes the
repository link on npmjs.com point at frontend-plugin-sample rather than
the repo root.

https://docs.npmjs.com/cli/configuring-npm/package-json#repository

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The frontend package had no CI at all. Its first and only compile was the
`npm run build` inside the release workflow's publish_to_npm job, which runs
after the GitHub release is published and after both PyPI uploads. A syntax
error in plugin.jsx would therefore be discovered only once two immutable
PyPI versions and an immutable GitHub release existed for a version that has
no npm package.

Compiling is the only check available today -- the package defines no lint or
test script -- but it is the one that would have caught that.

Uses `npm ci` rather than the release job's `npm install`, so the build is
pinned to the committed lockfile and fails loudly if the lockfile and
package.json disagree. Aligning the release job is left for a follow-up.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Like the frontend package, the tutor plugin was never built until the
release workflow needed to publish it, so a broken pyproject.toml surfaced
only after the GitHub release and the backend PyPI upload had gone out.

Also runs `twine check`, which validates the metadata PyPI will accept and
render. The long description is read dynamically from README.md, so it is
worth checking that it still renders.

The checkout uses fetch-depth 0 because setuptools-scm derives the version
from git tags and this project declares no fallback_version, unlike the
backend. The release job sidesteps that with SETUPTOOLS_SCM_PRETEND_VERSION,
so this exercises a path the release never does: building from a plain
checkout, the way a contributor would locally.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The release only required the backend test matrix, so it would tag, publish
a GitHub release, and upload to PyPI while the frontend and tutor packages
were still unbuilt. Neither the GitHub release nor a PyPI upload can be
withdrawn, so a package that fails to build in its own publish job leaves
the release permanently half-finished.

Requiring all three CI workflows means nothing gets tagged until every
package this workflow publishes is known to build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`npm install` reconciles the lockfile with package.json and carries on;
`npm ci` installs the lockfile exactly and fails if the two have drifted
apart. For the one build that becomes a published artifact, failing loudly
is what we want -- especially here, where every dependency in package.json
is `"*"`, so the lockfile is the only thing pinning anything at all.

`--include=dev` goes away because `npm ci` installs dev dependencies by
default.

This matches what frontend-ci.yml already does, so a green PR build and the
release build now install the same tree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
We only want `npm version` to rewrite package.json here; the tag for this
release is created by python-semantic-release.

This is a no-op today. npm gates all git work behind a single check that
stats `.git` in the package directory and does not walk up to the repo root:

    const isGitDir = newversion === 'from-git' || await git.is(opts)
    const doGit = gitTagVersion && isGitDir && await enforceClean(opts)

frontend-plugin-sample is a subdirectory with no `.git` of its own, so
`doGit` is already false and no commit or tag is made. Verified by
reproduction: run in a subdirectory package, npm writes the version only;
run with package.json at the git root, it also commits and tags.

The flag makes the intent explicit rather than leaving it resting on
repository layout, which would silently start committing and tagging if this
package ever moved to the repo root.

https://github.com/npm/cli/blob/latest/workspaces/libnpmversion/lib/version.js

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.04%. Comparing base (e0c2730) to head (bb471ca).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #66   +/-   ##
=======================================
  Coverage   93.04%   93.04%           
=======================================
  Files          16       16           
  Lines         503      503           
  Branches       18       18           
=======================================
  Hits          468      468           
  Misses         24       24           
  Partials       11       11           
Flag Coverage Δ
unittests 93.04% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@feanil
feanil marked this pull request as ready for review September 1, 2026 14:06
@feanil
feanil merged commit 1f6766e into main Sep 1, 2026
11 checks passed
@feanil
feanil deleted the feanil/more_maintenance branch September 1, 2026 14:06
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.

1 participant