Repository navigation
ci(nightly): admit Linux controls on fleet before paid overflow - #30085
Conversation
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base 73dd28e; does not block Shipped in:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/test_nightly_linux_admission.py">
<violation number="1" location="tests/test_nightly_linux_admission.py:58">
P3: The `urllib.request.urlopen` mock does nothing here because `exchange` goes through `build_opener(...).open`. Patch `transport.urllib.request.build_opener`, or assert that the opener is never built, so the test checks the no-network guarantee it names.</violation>
</file>
<file name="tests/test_ci_nightly_off_blacksmith.py">
<violation number="1" location="tests/test_ci_nightly_off_blacksmith.py:223">
P2: The new `endswith("|| 'cmux-fleet-linux-x64' }}")` clause can never be true: in nightly.yml the fleet literal is mid-expression and every job ends in `'blacksmith-32vcpu-ubuntu-2404' }}`. The older Blacksmith disjunct still passes these jobs, so the test no longer checks the fleet route. Match the fleet literal inside the parenthesized early-runner expression instead.</violation>
</file>
<file name="scripts/ci/nightly_linux_admission.py">
<violation number="1" location="scripts/ci/nightly_linux_admission.py:130">
P1: This branch routes unplaced nightly jobs to Blacksmith after a receipt, violating the repository’s explicit no-Blacksmith nightly rule. Keep the fallback on the owned fleet unless that policy changes.</violation>
</file>
<file name=".github/workflows/ci-guards.yml">
<violation number="1" location=".github/workflows/ci-guards.yml:920">
P2: The admission test shares a `run` step with the placement test, so a placement-test failure exits the shell before the admission test runs. Give it a separate `!cancelled()` step so both guard failures are reported.</violation>
</file>
<file name=".github/workflows/nightly.yml">
<violation number="1" location=".github/workflows/nightly.yml:827">
P2: This always requests a `signer-build` lease even when the downstream signer job is disabled (the `sign_pool` input defaults to false). Omit jobs whose `if` conditions are false so an unused lease cannot consume fleet capacity or hold up admission for the controls that will run.</violation>
</file>
<file name="tests/test_ci_portable_job_placement.py">
<violation number="1" location="tests/test_ci_portable_job_placement.py:267">
P2: This change drops the wiring guard that `signer-build` never routes to a Mac mini. The signer artifact must stay on Linux, so keep that assertion for this job.</violation>
<violation number="2" location="tests/test_ci_portable_job_placement.py:267">
P3: The test name still promises a check that Blacksmith comes after the fleet, but the Blacksmith assertion was removed. Either restore an ordering assertion against `'blacksmith-` or rename the test to match what it now checks.</violation>
</file>
<file name="tests/test-execution.toml">
<violation number="1" location="tests/test-execution.toml:1427">
P2: This registers the new test in the `linux-guard` lane, which runs it in CI. AGENTS.md forbids adding CI tests, and recent additions use `lane = "manual"` with a `reason` instead. Mark it manual and run it on the fleet yourself.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| self.assertTrue("needs.decide.outputs.early_" in runs_on | ||
| and runs_on.endswith("|| 'cmux-fleet-linux-x64' }}") |
There was a problem hiding this comment.
P2: The new endswith("|| 'cmux-fleet-linux-x64' }}") clause can never be true: in nightly.yml the fleet literal is mid-expression and every job ends in 'blacksmith-32vcpu-ubuntu-2404' }}. The older Blacksmith disjunct still passes these jobs, so the test no longer checks the fleet route. Match the fleet literal inside the parenthesized early-runner expression instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/test_ci_nightly_off_blacksmith.py, line 223:
<comment>The new `endswith("|| 'cmux-fleet-linux-x64' }}")` clause can never be true: in nightly.yml the fleet literal is mid-expression and every job ends in `'blacksmith-32vcpu-ubuntu-2404' }}`. The older Blacksmith disjunct still passes these jobs, so the test no longer checks the fleet route. Match the fleet literal inside the parenthesized early-runner expression instead.</comment>
<file context>
@@ -220,7 +220,9 @@ def test_light_nightly_overflow_stays_on_linux_without_a_fleet_pick(self) -> Non
# The sign pick takes GitHub-hosted Ubuntu, then Blacksmith Linux 4 vCPU on a re-run
# (tests/test_nightly_trusted_routing.py, the signer route).
- self.assertTrue("vars.LINUX_RUNNER == 'blacksmith-16vcpu-ubuntu-2404' && 'blacksmith-16vcpu-ubuntu-2404' || " in runs_on
+ self.assertTrue("needs.decide.outputs.early_" in runs_on
+ and runs_on.endswith("|| 'cmux-fleet-linux-x64' }}")
+ or "vars.LINUX_RUNNER == 'blacksmith-16vcpu-ubuntu-2404' && 'blacksmith-16vcpu-ubuntu-2404' || " in runs_on
</file context>
| self.assertTrue("needs.decide.outputs.early_" in runs_on | |
| and runs_on.endswith("|| 'cmux-fleet-linux-x64' }}") | |
| self.assertTrue("needs.decide.outputs.early_" in runs_on | |
| and "|| 'cmux-fleet-linux-x64')" in runs_on |
| self.assertLess(mini, overflow) | ||
| selected = f"needs.decide.outputs.early_{name.replace('cmux-', '').replace('-', '_')}_runner" | ||
| self.assertIn(selected, route) | ||
| self.assertLess(route.index(selected), route.index("'cmux-fleet-linux-x64'")) |
There was a problem hiding this comment.
P2: This change drops the wiring guard that signer-build never routes to a Mac mini. The signer artifact must stay on Linux, so keep that assertion for this job.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/test_ci_portable_job_placement.py, line 267:
<comment>This change drops the wiring guard that `signer-build` never routes to a Mac mini. The signer artifact must stay on Linux, so keep that assertion for this job.</comment>
<file context>
@@ -262,15 +262,10 @@ def test_early_nightly_controls_try_fleet_before_blacksmith_without_signing_secr
- self.assertLess(mini, overflow)
+ selected = f"needs.decide.outputs.early_{name.replace('cmux-', '').replace('-', '_')}_runner"
+ self.assertIn(selected, route)
+ self.assertLess(route.index(selected), route.index("'cmux-fleet-linux-x64'"))
+ self.assertIn("needs.decide.outputs.track == 'nightly-next'", route)
self.assertNotIn("secrets.APPLE", yaml.safe_dump(job))
</file context>
| self.assertLess(route.index(selected), route.index("'cmux-fleet-linux-x64'")) | |
| self.assertLess(route.index(selected), route.index("'cmux-fleet-linux-x64'")) | |
| if name == "signer-build": | |
| self.assertNotIn("mini_runner", route) |
| path = "tests/test_nightly_linux_admission.py" | ||
| lane = "linux-guard" |
There was a problem hiding this comment.
P2: This registers the new test in the linux-guard lane, which runs it in CI. AGENTS.md forbids adding CI tests, and recent additions use lane = "manual" with a reason instead. Mark it manual and run it on the fleet yourself.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/test-execution.toml, line 1427:
<comment>This registers the new test in the `linux-guard` lane, which runs it in CI. AGENTS.md forbids adding CI tests, and recent additions use `lane = "manual"` with a `reason` instead. Mark it manual and run it on the fleet yourself.</comment>
<file context>
@@ -1423,6 +1423,10 @@ lane = "linux-guard"
lane = "linux-guard"
+[[test]]
+path = "tests/test_nightly_linux_admission.py"
+lane = "linux-guard"
+
</file context>
| path = "tests/test_nightly_linux_admission.py" | |
| lane = "linux-guard" | |
| path = "tests/test_nightly_linux_admission.py" | |
| lane = "manual" | |
| reason = "Lawrence 2026-10-23: no new CI tests. Checks nightly Linux admission receipts and routing; run python3 tests/test_nightly_linux_admission.py yourself after changing scripts/ci/nightly_linux_admission.py or the nightly routing (no build)." |
| run: | | ||
| python3 tests/test_ci_portable_job_placement.py | ||
| python3 tests/test_nightly_linux_admission.py |
There was a problem hiding this comment.
P2: The admission test shares a run step with the placement test, so a placement-test failure exits the shell before the admission test runs. Give it a separate !cancelled() step so both guard failures are reported.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .github/workflows/ci-guards.yml, line 920:
<comment>The admission test shares a `run` step with the placement test, so a placement-test failure exits the shell before the admission test runs. Give it a separate `!cancelled()` step so both guard failures are reported.</comment>
<file context>
@@ -917,7 +917,9 @@ jobs:
- name: Validate portable placement (fleet Linux, then a free mini, then Blacksmith)
if: ${{ !cancelled() && matrix.group == 'ci' }}
- run: python3 tests/test_ci_portable_job_placement.py
+ run: |
+ python3 tests/test_ci_portable_job_placement.py
+ python3 tests/test_nightly_linux_admission.py
</file context>
| run: | | |
| python3 tests/test_ci_portable_job_placement.py | |
| python3 tests/test_nightly_linux_admission.py | |
| run: python3 tests/test_ci_portable_job_placement.py | |
| - name: Validate nightly Linux admission | |
| if: ${{ !cancelled() && matrix.group == 'ci' }} | |
| run: python3 tests/test_nightly_linux_admission.py |
| timeout-minutes: 5 | ||
| env: | ||
| JOBS: resolve-nightly-cmux-tui-client pick-nightly-build-runner resolve-nightly-ghostty-cli-helper build-nightly-inputs find-nightly-next-recovery signer-build | ||
| JOBS: resolve-nightly-cmux-tui-client resolve-nightly-ghostty-cli-helper build-nightly-inputs find-nightly-next-recovery signer-build |
There was a problem hiding this comment.
P2: This always requests a signer-build lease even when the downstream signer job is disabled (the sign_pool input defaults to false). Omit jobs whose if conditions are false so an unused lease cannot consume fleet capacity or hold up admission for the controls that will run.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .github/workflows/nightly.yml, line 827:
<comment>This always requests a `signer-build` lease even when the downstream signer job is disabled (the `sign_pool` input defaults to false). Omit jobs whose `if` conditions are false so an unused lease cannot consume fleet capacity or hold up admission for the controls that will run.</comment>
<file context>
@@ -792,23 +799,52 @@ jobs:
timeout-minutes: 5
env:
- JOBS: resolve-nightly-cmux-tui-client pick-nightly-build-runner resolve-nightly-ghostty-cli-helper build-nightly-inputs find-nightly-next-recovery signer-build
+ JOBS: resolve-nightly-cmux-tui-client resolve-nightly-ghostty-cli-helper build-nightly-inputs find-nightly-next-recovery signer-build
# Keep the compiler artifact executable on the Linux signing jobs.
LINUX_ONLY: signer-build
</file context>
| self.assertLess(mini, overflow) | ||
| selected = f"needs.decide.outputs.early_{name.replace('cmux-', '').replace('-', '_')}_runner" | ||
| self.assertIn(selected, route) | ||
| self.assertLess(route.index(selected), route.index("'cmux-fleet-linux-x64'")) |
There was a problem hiding this comment.
P3: The test name still promises a check that Blacksmith comes after the fleet, but the Blacksmith assertion was removed. Either restore an ordering assertion against 'blacksmith- or rename the test to match what it now checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/test_ci_portable_job_placement.py, line 267:
<comment>The test name still promises a check that Blacksmith comes after the fleet, but the Blacksmith assertion was removed. Either restore an ordering assertion against `'blacksmith-` or rename the test to match what it now checks.</comment>
<file context>
@@ -262,15 +262,10 @@ def test_early_nightly_controls_try_fleet_before_blacksmith_without_signing_secr
- self.assertLess(mini, overflow)
+ selected = f"needs.decide.outputs.early_{name.replace('cmux-', '').replace('-', '_')}_runner"
+ self.assertIn(selected, route)
+ self.assertLess(route.index(selected), route.index("'cmux-fleet-linux-x64'"))
+ self.assertIn("needs.decide.outputs.track == 'nightly-next'", route)
self.assertNotIn("secrets.APPLE", yaml.safe_dump(job))
</file context>
| admission.validate(value, REQUEST) | ||
|
|
||
| def test_plain_public_http_cannot_supply_an_admission_receipt(self): | ||
| with mock.patch("urllib.request.urlopen", return_value=io.BytesIO(b"{}")): |
There was a problem hiding this comment.
P3: The urllib.request.urlopen mock does nothing here because exchange goes through build_opener(...).open. Patch transport.urllib.request.build_opener, or assert that the opener is never built, so the test checks the no-network guarantee it names.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/test_nightly_linux_admission.py, line 58:
<comment>The `urllib.request.urlopen` mock does nothing here because `exchange` goes through `build_opener(...).open`. Patch `transport.urllib.request.build_opener`, or assert that the opener is never built, so the test checks the no-network guarantee it names.</comment>
<file context>
@@ -0,0 +1,274 @@
+ admission.validate(value, REQUEST)
+
+ def test_plain_public_http_cannot_supply_an_admission_receipt(self):
+ with mock.patch("urllib.request.urlopen", return_value=io.BytesIO(b"{}")):
+ with self.assertRaises(ValueError):
+ admission.exchange("http://203.0.113.1/v1/nightly-linux-admission")
</file context>
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Review (codex, router-review): SHIP at ec0df76 Reviewer: codex gpt-6.1-sol xhigh, fast tier. Follow-ups: 2. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward. SHIP at ec0df76 Visual verdict: no images; Leo tries the build
155 focused tests passed. Follow-ups:
|
GitHub Actions
Inactive
|
Review (codex gpt-6-astra high): SHIP at 8ed7063. Reviewed 73dd28e..HEAD with no blocking findings. Four secret-free controls use validated per-job admission; stale outputs retain fleet fallback and delayed terminal decisions remain valid. Credentialed picker remains separate and signer-build is unchanged. Five focused regressions and diff whitespace checks passed independently. The lane also passed all 134 relevant policy tests. |
GitHub Actions
Inactive
|
Found 1 test failure on Blacksmith runners: Failure
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
…tialed set fleet_linux now comes from place-early-mac (the credentialed picker stays off the GCP label, MAC_FIRST) and reaches only the secret-free nightly Linux jobs; credentialed jobs keep #30158's validated ephemeral set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9093a0f1b2d0cbb0b2c
Early NEXT nightly controls can queue on Blacksmith before the GCP fleet has had a chance to start. Request an identity-bound controller lease, reserve one unconsumed JIT runner per job, and permit paid overflow only with a matching recorded 180-second no-start. Stale, missing or invalid receipts retain the existing owned fleet routes. No job or workflow is cancelled.
Leo: “FOR OUR NIGHTLIES ARE THEY END TO END ON OUR FLEET? WE CAN'T BE RELYING ON BLACKSMITH THAT CUTS OUT”.
This routes four controls: the TUI resolver, helper resolver, input bundler and recovery finder. The credentialed Mac-build picker remains on its existing owned-mini placement; its routing App key never enters the secret-free GCP pool. Promotion is a separate owned-Mac change. Signer routing and compilation are unchanged by this PR; #30081 owns that route. Input-bundle notices bootstrap Rust with the already-used pinned setup action.
Only the NEXT track uses the new selectors; other nightly tracks retain their prior routes. The controller binds receipts to repository, run, attempt, source and job keys. The client accepts HTTPS with normal certificate validation or the existing encrypted Tailscale connection to a numeric online peer, with redirects and environment proxies disabled. Future-dated receipts and inconsistent no-start durations are refused. A fleet decision committed before expiry remains valid through delayed response delivery; readiness first observed after expiry cannot win. Each unique runner label belongs to one lease/job, so an unrelated queued job cannot consume it. The controller retains downstream listeners while their parent dependency runs and reclaims them after the exact job/run finishes. The client long-polls within a 200-second deadline with 35-second HTTP bounds; optional mini-capacity reads have a 30-second alarm, cleared and restored in finally. Durable receipts are uploaded as an Actions artifact.
Prior art: #29864 supplies portable placement; #29981 exposes GCP demand but does not provide a nightly-safe no-start handoff. Existing signer artifact caches and warm-source declarations remain; no Swift build or shared compile-cache change. The service prerequisite is HQ#5785, merged at c5353e24bf092f769eafbf49a9bb621515e159d9 after exact-head review at 4503b288 and fleet controller compilation/tests.
Observed / Not run
At 9e3ed62, all 156 focused Python tests pass: admission and transport (25), portable placement (28), nightly trusted routes (29), nightly pooled routes (16), routing table (26), step coverage (5), fork routing (23), Linux runner-size policy (4). The test registry validates the new test's automatic PR execution. New workflow-selector regressions failed 48 assertions before wiring the caller. Review regressions reproduced future-dated/public-HTTP receipt acceptance and non-NEXT routing (six failed assertions), then passed after repair.
Two existing tests still fail on the base: nightly.yml exceeds the 490,000-byte margin (493,159 bytes on base), and the variable report omits CI_CONTROL_RUNNER. These exact named failures were reproduced on base. The self-hosted shell guard also reports eight unchanged offending selectors: each is present byte-for-byte on the base. The new output-name false positive from review is removed. HQ #5798 supplies the receipt-authority correction, and #5804 adds the GitHub CLI prerequisite. Both are deployed in controller version 80ada0f87e20. The final reviewed head 8ed7063 passes 134 relevant policy tests after merging the current base. End-to-end nightly admission evidence is pending; no runtime speedup is claimed yet.
Performance
Changelog
none
Agent session: agent session
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Routes early NEXT nightly controls onto reserved fleet Linux runners before paid overflow, so builds no longer queue on Blacksmith waiting for the GCP fleet to start. Controls request an identity-bound lease from the fleet controller, which reserves an unconsumed JIT runner per job; paid overflow requires a matching recorded 180-second no-start receipt. Missing, stale, or invalid receipts keep the existing owned-fleet routes, and no job or workflow is cancelled.
Fleet admission
Scope
Written for commit 8ed7063. Summary will update on new commits.