ci: run the GitHub Actions checks on CircleCI's self-hosted runner - #193
ci: run the GitHub Actions checks on CircleCI's self-hosted runner#193marvin-bot-coasys wants to merge 6 commits into
Conversation
Translates .github/workflows/build.yaml and ad4m-compat.yaml to .circleci/config.yml on `coasys/marvin`, the machine runner that already builds AD4M. Step one toward WE tests against a real AD4M executor, which a hosted runner cannot have. Same six jobs, same names, same dependency shape: lint and rust need nothing built; typecheck, test and schemas take the build's result rather than rebuilding it. Mapping, where CircleCI has no equivalent of an action: - setup-node -> nvm, reading the same .nvmrc - pnpm/action-setup -> corepack, reading the same packageManager field - actions/cache -> restore_cache/save_cache on the pnpm store - upload/download-artifact -> persist_to_workspace/attach_workspace Two things the runner changes, both handled rather than inherited: Machine runners only support `machine: true` — a `docker:` executor needs CircleCI's container runner — so every job runs on the host as `marvin`. The working directory persists between jobs (cleanup_working_directory: false), and `checkout` only resets tracked files. build.yaml's drift check reads `git status --porcelain`, which on a hosted runner is pristine by construction and here is not: a leftover untracked file fails the check on somebody else's mess, or hides real drift by already being there. Hence clean_checkout — `git clean -xfd` before and after checkout, and a hard failure if the tree is still dirty, so the check means the same thing it means upstream. `concurrency: cancel-in-progress` has no config equivalent; it is the project's "Auto-cancel redundant workflows" setting. The nightly compat job carries two documented gaps against ad4m-compat.yaml: it tests `dev` HEAD rather than the last commit green on `dev` (that control needs run history this project does not have yet), and it reports into the build log rather than a tracking issue (which needs a GH_TOKEN project variable). Both are noted in the file.
✅ Deploy Preview for coasys-we ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…irty
The runner is shared with AD4M, whose jobs leave .hc-toolchain/ behind — a
directory with its own .git. A single -f skips nested repositories
('Skipping repository .hc-toolchain/src'), so the tree stayed dirty and the
guard refused to let the drift check run against it, which is what it is for.
… need it save_cache, persist_to_workspace and store_artifacts all upload to CircleCI and are charged even when the compute is free, which makes a self-hosted runner the expensive kind of free. Every job here runs on one host, so all three were paying to move bytes that never had to leave the machine. - pnpm store: no cache steps at all. It lives in ~/.local/share/pnpm/store, outside the repo, so git clean -xffd cannot reach it and it is already warm (6.8G on disk). The GitHub workflow caches it because a hosted runner starts empty; this one does not. - build output: handed between jobs through $CI_ARTIFACT_DIR/we/$CIRCLE_SHA1, the same local-stash mechanism ad4m's config already uses for its binaries, plus the same one-day sweep so it cannot grow forever. - compat.log: kept on the runner, tail printed into the failing build log. Also faster: no upload or download on either side.
WE shared coasys/marvin with AD4M for one afternoon, which was enough to show why it should not. Both projects land in the same runner working directory, so WE's clean_checkout wiped AD4M's cached build trees out of all four of them — and AD4M's leftovers had already broken WE's first run. Four dedicated runner instances (marvin-we1..we4), each with its own working directory under /home/marvin/we-ci-workdir-N. Same hardware, separate pools, no shared directory to fight over. Sizing is deliberate rather than symmetric: lint and build each set --max-old-space-size=8192, so four concurrent WE jobs is the honest ceiling on a 59GB box that is also running AD4M's four. Grow it after measuring peak usage, not before. The -ff in git clean stays. The dedicated class removes that particular neighbour, but WE's own tooling is free to write a nested repo into an ignored directory, and this guard should not have to be rediscovered the day it does.
…ks now build.yaml and ad4m-compat.yaml drop their automatic triggers. The same six jobs and the same nightly now run on CircleCI against the self-hosted coasys/marvin-we pool, and running both would mean every push paying twice for the same answers and two check lists to read on every PR. Deactivated, not deleted, and the distinction is deliberate. Those files carry the reasoning the CircleCI translation was built from — why there are five jobs and not one, why the drift check has no exclusions, why the build outputs are derived from git status --ignored rather than a path list, why the nightly tests the last green commit rather than HEAD. Deleting the files would delete the argument and keep only the result. workflow_dispatch stays on both. If CircleCI or the runner host is unavailable this is the fallback, and a fallback that has never been runnable is not one. It also remains the way to run a WE branch against an AD4M branch carrying an unpublished API. Each file says what to restore to reactivate it.
jhweir
left a comment
jhweir
left a comment
There was a problem hiding this comment.
Review — #193 ci: run the GitHub Actions checks on CircleCI's self-hosted runner
Reviewed the full .circleci/config.yml against the two workflows it translates, and checked the
claims below against the Actions API, the commit status API and the PR list.
The translation is good, and most of what follows is not about it. clean_checkout is the right
insight about persistent working directories, and the -ff nested-repo reasoning is the kind of
thing only learned the hard way. Declining CircleCI network storage on a self-hosted runner is a
real cost observation most translations would miss. I ran the packaging pipeline
(git status --porcelain --ignored=traditional | awk | grep) against the tree: identical to
upstream's, the node_modules exclusion leaves zero survivors, and there are no space-containing
ignored paths to trip the $2.
The six checks can move now. The nightly cannot, for reasons that are mostly not this PR's fault,
and the forked-PR question needs an answer before merge. The rest is a stale description and a
handful of small things. A proposed split is at the end.
The description predates the last four commits
The body was written for the first commit and not updated. Four places now disagree with the diff:
- It says the jobs run on
coasys/marvin. They run oncoasys/marvin-we, and the config's own
header explains why that had to change. - The table maps
actions/cachetorestore_cache/save_cacheandupload-artifactto
persist_to_workspace.adc425e6removed all four, deliberately, and the config now argues
against using them. - "Before this can run" says the project does not exist. It exists, it has run, and all six jobs
are green on this PR. - "This does not remove the GitHub Actions workflows. Both run until CircleCI has proved itself
green here." The last commit deactivates both:build.yamllosespushandpull_request,
ad4m-compat.yamlloses itsschedule. There is no overlap period. The check list on this PR is
CircleCI and Netlify only.
The last one matters most, because the overlap is the risk mitigation a reviewer would approve on
and it is not in the change. But for the six checks the diff is right and the sentence is wrong.
build.yaml's own comment makes the case: two check lists on every PR, able to disagree, is worse
than one. CircleCI is already green here, so it is not unproven. dev has no branch protection and
no rulesets (checked), so a CircleCI outage degrades to "nobody notices a bug for a day" rather than
a blocked repo. workflow_dispatch stays, so the manual fallback for the checks genuinely works.
So: rewrite the body to describe the change being made. The checks move, workflow_dispatch is the
manual fallback, the workflow files are kept for the reasoning they carry, and the runner pool is
marvin-we.
Blocking: the nightly
The word to be careful with is "fallback". It is accurate for build.yaml and not for
ad4m-compat.yaml, and the reason is worse than the frozen-control problem I started from.
It is already red, for its own reason
The GitHub nightly has failed every scheduled run since at least 31 August, and the last green run
was 30 August. Since 7 September the failure is self-inflicted. The workflow rewrites the pin to
file:./ad4m/core and then runs pnpm test, and 81447543 added a test that asserts the pin
names a published version:
packages/backend-system/ad4m/tests/ad4mAdapter.test.ts:195
AssertionError: a `file:` link names no build, so the profile is unverifiable — the pin must
name a published version before merge: expected 'file:./ad4m/core' to be '0.13.0-test-model-layer'
Issue #176 is open, updated nightly, and currently says "AD4M is the difference". It is not. That
assertion is on my side to fix — the compat job needs a way past it that does not weaken the gate —
and it is not this PR's problem. What is this PR's problem is what it does around that state:
- The CircleCI translation performs the same override and runs the same tests, so it will fail
every night for the same reason, into a build log, with noGH_TOKENto file anything and no
notification configured. - Removing the GitHub
schedulefreezes #176 open. Only the scheduled path closes it, and the
CircleCI job cannot.
The fallback that misreports
Separately: ad4m-compat.yaml resolves its control by asking for the last successful
build.yaml run on dev:
repos/coasys/we/actions/workflows/build.yaml/runs?branch=dev&status=success&per_page=1
Once build.yaml stops running on push, that query does not fail and does not return empty. It
keeps returning the last green run from before the merge, indefinitely — 821ef763, as of today.
A manual dispatch would test a frozen SHA that gets staler every week, while setting
controlled=true and reporting "last commit green on dev". The else branch that warns and falls
back to HEAD is unreachable: there is a green run, it is just old. That defeats the one thing
controlled exists for.
The control does not need CircleCI run history
The config says the CircleCI equivalent of the control "needs this project's own run history,
which does not exist until this config has been running for a while". That premise is wrong, and
dropping it makes the whole thing simpler.
CircleCI posts a commit status to GitHub for every job. This PR's head commit carries six of them
(ci/circleci: build, lint, rust, schemas, test, typecheck, all success), and so will
every push to dev once this merges. So "the last commit green on dev" is answerable from
GitHub's own commit status API, from either runner, today: walk dev's commits and take the first
whose ci/circleci:* statuses are all success. The GitHub workflow already has the token; the
CircleCI job needs none on a public repo. That is the long-term answer the config defers, and it
also fixes the frozen-control problem in build.yaml for any future dispatch.
What to do
Keep ad4m-compat.yaml's schedule in this PR. A red nightly that files a wrong issue is bad; a
red nightly nobody is told about is worse, and the wrong issue at least keeps #176 alive until it can
be corrected. Move the nightly to a follow-up PR that does three things: resolves the control from
commit statuses on both runners, gives the CircleCI job a channel that reaches a person (the
GH_TOKEN project variable, or a scheduled pipeline whose failure notifies followers), and closes
or corrects #176. The pin-assertion fix on my side lands before or alongside it.
What to avoid is the position the PR currently takes: off, and described as available.
Needs an answer before merge: forked pull requests
coasys/we is public. Every job here runs directly on Coasys's hardware as marvin, with
~/.cargo, ~/.nvm, the pnpm store and $CI_ARTIFACT_DIR, on the same machine that builds AD4M.
CircleCI's Build forked pull requests defaults to off, and I cannot check the project setting
from here, so this is a question rather than a confirmed defect. But it is not hypothetical:
- Nine PRs in this repo's history came from a fork — eight of them HexaField's (#2, #3, #4, #5,
#53, #65, #71, #78, the last in June), one fromdata-bot-coasys(#10). - With the setting off, those PRs get no CI at all once
pull_requestis gone from
build.yaml. GitHub Actions ran on them with a restricted token; CircleCI will not run at all. - With the setting on, they are arbitrary code from a stranger executing on that machine.
Neither is a default to fall into. The clean answer is to keep pull_request on build.yaml with
each job gated on github.event.pull_request.head.repo.fork: forks get hosted CI and never touch
the runner, same-repo branches get CircleCI, and no PR has two check lists. Whatever is decided,
name the setting and its verified value in the config's settings comment block, next to auto-cancel.
Smaller things
-
The
ciworkflow has no branch filters, so it runs on every push to every branch, where
build.yamlran onpull_requestplus pushes todev. Config filters cannot express that
shape. The equivalent is the project setting Only build pull requests, which also always
builds the default branch. It belongs in the same comment block as auto-cancel, and it matters on
a four-instance pool. -
CircleCI builds the branch head, not the merge ref.
pull_requeston Actions checked out
refs/pull/N/merge, the PR as it would be after merging intodev. CircleCI checks out
CIRCLE_SHA1, the PR as it is. With nothing forcing branches up to date, a PR green on its own
can breakdevon merge and nothing will have said so. Not in the "Same shape" table; worth a
line there, and a line in the config. -
triggers:/schedule:is CircleCI's deprecated scheduled-workflow syntax. The comment
acknowledges it as a placeholder. CircleCI announced a sunset for end of 2022 and then postponed
it with no new date, so it works, but it is worth an issue rather than a comment — and if the
nightly moves to a scheduled pipeline in the follow-up, that pipeline's failure notifications are
the reporting channel the job is missing. -
"Full log kept on the runner at …" in the compat Report step is undercut by the
build
job's sweep of the same directory, which removes anything older than a day. Either exempt
compat-*.logfrom the sweep or say "for a day". -
restore_buildassumes all four runner instances share one physical host. It fails loudly
if that stops being true, which is the right failure mode, but the assumption belongs next to the
executor definition as a constraint on future changes, not only as an aside about
persist_to_workspace. -
"Node from .nvmrc" is copied three times. The copies in
rustandad4m-compatdo not
. "$BASH_ENV"the waysetup's does. That works today because CircleCI sources it between
steps, and breaks the moment someone callsnodein the same step. Extract a fourth command.
Proposed split
- This PR: the six checks. Correct the description. Restore
ad4m-compat.yaml'sschedule.
Keeppull_requestonbuild.yamlgated to forks. Add Only build pull requests, the
forked-PR verdict, and the head-versus-merge note to the settings comment block. The small items
above as you see fit. - Follow-up PR: the nightly. Control from commit statuses on both runners, a channel that
reaches a person, and #176 closed or corrected. Then deactivate the GitHub schedule. - On my side, before 2: the pin assertion, so a source build can pass the compat job without
the gate losing the check.
3 participants
Translates
.github/workflows/build.yamlandad4m-compat.yamlinto.circleci/config.yml, running oncoasys/marvin— the self-hosted machine runner that already builds AD4M.Step one toward WE tests against a real AD4M executor: a hosted runner cannot hold an executor, a Holochain conductor and a bootstrap server between jobs; that box already does.
This does not remove the GitHub Actions workflows. Both run until CircleCI has proved itself green here.
Same shape as build.yaml
Six jobs, same names, same dependency graph.
lintandrustneed nothing built;typecheck,testandschemastake the build's result rather than rebuilding it.actions/setup-node(node-version-file: .nvmrc)nvm, reading the same.nvmrcpnpm/action-setup(no version →packageManager)corepack, reading the samepackageManageractions/cacheon the pnpm storerestore_cache/save_cacheupload-artifact/download-artifactpersist_to_workspace/attach_workspace.github/actions/setupcommands: setup.github/actions/restore-buildcommands: restore_buildWhat the self-hosted runner changes
No
docker:executor. Machine runners only supportmachine: true— adocker:executor needs CircleCI's container runner (Kubernetes). Every job runs on the host asmarvin.The working directory persists (
cleanup_working_directory: false), andcheckoutonly resets tracked files. That matters for exactly one check, and it matters silently:build.yaml's drift check isgit status --porcelain, which reports untracked files. On a hosted runner the tree is pristine by construction. Here it is not — a file left by an earlier job is indistinguishable from one this build generated, so the check either fails on somebody else's mess or, worse, passes on genuine drift because the file was already there.Hence
clean_checkout:git clean -xfdbefore and aftercheckout, and a hard failure if the tree is still dirty. It costs anode_modulesreinstall per job, paid back by the pnpm store cache, which lives in the runner's home directory rather than the repository.Needs a project setting, not config
concurrency: cancel-in-progresshas no config equivalent. It is Project Settings → Advanced → Auto-cancel redundant workflows, and it should be on to match the GitHub behaviour.Nightly compat job: two documented gaps
ad4m-compatis translated but is not yet at parity, and the file says so rather than pretending:dev" from the Actions API and tests that, which is what makes a failure attributable to AD4M by construction. The CircleCI equivalent needs this project's own run history, which will not exist until this has been running a while. Until then it testsdevHEAD — upstream's documentedcontrolled=falsefallback.gh issue create/edit/closeneeds a token withissues: write. That is a project environment variable (GH_TOKEN), not something this file can carry. Without it the job reports into the build log and storescompat.logas an artifact.Before this can run
gh/coasys/wedoes not exist as a CircleCI project yet (/api/v2/project/gh/coasys/we→ 404), and for a GitHub-OAuth org there is no API to create one. It needs Set Up Project once in the CircleCI UI, pointed at this branch's config. Everything else is in this file.circleci config validatepasses.