docs: reflect 6-agent runner pool + Server→Node rename residuals - #104
Merged
Conversation
Three things bled stale when the pool grew from 1→3→6 m3-ultra agents
and the Heavy job was renamed `Server + Shared Tests` → `Node + Shared
Tests`:
1. scripts/ci-runner/README.md still described a "single runner"
topology, including the explicit advice "add a single self-hosted
runner only (one concurrent job per repo) and let GitHub queue the
rest" — directly contradicted by the live pool. Rewritten:
- Title + intro pluralized; hardware-target section explains the
6-agent pool sharing one M3 Ultra host.
- New "Scaling out" section documenting the procedure used to add
dfx01-4/5/6 (registration-token reuse, tarball cache in /tmp,
SIGPIPE foot-gun with `set -o pipefail` + `head`).
- Operations section now uses `${RUNNER_DIR}` so snippets work for
any agent; added a pool-wide loop for status / cache wipe.
- Disk + RAM headroom rewritten with the measured budget table
(3 parallel jobs → ~14 GB cargo RSS, ~85 GB app memory, 0 swap;
6 parallel forecast → ~29 GB / ~95 GB / 0 swap).
- Naming-drift note for the legacy `actions-runner-zkcoins-server`
/ `…-zk-coins-server-N` directories that predate the repo rename.
- "Activating the CI jobs (historical)" section: branch protection
migrated develop → main, required-check list updated to the
current 4 contexts (incl. the Node + Shared Tests rename).
2. ci.yaml carried two stale comment blocks: the concurrency rationale
called M3 Ultra capacity "scarce" (single-runner framing) and the
node-tests job described itself as running on "the single self-
hosted M3 Ultra". Both reworded to reflect the 6-agent pool.
3. CONTRIBUTING.md and README.md referenced "the self-hosted M3 Ultra
runner" (singular) and the old `Server + Shared Tests` job name in
the CI/CD table + pre-push narrative. Pluralized and renamed; the
cargo invocation in README.md was also updated `cargo test` →
`cargo nextest run -p node -p shared --release --all-features
--test-threads=1` to match the workflow.
Branch protection on `main` was patched out-of-band today to rename
the required context `Server + Shared Tests (M3 Ultra)` → `Node +
Shared Tests (M3 Ultra)`; this commit aligns the docs with that
config change.
Senior-review pass on PR #104 flagged two BLOCKING gaps the original commit missed (the PR description claimed no non-historical `Server + Shared Tests` references would remain): - ROADMAP.md test-plan paragraph still cited the old job names. - program-plonky2/CONTRIBUTING.md described the CI gate as running `-p server -p shared` against `Server + Shared Tests` — both pieces stale post-rename. Also addressed the consistency findings: - scripts/ci-runner/README.md budget table prose contradicted itself (called 6 jobs a "forecast" while the prose explained that 3 PRs × 2 jobs already saturates the pool at 6 concurrent agents). Reworded to make clear the snapshot was captured under the pre-expansion 3-runner topology and the saturated column projects the linear envelope of the new 6-agent pool. - scripts/ci-runner/README.md:13 used "Six" (spelled-out) while every other reference uses the digit "6". Harmonised. - CONTRIBUTING.md, README.md, and the budget-table prose drifted on the `--test-threads` form (`=1` vs ` 1`) versus the actual ci.yaml command, and elided the `-E 'not binary(api_remote)'` test filter that the workflow uses. Quoted commands now match ci.yaml verbatim. - .github/workflows/ci.yaml sccache comment still read "The M3 Ultra runner is self-hosted" (singular) while the neighbouring updated comments now talk about the pool. Pluralised. GiB vs GB nit (`1 TB host` vs `>600 GiB free`) left as-is — matching the conventional usage (TB / GB for disk capacity, GiB for sccache cap).
…ne budget table Second senior-review pass on PR #104 flagged a handful of consistency nits left after the first fix-up commit: - scripts/ci-runner/README.md `## Tracking` section was still singular ("The runner is a launchd service") — pluralised to "Each agent". - program-plonky2/CONTRIBUTING.md still linked to PRs/issues under the pre-rename `zk-coins/server` org URL (PR #17, PR #48, issue #50). GitHub auto-redirects, but the cross-link to the top-level CONTRIBUTING.md inconsistency was distracting — all three rewritten to `zk-coins/node` to match the rest of the docs. - .github/workflows/ci.yaml had three remaining singulars: the Docker-socket comment on both node-tests + coverage jobs ("The M3 Ultra runner on dfx01 runs Colima"), the sccache cap comment ("every m3-ultra runner on the host; with 3+ parallel runners"), and the concurrency block's last line ("free the runner"). All four reworded to use the pool / per-agent framing consistently. - scripts/ci-runner/README.md budget table: - Prose said "linearly projects to 6", but the App-memory row goes ~85 GB → ~95 GB, which is bounded by the 96 GB host RAM ceiling, not a linear extrapolation. Reworded to call out the cache-bound ceiling. - "CPU cores in use: 3 of 28 → 6 of 28" misleadingly implied an idle box; renamed to "Active test processes (`--test-threads 1`)" with cell content stripped of the misleading "of 28 cores" framing. - ci.yaml:245 comment still spelled `--test-threads=1` (equals form) while the actual command on :256 uses ` 1` (space form). Aligned. Out-of-scope (flagged in review, intentionally not touched in this PR): - deploy-dev.yaml + deploy-prd.yaml still use singular "the self-hosted M3 Ultra runner" framing. Not in the PR's stated scope; carry as a follow-up if the framing becomes confusing.
…rs-and-job-rename # Conflicts: # program-plonky2/CONTRIBUTING.md
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.
Summary
Documentation cleanup after two recent infra changes that left stale references behind:
dfx01,dfx01-2..dfx01-6).Server + Shared Tests→Node + Shared Tests. Workflow file was updated; docs + branch protection lagged behind.Branch protection on
mainwas already patched today (out-of-band, viagh api) to rename the required-check context. This commit aligns the docs with that config change.Findings audited (subagent pass) and resolved
scripts/ci-runner/README.md(~150 LOC of changes)dfx01-4/-5/-6(token-reuse, tarball cache in/tmp, SIGPIPE foot-gun when looping withset -o pipefail+head).${RUNNER_DIR}so snippets work for any agent; added pool-wide loop helpers (status / cache wipe).actions-runner-zkcoins-server(no hyphen) and…-zk-coins-server-Ndirectory names that predate the repo rename..github/workflows/ci.yamlnode-testsjob header: "single self-hosted M3 Ultra" → "one of 6 agents on dfx01 sharing the host's 96 GB / 28 cores".node-tests+coverageoverview comment: pluralized.CONTRIBUTING.mdServer + shared tests→Node + shared tests.ci.yaml (Server + Shared Tests)→(Node + Shared Tests); command updatedcargo test -p node -p shared→cargo nextest run -p node -p shared --release --all-features --test-threads=1to match the workflow.README.mdcargo test --all-features→cargo nextest run -p node -p shared --release --all-features --test-threads=1; "the self-hosted M3 Ultra runner" → "the self-hosted M3 Ultra runner pool".Out of scope (deliberately deferred)
dfx01/dfx01-2/dfx01-3to refresh the local.runner.gitHubUrl(still points atzk-coins/serverfrom before the rename) and to migrate the launchd plist label fromactions.runner.zk-coins-server.*to…zk-coins-node.*. GitHub silently routes correctly after a rename — drift is metadata-only. Plan separately as an operational task.actions-runner-zkcoins-server(no hyphen, dfx01) →actions-runner-zk-coins-node-1for naming-pattern consistency. Bundle with the re-register cycle above.Test plan
ci:fullnot required for review)Server + Shared Tests,server-tests, orsingle self-hosted M3 Ultrareferences in non-historical contextServer + Shared Tests (M3 Ultra)required check