ci: run checks and builds on GitHub Actions, with release-intent automation - #87
Conversation
Runs the header, desktop, and Go test suites on pull requests and pushes to main and develop, plus unsigned installers across a six-way matrix. Windows builds natively rather than cross-compiled under wine. Also runs services/build.sh and build.bat natively on each platform. That is a second, independent build path over the same Go code: it parses versions with jq and stamps -ldflags -X main.Version, where the desktop path cross-compiles through a TypeScript script. Either can break alone. Neither workflow holds a secret or requests an OIDC token: both execute code from the pull request, so there must be no credential for that code to reach. Actions are pinned to full commit SHAs. Signed releases are built elsewhere, from a tag. Signed-off-by: Terve <ntervalon@nvidia.com>
Records the 0.1.0 and 0.1.1 notes as published on the releases page. This repository had no changelog, so release history was only discoverable there. Seeded from the published notes rather than carrying over historical development entries, and tracks the 0.1.x version that desktop/package.json and the release tags use. Signed-off-by: Terve <ntervalon@nvidia.com>
The field called `product` only ever versioned the services suite: it is declared inside services/, stamps the standalone installer and Go main.Version, and the version users install lives in desktop/package.json, which was explicitly out of scope for it. The two had drifted to 0.91.7 and 0.1.1, which is the clearest evidence they were never one number. `installer` is dropped: documented as 'always equals product', it could never differ from another field. Readers of '.installer // .product' now read '.services'. Updates all nine consumers, including build.sh which was reading .product and would have silently resolved null. The versions card already displayed this as 'Services', so no user-visible string changes. Signed-off-by: Terve <ntervalon@nvidia.com>
Contributors declare version bumps and changelog text in the pull request body; a check validates it, and after the merge lands on develop a bot applies it to desktop/package.json, services/versions.json, and CHANGELOG.md in one commit via the git data API. The contents API is one commit per file, which would land a release in pieces with a window where the changelog names a version package.json does not carry. The release version is not declared. It patch-bumps automatically whenever an intent declares a release, so the only judgement left is services and component severity; a minor or major release is a deliberate manual bump. validate_pr.py reads the body from the webhook payload, so it needs no credential and runs on fork pull requests. It has its own workflow so it can also trigger on 'edited': a body can change after checks go green, and apply reads the live body at merge time. Apply rejects unknown keys for the same reason, but tolerates missing ones, which is a concurrent pull request adding a component rather than tampering. The app token gets contents: write and not the Workflows permission, so a compromise of the apply job cannot rewrite the pipeline. Ports from the GitLab implementation, dropping its description-truncation fallback, the protected-variable split that fallback needed, and its url.insteadOf workaround — all GitLab Runner specifics. Also corrects the pull request template, which told contributors to hand-edit services/versions.json; the bot-owned path check rejects exactly that, so every pull request would have failed. Signed-off-by: Terve <ntervalon@nvidia.com>
Node and Go versions were duplicated between ci.yml and build.yml. GitHub Actions has no shared constants file — env: is per-workflow and one workflow cannot read another — so a local composite action is the only way to declare them once without moving them out of the repository into repository variables, where they would stop being reviewable. Bumping one copy and forgetting the other would have pull request checks and release builds running different toolchains, which is the failure this removes. Inputs keep what each job actually needs: the header check takes Node without an npm cache it has no install to populate, and the build-script job takes Go without Node. Checkout stays in the callers, since a local action cannot exist before its own repository is on disk. Signed-off-by: Terve <ntervalon@nvidia.com>
setup-go caches by default and looks for a dependency file at the repository root. Every Go module lives under services/, so the action found nothing and reported 'Dependencies file is not found' on each run — a warning in current versions, a hard failure in some earlier ones. Naming the path fixes that and makes the caching real: a cold build pulls roughly 40 seconds of modules, repeated across the services job, the three-platform build-script job, and the installer builds. It also pins the cache key, which is version-dependent otherwise: older setup-go hashes go.sum, newer hashes go.mod. Signed-off-by: Terve <ntervalon@nvidia.com>
The check asserted 'at least 13'. A floor stops asserting anything the moment the real number grows: add a fourteenth component and it keeps passing, so the check quietly becomes decoration. A hardcoded '-eq 13' fixes that but still only sees the count, and 13 is a magic number with no relationship to where components are actually declared. Compares the staged names against the component keys in versions.json instead. Adding, removing, or renaming a component now fails until the build script and the manifest agree, and the failure names which side is missing what rather than reporting a number. Signed-off-by: Terve <ntervalon@nvidia.com>
The product-to-services rename changed a line that verify-service-contracts generates into docs/services-api.md, but the generated file was not regenerated to match, so the freshness gate failed. Generated output only; produced by npm run service-contracts:write. Signed-off-by: Terve <ntervalon@nvidia.com>
The check compared two lists built by different pipelines, so a stray carriage return made every entry differ while printing identically. The failure output then showed the same names on both sides, which reads as nonsense rather than as a diagnosis. Both sides now run through one normalizer, in an order that matters: CR is stripped before the .exe suffix, because under Git Bash a CRLF line leaves .exe mid-string and 's/\.exe$//' never matches. LC_ALL=C makes the sort byte-wise, which is what the comm calls already assumed. On failure it also dumps the raw listing and both lists through 'sed -n l', so an invisible character shows up as \r instead of as two identical lists. Verified against a CRLF versions.json with .exe binaries, a POSIX checkout, a missing component, and an undeclared one. Signed-off-by: Terve <ntervalon@nvidia.com>
mkalkere
left a comment
mkalkere
left a comment
There was a problem hiding this comment.
Ran the release-intent scripts locally off this branch. test_lib.py: 27/28 (the 28th needs a repo checkout since it reads the real versions.json). validate_pr.py passes on this PR's body. apply_pr.py --dry-run with a release intent gives 0.1.1 -> 0.1.2, services 0.91.7 -> 0.91.8, package.json touched on one line, changelog entry lands in the right spot. Matches the validation section.
A few things:
-
apply_pr.py only applies the intent for the head commit's PR. If one push to develop ever carries two merges (direct push of a range, merge queue batch), the second intent is silently skipped and nothing fails. The description covers this for main cuts but develop has the same hole. Either iterate the push's commits or document that develop doesn't take batched pushes.
-
_parse_section ends a section at the first line starting with "### ". A changelog body containing a literal "### " line gets cut silently. Minor, a doc note would do.
-
The app plus RELEASE_INTENT_APP_ID / RELEASE_INTENT_APP_PRIVATE_KEY have to exist before the first push to develop, or the apply job fails. It fails loud (exit 2) and the README covers backfill, so not a defect, but I'd make it a pre-merge checkbox in the description.
-
The whole chain assumes branch protection requires the release-intent check. Admin-merging around it with a bad body fails loud at apply (exit 1) and versions just don't bump. Fine, but worth stating as an assumption.
Checked: macos-15-intel is a valid label (good until the macOS 15 image retires in fall 2027), so the mac-x64 leg will run. The product -> services rename covers all nine consumers including both sides of the IPC type, and dropping the installer key is deliberate and tested. Token scoping looks right: one-hour app token, contents:write, no Workflows permission, apply only on push to develop.
Three review findings. The apply workflow's concurrency group did not serialize applies, it dropped them. GitHub keeps one pending run per group and cancels any earlier one, and cancel-in-progress: false only protects the running job, not the queued one. Because an app installation token push does start workflow runs, the bot's own commit queues a third run that evicts a genuine merge waiting behind it — a lost bump with no failure anywhere. apply_pr.py already handles ordering by committing with force: false and re-reading a moved ref, so the group was an extra failure mode rather than belt and braces. Replaces it with a job-level guard that skips the bot's own pushes without booting a runner. The check workflow gated pull requests into main, but apply only runs on pushes to develop, so a pull request targeting main was required to declare bumps and then had them silently discarded. Restricting the check to develop also unblocks the release cut, whose develop -> main pull request carries the bot's own edits to the two paths check_forbidden_paths rejects. The installer path filter covered .github/workflows/** but not .github/actions/**, so a Node or Go bump in the composite action ran ci.yml on the new toolchain and skipped the six-way matrix. Signed-off-by: Terve <ntervalon@nvidia.com>
The manifest currency check compared the fingerprint, platform, arch, and component versions but not the services version, so a services-only bump left an existing manifest counting as current and the stale version was what the UI reported in dev. Verified: with no change the build skips as before; bumping only services now rebuilds and rewrites the manifest. Signed-off-by: Terve <ntervalon@nvidia.com>
AGENTS.md, CONTRIBUTING.md, and desktop/docs/services-backend.md all still told contributors to bump services/versions.json by hand, which CI now rejects. services/readme.md and the pull request template were updated when the rule landed; these three were missed. The internal job that used to enforce this is stripped from the public cut, so for public contributors the rule is new and the docs are the only thing announcing it. CONTRIBUTING.md also told contributors to branch from main and never named develop, walking them straight into a pull request whose declared bumps would be discarded on merge. Records four limits that were real but undocumented: apply resolves only the head commit's pull request, so develop must take one merge per push; the apply workflow must not gain a concurrency group; a literal '### ' line truncates a changelog body; and the gate assumes branch protection requires the check. Also notes that the app and its two settings must exist before the first push to develop. Signed-off-by: Terve <ntervalon@nvidia.com>
|
Thanks for running it end to end; matching numbers on the dry run is reassuring. All four are real. Three are now documented as known limits in scripts/release-intent/README.md (e035658) rather than fixed. Batched pushes — agreed, but since I'll be the one merging everything (along with the team) for now I think not worth the fixes to get there since we will be doing squash commits. '###' truncation — documented in the README and as a comment in the PR template, right next to the fence where an author would hit it. Credential before first push — added to the README's credential section, and I've already verified it, but merging is the definitive check. Branch protection assumption — stated in the README, including the admin-merge behaviour: fails loud at exit 1, versions simply don't bump. |
|
Verified the follow-ups — b0c3815, e035658, and 2d87948 all contain exactly what's described above. The diffs read clean. Documenting the four findings as known limits rather than fixing them is a reasonable call, and the batched-push deferral makes sense while merges stay manual. Only thing I'd keep an eye on: the batched-push case is the one I'd least want to rediscover the hard way, since the skip is silent. It's documented now, so that's a conscious trade rather than a trap — just flagging it in case merge-queue batching ever comes up. Thanks for the thorough response. |
develop unified ollama-proxy and lmstudio-proxy into nvpair-proxy, taking
versions.json from 13 components to 12 and moving product/installer to 1.2.0.
Conflicts were all the same collision: this branch renames product/installer
to a single services key, develop changed the component list underneath it.
Resolved by taking develop's components and this branch's key.
services/versions.json services: 1.2.0, develop's 12 components
services/build.sh V_SERVICES + nvpair-proxy
services/build.bat same
Three follow-ups git could not do:
- the pull request template's bump list had to move to the 12 new keys, or
every pull request would fail the strict key check
- services/nvpair-cluster-manager/spec.md, new on develop, told contributors
to bump installer and product by hand
- docs/services-api.md regenerated
The two clocks have not converged: product went to 1.2.0 while
desktop/package.json stayed 0.1.1, so the rename is if anything better
motivated than when it was written.
3 participants
Description
Moves checks and unsigned installer builds to GitHub Actions on GitHub-hosted
runners, and ports the release-intent automation that keeps version bumps and
changelog entries out of merge-conflict territory.
Self-hosted runners cannot serve a public repository: on a fork pull request
the workflow file comes from the contributor's merge ref, so
runs-onisattacker-chosen. Everything here runs on hosted runners and holds no secret,
so there is no credential for pull request code to reach. Signed releases stay
on internal infrastructure and are built from a tag.
Also renames the
versions.jsonfieldproducttoservices. It only everversioned the services suite — it stamps the standalone installer and Go
main.Version, while the version users install lives indesktop/package.json, which was explicitly out of scope for it. The two haddrifted to
0.91.7and0.1.1.Resolves #22
Release intent
Changelog title
n/a
Changelog body
n/a
Bumps
Scope
Included: workflows, the release-intent scripts, the
producttoservicesrename across all nine consumers, a changelog seeded from the published 0.1.0
and 0.1.1 release notes.
Excluded: signing, publishing, and release. Those stay on internal
infrastructure and are a separate piece of work.
Validation
Locally, on this branch:
python3 scripts/release-intent/test_lib.py— 28 testsnpm --prefix desktop run typecheck— cleannpm --prefix desktop run lint— 0 errorsnpm --prefix desktop run dead-code:check— no dead codenpm --prefix desktop run test:unit— 208 testsactionlinton all four workflows — cleannode scripts/spdx-headers.mjs— 0 missingapply_pr.py --dry-runproduces0.1.1 -> 0.1.2and a correctly numberedchangelog section
No workflow has executed before this pull request. The six-way build matrix is
unproven, and two legs are worth watching: Windows builds natively rather than
cross-compiled under wine, and
macos-latestis the smallest box in the matrixat 3 cores and 7 GB.
Risk
All bumps are
none: no Go source changed, so no compiled output changes andthis is not a release.
services/versions.jsonandCHANGELOG.mdare modified, which the newbot-owned path check rejects by design. The
allow-owned-filesmarker at thetop of this description is the documented override for a schema change the bot
cannot make itself.
Release intent applydoes not run on a pull request. It first runs on mergeto
develop, and needs the GitHub App configured before then or it will fail.Checklist
git commit -s).credentials, private data, internal URLs, internal issue identifiers, and
generated artifacts.