Skip to content

ci(ios): queue test-ios runs for the owned pool within CI_PR_POOL_QUEUE_ROUNDS - #14630

Merged
teamleaderleo merged 1 commit into
mainfrom
ci-ios-picker-full
Sep 25, 2026
Merged

teamleaderleo merged 1 commit into
mainfrom
ci-ios-picker-full

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Why

Run 36136190497 (PR #14602) logged every pool is full after replaying 22 newer run(s) and sat 15 minutes QUEUED on blacksmith-6vcpu-macos-26 before it was cancelled.

The owned pool was not full. The janitor snapshot it read (12:34:57 UTC) had glaeda-std-xcode-26.6 at 8 running of 32 slots, 15 queued (root jobs; the root runners were the bottleneck), and committed 43. glaeda-ios-sim had no entry, so all 8 simulator minis were free. The 6vcpu macOS 26 pool had 62 queued, the 12vcpu 23.

ios_runner_pool.py passed no queue rounds, so pr_runner_pool.settings() got rounds 0, the kill-switch rule: taken = max(running + queued, committed) = 43 > 32 before any replay, then 3 more per replayed PR run. Pull request CI dropped that rule in #14401 (the snapshot's copied settings show queue_rounds 2); the iOS picker never picked it up. With no owned room, the fallback is Blacksmith by design, and iOS then keeps its own variable.

"Newer" means created after the snapshot, not after this run: those runs are not in the snapshot, so replaying them is right. Replaying the real snapshot through the picker offline reproduces the log line exactly with rounds 0, and picks glaeda-std-xcode-26.6 ("0 of 32 owned machines free and 14 queue places within 20 min, Blacksmith's expected wait 26 min") with rounds 2.

What

  • ios_runner_pool.py takes --queue-rounds (test-ios.yml passes vars.CI_PR_POOL_QUEUE_ROUNDS; "" is the default 1 round, 0 the kill switch, omitted is 0 for old callers) and uses pull request CI's queueing rule through e2e_runner_pool.settings(). Simulators still have to be free now.
  • owned_pool_rescue.py gives test-ios.yml runs (dispatch and pull request) the same queue_seconds() allowance as ci.yml runs, so a job queued on purpose is not moved after the bare 90 s budget.
  • E2E is unchanged (still rounds 0).

Tests

  • python3 tests/test_ci_pr_runner_pool.py: 158 OK, including the incident snapshot (rounds 0 stays on Blacksmith, 2 takes the std pool, "" is 1 round, the queue bound still sends a flood to Blacksmith, simulators are not queued for, an invalid value reads nothing) and the workflow wiring.
  • python3 tests/test_ci_owned_pool_rescue.py: 92 OK, test-ios.yml gets the allowance; E2E and ios-screenshots do not.
  • python3 tests/test_ci_workflow_run_sources.py, tests/test_ci_fork_runner_routing.py, tests/test_ci_self_hosted_guard.sh pass.

Follow-up (pre-existing, from review): iOS runs since the snapshot are charged simulators but not std-pool machines, so a burst of package-only runs in one snapshot window can overshoot the queue bound.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes iOS test runs incorrectly landing on Blacksmith queues while owned macOS pools had room, by letting the iOS picker queue for owned pools within CI_PR_POOL_QUEUE_ROUNDS.

  • Replaces the kill-switch rule (counted every in-flight run's whole future peak as taken) with pull request CI's queueing rule; run 36136190497 sat 15 minutes behind 62 queued jobs while 8 of 32 owned machines ran.
  • Passes --queue-rounds from test-ios.yml; omitted is 0 rounds (old behavior), "" is the default 1 round.
  • Simulators must still be free now; they are not queued for.
  • Gives test-ios.yml runs the same queue allowance in owned_pool_rescue as CI runs, so intentionally queued jobs are not moved after the bare 90 s budget.

Written for commit 1b8a553. Summary will update on new commits.

Review in cubic

…UE_ROUNDS

ios_runner_pool.py read no queue rounds, so it used the kill-switch rule:
an owned pool only with the run's peak free counting every in-flight run's
whole future peak (the janitor's `committed`). Run 36136190497 read 43 of 32
std machines taken while 8 ran, and sat 15 minutes on the 6vcpu macOS 26
pool behind 62 queued jobs.

The picker now takes --queue-rounds (vars.CI_PR_POOL_QUEUE_ROUNDS) and
places the run by pull request CI's rule; with the rounds it takes the std
pool's queue places. Simulators must still be free now. The rescue gives
test-ios.yml runs the same queue allowance as CI runs, so their queued owned
jobs are not moved after the bare 90 s budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo
teamleaderleo merged commit 088034b into main Sep 25, 2026
13 of 16 checks passed
@teamleaderleo
teamleaderleo deleted the ci-ios-picker-full branch September 25, 2026 13:11
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 1b8a5536d2, merged 2026-09-25 13:11:10 UTC

  • Not verified at merge: ci-status (not reported), Web complexity (not reported), web-validation (not reported)
  • Skipped by policy: web-build, web-database-tests, web-tests
  • Full suite: runs on main after merge.

Labeled merged-unverified: if main breaks near this merge, look here first.

@github-actions github-actions Bot added the merged-unverified A judging check was not green at merge; see the merge receipt comment label Sep 25, 2026
@coderabbitai

coderabbitai Bot commented Sep 25, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: fbbcef5f-beab-40ae-b78f-16facbcf12f6

📥 Commits

Reviewing files that changed from the base of the PR and between c055747 and 1b8a553.

📒 Files selected for processing (6)
  • .github/workflows/test-ios.yml
  • scripts/ci/e2e_runner_pool.py
  • scripts/ci/ios_runner_pool.py
  • scripts/ci/owned_pool_rescue.py
  • tests/test_ci_owned_pool_rescue.py
  • tests/test_ci_pr_runner_pool.py
 ___________________________________________________________________
< Your tests are like unicorns: frequently referenced, rarely seen. >
 -------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 25, 2026
60d8ac1 Land ejc3's isolated defaults test from manaflow-ai#12598 (manaflow-ai#14504)
e926f24 fix: resolve the terminal Copy guard through the Command-aware keyboard layout (manaflow-ai#10872) (manaflow-ai#13015)
0df2946 Fix startup-race crash in v2RefreshKnownRefs against a half-restored session (manaflow-ai#2751) (manaflow-ai#9627)
611eeac ci(e2e): queue E2E runs for the owned pool within CI_PR_POOL_QUEUE_ROUNDS (manaflow-ai#14640)
27b8cbc ci: seed the Swift package cache from main pushes (manaflow-ai#14638)
be46dba ci: stop E2E from saving an unresolved Swift package cache (manaflow-ai#14632)
2486e99 ci: run-e2e.sh --wait asks glaeda-gh instead of polling GitHub (manaflow-ai#14622)
088034b ci(ios): queue test-ios runs for the owned pool within CI_PR_POOL_QUEUE_ROUNDS (manaflow-ai#14630)

# Conflicts:
#	.github/workflows/main-regression-bisect.yml
#	.github/workflows/perf-activation.yml
#	.github/workflows/seed-derived-data.yml
#	.github/workflows/test-e2e.yml
#	.github/workflows/test-ios.yml
#	.github/workflows/test-macos-suite.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-unverified A judging check was not green at merge; see the merge receipt comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant