-
Notifications
You must be signed in to change notification settings - Fork 0
Move non-compile CI jobs to GitHub-hosted runners #322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: fix/rustsec-wasmtime-rkyv
Are you sure you want to change the base?
Changes from all commits
db44a91
0e2bc88
c8eda3e
f872cb0
1bf74b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Paths hidden from `fd`, and therefore from the estate-wide `mdformat-all` | ||
| # formatter, which discovers Markdown with `fd` and pipes it through | ||
| # `mdtablefix --wrap`. | ||
| # | ||
| # `mdtablefix` has no exclusion flag, so a file it must not touch has to be | ||
| # withheld at discovery. `markdownlint-cli2` already skips the paths below via | ||
| # `.markdownlint-cli2.jsonc`; this file closes the other half of `make fmt`. | ||
| # | ||
| # This is deliberately `.fdignore` rather than `.ignore`: ripgrep does not read | ||
| # it, so these paths stay searchable. | ||
|
|
||
| # Expected-output fixtures for the html_to_markdown converter tests. Their | ||
| # content is a byte-for-byte record of converter output, which does not wrap | ||
| # prose. Reflowing them to 80 columns makes | ||
| # `convert_test_pages_to_markdown` fail against a fixture that no longer | ||
| # describes any real conversion. | ||
| tests/test-pages/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| # ADR-013 — Split CI runners by compile cost | ||
|
|
||
| **Status:** Accepted **Date:** 2026-08-05 **Deciders:** `@leynos` | ||
|
|
||
| ## Context | ||
|
|
||
| Every Linux job in the repository ran on `ubicloud-standard-8`, an 8-vCPU paid | ||
| runner chosen because a full workspace build is the dominant CI cost. That | ||
| choice was applied uniformly rather than per job, so it also covered work that | ||
| never compiles anything: the two pull-request labelling workflows, the | ||
| regression-test check, the Claude Code review, and the weekly dependency | ||
| audit. Those jobs are single-threaded shell scripts, an action call, or an | ||
| API-bound agent run; the extra vCPUs sit idle. | ||
|
|
||
| A July 2026 Ubicloud usage audit attributed roughly 1,600 billed premium-8 | ||
| minutes per month to those five workflows. Axinite is a public repository, so | ||
| GitHub-hosted `ubuntu-latest` runners execute the same work at no cost, and | ||
| the standard 2-vCPU hosted runner is not the bottleneck for any of them. | ||
|
|
||
| ## Decision | ||
|
|
||
| Select the runner per job, from the job's compile cost: | ||
|
|
||
| - A job that compiles the workspace — `cargo build`, `cargo test`, | ||
| `cargo nextest`, `cargo clippy`, `cargo llvm-cov`, `cargo component`, or the | ||
| `make` targets that wrap them — runs on `ubicloud-standard-8`. | ||
| - A job that does not compile runs on GitHub-hosted `ubuntu-latest`. | ||
|
|
||
| The five non-compiling workflows moved accordingly. Windows jobs keep | ||
| `windows-latest` and the release workflow keeps its pinned `ubuntu-22.04` | ||
| images, both for reproducibility rather than cost. | ||
|
|
||
| The policy is enforced by `tests/workflow_contracts/runner_policy_test.py`, | ||
| which records the runner for every job in the repository. Adding a job, or | ||
| moving one between pools, fails until the recorded policy is updated | ||
| deliberately. The same suite asserts that no job on the free pool runs a | ||
| compile command or installs a Rust build cache. | ||
|
|
||
| ## Rationale | ||
|
|
||
| Runner selection is a per-job property, not a per-repository one. Paying for | ||
| vCPUs a job cannot use is waste with no compensating benefit, and the | ||
| alternative — leaving everything on the paid pool because it is simpler — | ||
| costs roughly 1,600 billed minutes a month for jobs whose wall-clock time is | ||
| dominated by network round trips and process startup. | ||
|
|
||
| Encoding the rule as a contract test rather than a comment matters because the | ||
| failure mode is silent: a new job copied from an existing workflow inherits | ||
| whichever runner the template used, and nothing surfaces the mistake until the | ||
| next billing audit. A test that enumerates every job turns that into a | ||
| review-time question. | ||
|
|
||
| `release-plz.yml` is deliberately untouched. Its jobs are gated to the `nearai` | ||
| repository owner and never execute here, so moving them would change nothing | ||
| observable while diverging from upstream. | ||
|
|
||
| ## Consequences | ||
|
|
||
| - Non-compile pull-request feedback moves to the free pool, so it competes for | ||
| GitHub's shared hosted-runner concurrency rather than Ubicloud's. These jobs | ||
| run on every pull-request event, so any queueing regression is visible | ||
| immediately. | ||
| - Adding a workflow or a job now requires an edit to `RUNNER_POLICY` in | ||
| `tests/workflow_contracts/runner_policy_test.py`. That is the intended | ||
| friction: the runner choice becomes an explicit review decision. | ||
| - A free-runner job that later grows a build step fails its contract test | ||
| rather than silently running a compile on a 2-vCPU machine. | ||
|
|
||
| ## Alternatives considered | ||
|
|
||
| - **Leave everything on Ubicloud.** Simplest, and wrong: it keeps paying for | ||
| capacity that five workflows demonstrably cannot use. | ||
| - **Move every Linux job to `ubuntu-latest`.** Free, but the workspace build | ||
| is the reason the paid pool exists; hosted runners lack both the vCPUs and | ||
| the disk headroom the coverage and end-to-end jobs need. | ||
| - **Document the split without a test.** Rejected for the silent-inheritance | ||
| failure mode described above. | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -76,6 +76,36 @@ that assert on printed startup or boot-screen content — for example, the | |||||||||||||||||||
| `print_startup_info_matches_snapshot` test in `src/startup/boot.rs`. The crate | ||||||||||||||||||||
| is compiled only when running tests and has no effect on the production binary. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ### Runner selection | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Runners are chosen per job, from the job's compile cost: | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - A job that compiles the workspace runs on `ubicloud-standard-8`. That covers | ||||||||||||||||||||
| `test.yml`, `code_style.yml`, `coverage.yml`, `codescene-coverage.yml`, | ||||||||||||||||||||
| `e2e.yml`, and `staging-ci.yml` — anything invoking `cargo build`, | ||||||||||||||||||||
| `cargo test`, `cargo nextest`, `cargo clippy`, `cargo llvm-cov`, | ||||||||||||||||||||
| `cargo component`, or the `make` targets that wrap them. | ||||||||||||||||||||
| - A job that does not compile runs on GitHub-hosted `ubuntu-latest`, which is | ||||||||||||||||||||
| free for this public repository. That covers `pr-label-classify.yml`, | ||||||||||||||||||||
| `pr-label-scope.yml`, `regression-test-check.yml`, `claude-review.yml`, and | ||||||||||||||||||||
| `audit.yml`. These are single-threaded shell scripts, action calls, or | ||||||||||||||||||||
| API-bound agent runs; the scheduled audit installs `cargo-audit` as a | ||||||||||||||||||||
| prebuilt binary and only reads the lockfile. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Windows jobs use `windows-latest` and `release.yml` pins `ubuntu-22.04`, both | ||||||||||||||||||||
| for reproducibility rather than cost. `release-plz.yml` is left on Ubicloud | ||||||||||||||||||||
| because its jobs are gated to the `nearai` repository owner and never execute | ||||||||||||||||||||
| here. | ||||||||||||||||||||
|
Comment on lines
+95
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Correct the The text states that Also consider naming job scope for the audit. ♻️ Proposed fix-Windows jobs use `windows-latest` and `release.yml` pins `ubuntu-22.04`, both
-for reproducibility rather than cost. `release-plz.yml` is left on Ubicloud
-because its jobs are gated to the `nearai` repository owner and never execute
-here.
+Windows jobs use `windows-latest`. `release.yml` pins `ubuntu-22.04` for every
+job except `build-local-artifacts`, which selects its runner from the release
+matrix. Both choices serve reproducibility rather than cost. `release-plz.yml`
+is left on Ubicloud because its jobs are gated to the `nearai` repository owner
+and never execute here.Triage: 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| `tests/workflow_contracts/runner_policy_test.py` records the runner for every | ||||||||||||||||||||
| job in the repository and fails when the mapping drifts, so adding a job or | ||||||||||||||||||||
| moving one between pools requires a deliberate edit to `RUNNER_POLICY`. The | ||||||||||||||||||||
| same suite asserts that no job on the free pool runs a compile command or | ||||||||||||||||||||
| installs a Rust build cache. Run it with `make test-workflow-contracts`. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| The rationale is recorded in | ||||||||||||||||||||
| [ADR 013](adr-013-split-ci-runners-by-compile-cost.md). | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ### Workflow pins and Dependabot | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Dependabot owns the upgrade of GitHub Actions and reusable workflows, including | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| //! Auth detection tests. | ||
|
|
||
| use anyhow::Context as _; | ||
|
|
||
| use super::super::{check_auth_required, parse_auth_result}; | ||
| use super::*; | ||
|
|
||
|
|
@@ -13,21 +15,25 @@ fn check_auth_json(tool_name: &str, json: serde_json::Value) -> Option<(String, | |
|
|
||
| /// Assert that an auth-awaiting detection result is `Some`, and that the | ||
| /// returned name and instructions match the expected values. | ||
| /// | ||
| /// Returns an error when detection did not fire, leaving the calling test to | ||
| /// decide how that failure surfaces. | ||
| fn assert_auth_detected( | ||
| detected: Option<(String, String)>, | ||
| expected_name: &str, | ||
| expected_instructions_fragment: &str, | ||
| ) { | ||
| ) -> anyhow::Result<()> { | ||
| assert!(detected.is_some(), "expected auth detection to fire"); | ||
| let (name, instructions) = | ||
| detected.expect("expected auth detection to fire and return (name, instructions)"); | ||
| detected.context("expected auth detection to fire and return (name, instructions)")?; | ||
| assert_eq!(name, expected_name); | ||
| assert!( | ||
| instructions.contains(expected_instructions_fragment), | ||
| "instructions did not contain {:?}: {:?}", | ||
| expected_instructions_fragment, | ||
| instructions, | ||
| ); | ||
| Ok(()) | ||
|
Comment on lines
+18
to
+36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Return precondition failures through the helper result. Remove the assertion that precedes error conversion in each helper. Each assertion panics before the later
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| #[test] | ||
|
|
@@ -45,7 +51,8 @@ fn test_detect_auth_awaiting_positive() { | |
| ), | ||
| "telegram", | ||
| "Telegram Bot API", | ||
| ); | ||
| ) | ||
| .expect("expected auth detection to fire"); | ||
|
Comment on lines
+54
to
+55
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Propagate helper errors from all affected agent tests. Replace the newly added
As per path instructions, 📍 Affects 5 files
🤖 Prompt for AI AgentsSource: Path instructions |
||
| } | ||
|
|
||
| #[test] | ||
|
|
@@ -129,7 +136,8 @@ fn test_detect_auth_awaiting_tool_activate() { | |
| ), | ||
| "slack", | ||
| "Slack Bot", | ||
| ); | ||
| ) | ||
| .expect("expected auth detection to fire"); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: leynos/axinite
Length of output: 4415
🏁 Script executed:
Repository: leynos/axinite
Length of output: 4969
Add the required ADR sections.
Use
## Status,## Date, and## Context and Problem Statement. The ADR currently combines Status and Date in one metadata line and uses## Context.Triage:
[type:docstyle]🤖 Prompt for AI Agents
Source: Coding guidelines