ci: trigger on pull requests only, never on push - #32
Closed
serialexperimentslainnnn wants to merge 5 commits into
Closed
serialexperimentslainnnn wants to merge 5 commits into
serialexperimentslainnnn wants to merge 5 commits into
Conversation
A branch with an open pull request already fires `pull_request` on every push to
it (the `synchronize` event), so keeping a `push` trigger meant two complete
pipelines per commit for identical information. This removes the duplication at
its source instead of relying on the concurrency group to cancel one in time.
The result is the intended shape: a PR into develop runs the two required suites
once, a PR from develop into main runs all eight.
Two things this gives up, recorded because each removes something the current
setup was leaning on:
- There is no longer a CI run on the push a merge into develop creates. That
run was the stated justification for dropping the up-to-date requirement on
develop: two pull requests that are green apart can break together, and
develop's own run was what would have caught it. It is now caught at the pull
request into main, where the full gate runs — later, but still before
anything is published.
- A branch with no open pull request gets no checks at all, and a pull request
from a fork is the only path that would ever exercise them for an outside
contributor.
release.yml is untouched: it carries its own `push: branches: [main]` trigger and
still fires on the merge that publishes.
Points the five toolchain jobs at ghcr.io/serialexperimentslainnnn/cc-ci and drops
the setup-java and setup-node steps, which provisioned inside a container that
already has both. `Build plugin` keeps no container: it only unzips an artifact.
GRADLE_USER_HOME is set per job and MUST match the value in the Dockerfile. If the
two diverge nothing fails — the run simply re-downloads everything the image
already holds, and the image appears to have bought nothing. That silence is the
reason it is stated at the setting rather than assumed.
NOT VERIFIED against a real image: at the time of writing it has not been built or
pushed. Two things have to be true before this can merge, and both fail in ways
that look like something else:
- the package must be public (or linked to this repo), or every job dies on a
401 that reads like a wrong image name;
- the warmed caches must actually be in the image — `docker run --rm IMAGE
sh -c 'ls /opt/gradle-home/caches'` answers it in seconds.
Also still open: whether gradle/actions/setup-gradle should stay. It restores its
own cache over GRADLE_USER_HOME, so it now layers on top of the baked one. That
may be a useful increment or redundant work; it needs measuring, not guessing.
The package stays private. Each container job authenticates with the GITHUB_TOKEN the run already has, so there is no new secret to create, store or rotate, and the credential expires with the job. `packages: read` is granted per job rather than at the top level, keeping the default token read-only on everything else. Without it the pull fails with a 401 that reads like a wrong image name rather than a permission problem — which is exactly the kind of error that gets debugged in the wrong place. One prerequisite this does NOT remove: the package must be linked to this repository, or the token has no grant on it. That is done once, from the package settings, and it is what makes "same account" mean "same permissions" here.
The cleanup step wipes /warmup, and `npm ci` had installed node_modules inside it — so the image built the frontend dependencies and deleted them moments later. The warm-up looked like it worked and bought nothing: CI would re-download the whole tree on every run, silently, because nothing fails when a cache is missing. Setting npm_config_cache moves the reusable part to /opt/npm-cache, which the cleanup does not touch. node_modules stays disposable, and that is correct independently of this bug: it must match the package-lock.json of the commit CI checks out, not the one that happened to be current when the image was cut. Found by a question about what that `rm -rf` actually deletes, which is a better review than reading the line I had just written myself.
The image was already pulled by most jobs; the remaining ones provisioned their own JDK, Node and Gradle cache and so ran on a toolchain nothing else had used. Now `Build plugin`, the protocol-drift check and the release gate use it too, which is the point of having built it. `gradle/actions/setup-gradle` is removed everywhere rather than set to read-only, because it was not doing the job it appeared to be doing. The warm GRADLE_USER_HOME measures 31 GB — 23 GB of extracted IDE transforms under caches/9.5.1 and 7.7 GB of downloaded IDE artifacts under modules-2 — and an Actions cache entry is capped at 10 GB per repository. It could only ever have stored a fraction, evicted it, and re-downloaded the rest next run. The image has no such ceiling. The trade is explicit and worth stating: refreshing what CI has cached is now a deliberate rebuild-and-push, not something that drifts between runs. Also removes the verifier's `Free disk space` step. Inside a container those paths are the IMAGE's, not the runner's, so it had been freeing nothing while looking like this job's safety margin. The margin now comes from the IDEs being baked: nothing is downloaded or extracted at verify time. Recorded because it is the failure everyone hits once: the private package must be granted Read access to this repository in its own settings. The `packages: read` permission widens what the token may ASK for; it does not authorise it against a package the repo was never linked to, and without the link the pull fails with a bare `denied` that reads like a wrong image name. Not containerised, deliberately: `publish`, which holds the Marketplace token and the signing key and is not a test, and CodeQL, which is weekly and is where a container breaks quietly. Not verified: that the image builds with no network at all. The one attempt failed on uid mapping, which says nothing about CI, where the container runs as root.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A branch with an open pull request already fires
pull_requeston every push to it (thesynchronizeevent), so keeping apushtrigger meant two complete pipelines per commit for identical information. This removes the duplication at its source instead of relying on the concurrency group to cancel one in time.The result is the intended shape: a PR into develop runs the two required suites once, a PR from develop into main runs all eight.
Two things this gives up, recorded because each removes something the current setup was leaning on:
release.yml is untouched: it carries its own
push: branches: [main]trigger and still fires on the merge that publishes.Pull request
Summary
What does this PR change and why? One short paragraph is fine.
Related issue
Closes #
Type of change
Risk and rollback
Risk: what breaks if this is wrong, and for whom? (
noneis a validanswer for docs-only changes — say so rather than leaving it blank.)
Rollback: how is this undone once released? Reverting the commit is not a
rollback for a published plugin — a user on the bad version stays there until
they update. If the change touches persisted settings, the transcript format,
or the permission surface, say what happens to a user who already ran it.
Checklist
developbranch (ormainonly for hotfixes).commit-msghook enforces it —install once with
git config core.hooksPath .githooks)../gradlew test verifyPlugin buildPluginpasses locally.verifyPluginis Compatible across the declared range (251 → 263.*)and reports no new internal-API usage (
@ApiStatus.Internal).The CDN download is unreliable here; use
-PlocalIdePath=<dir>[,<dir>…]with locally-extracted IDEs.src/test/kotlin/…forKotlin,
src/test/frontend/…(npm test) for anything undersrc/main/resources/jcef/../gradlew checkDriftis green and the baseline inscripts/drift-baseline.propertiesmatches what was verified.recorded in
THIRD-PARTY-NOTICES.mdif itships in the artifact.
CHANGELOG.mdand
RELEASE_NOTES.mdunderUnreleased.paths in the diff or commit messages.
CONTRIBUTING.md, thearchitectural contract in
CLAUDE.md, and the recordeddecisions in
docs/adr/.How was this tested?
./gradlew test) and frontend tests (npm test)./gradlew runIde) — describe the scenarios youexercised.
visible on every control touched. Automated checks catch roughly half of
real accessibility barriers and none of the judgement calls, so this one
is not delegable to a tool.
Notes for reviewers
Anything tricky, follow-up work, or open questions.