Skip to content

ci: flag condition polls bounded by a Task.yield() count - #14019

Merged
teamleaderleo merged 2 commits into
mainfrom
ci/determinism-yield-count-poll
Sep 23, 2026
Merged

teamleaderleo merged 2 commits into
mainfrom
ci/determinism-yield-count-poll

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

check-test-determinism.py reported 0 findings in cmuxTests, yet 27 waits there polled a condition for a fixed number of Task.yield() calls (#13903). A yield waits for nothing. N yields take however long N reschedules happen to take, so the bound shrinks exactly when the runner is busy, and a slow pass becomes a failure. That is the shape test-determinism.md bans, and it produced the SimulatorPanelThemeTests flake that #13907 fixed. The checker could not see it because it only matched sleep call sites.

The checker now has a yield-count-poll rule for Swift test files. It flags a for _ in 0..<N / for _ in A...B loop that awaits Task.yield(), exits on a condition (break or return), and reads no clock. It does not flag:

On current main the rule finds 126 sites in 58 files. That includes the 17 remaining cmuxTests sites #13903 lists after #13907, and a random sample of the Packages/**/Tests hits were all real condition polls. They are allowlisted per file in .github/test-determinism-allowlist.txt, with a reason pointing at #13903, so the --strict gate stays green. New test files with the shape fail it. The allowlist is keyed by (file, rule), as for the existing rules, so a new site added to an already-listed file is not caught. The rules doc now names the shape beside the sleep ban.

Refs #13903. This is the checker half. Converting the 126 sites is separate work, and each file comes off the allowlist once it is converted.

Testing

  • Added 4 positive and 6 negative self-test fixtures: the SimulatorPanelThemeTests shape, return and guard exits, a one-line loop, a deadline-bounded loop, a bare drain, a cancellation-only exit, a named index, a Python analogue, and a string literal. Commit 1 adds only the fixtures, and --self-test fails with 4 missing positives. Commit 2 passes (142 positive + 92 negative).
  • python3 scripts/check-test-determinism.py --strict on the branch rebased onto current main: 0 active, 126 allowlisted.
  • I ran every guard command in ci-guards.yml locally, and all passed except three that fail for environment reasons: test_ghostty_zig_version_sync.sh and lint-stored-dispatch-work-items.py need submodules this checkout lacks, and the app-host catalog-diff line needs $RUNNER_TEMP and a base SHA. The ci-cache-receipts.yml invocation (--roots tests/test_ci_cache_restore_receipt.py --strict) passes.

Demo Video

Not applicable (CI tooling change).

Checklist

  • I tested the change locally
  • I added or updated tests for behavior changes
  • For iOS connectivity, auth, lifecycle, workspace or terminal changes, I updated the deterministic soak coverage (not applicable: static checker only)
  • I updated docs/changelog if needed (.github/review-bot-rules/test-determinism.md)
  • I requested bot reviews after my latest commit
  • All code review bot comments are resolved
  • All human review comments are resolved

— Ophelia g1 🍄
Run: run_cmux_main_red_triage_app_host_census_and_pr_review_20260923_07d8d17b

🤖 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

Adds a yield-count-poll rule to check-test-determinism.py that flags Swift loops which poll a condition for a fixed number of Task.yield() calls. A yield waits for nothing, so the bound shrinks exactly when the runner is busy and a slow pass becomes a failure; the checker previously missed this shape because it only matched sleep call sites (refs #13903).

  • Skips loops that read a clock or deadline, drain yields with no condition, exit only on cancellation, or use a named index.
  • Allowlists the 126 existing sites in 58 files by (file, rule) so the --strict gate stays green; new files with the shape now fail it.
  • Adds 4 positive and 6 negative self-test fixtures and names the shape in the rules doc.

Migration

  • Existing test files must come off the allowlist one at a time as they convert to deadline-bounded polls.

Written for commit aa3b9ab. Summary will update on new commits.

Review in cubic

teamleaderleo and others added 2 commits September 23, 2026 09:16
A loop that polls a condition for N Task.yield() calls fails on correct
code when the runner is busy, which is the shape the determinism rules
ban, but check-test-determinism.py reports none of them because it only
matches sleep call sites (#13903). These self-test fixtures describe a
yield-count-poll rule and fail until the detector exists.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
check-test-determinism.py now reports yield-count-poll: a Swift
`for _ in 0..<N` loop that awaits Task.yield() and exits on a condition,
with no deadline in the body. A yield waits for nothing, so the bound
shrinks exactly when the runner is busy (#13903). Loops that already
read a clock, drain yields with no condition, or exit only on
cancellation are not flagged.

The 126 existing sites in 58 files are allowlisted by file so the strict
gate stays green; new files with the shape fail it. The rules doc names
the shape beside the sleep ban.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cursor

cursor Bot commented Sep 23, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 18 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: c010168e-e108-4f6d-8bb1-9dd522710d8c

📥 Commits

Reviewing files that changed from the base of the PR and between 4b82298 and aa3b9ab.

📒 Files selected for processing (3)
  • .github/review-bot-rules/test-determinism.md
  • .github/test-determinism-allowlist.txt
  • scripts/check-test-determinism.py

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.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Self-review. What I verified:

CI tooling and docs only, so I'm enabling squash auto-merge.

— Ophelia g1 🍄

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 23, 2026 16:17
@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@teamleaderleo
teamleaderleo merged commit 8abd2e9 into main Sep 23, 2026
57 of 58 checks passed
rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 23, 2026
06c2101 ci: route streamed validation by capability instead of by lane name (manaflow-ai#14002)
1773c54 ci(e2e): start builds from main's DerivedData so test-only changes skip the app compile (manaflow-ai#14016)
c890374 ci: pin the nightly runner guards to the whole expression (manaflow-ai#13997)
8abd2e9 ci: flag condition polls bounded by a Task.yield() count (manaflow-ai#14019)
e4ca672 ci(ios): record the cmux.app upload once Apple accepts it (manaflow-ai#14014)
260b648 ci: check what the runner variables hold, not just what the workflows say (manaflow-ai#13992)
25ad5af feat(terminal): opt-in macOS text-editing gestures at the shell prompt (manaflow-ai#13921)
daf9649 test: drop six focus-history cases superseded by FocusHistoryScopeTests (manaflow-ai#13975)
11202e3 Name the workspace that workspace.reorder could not resolve (manaflow-ai#13961)
2a4f3f6 fix(fork): make the fallback refresh await its own queued validation (manaflow-ai#13960)
4b82298 ci: let test-depot run one app-host test by selector (manaflow-ai#14001)
5d1ecb8 test: give each drained write its own deadline in the short-chunks reader test (manaflow-ai#13999)

# Conflicts:
#	.github/workflows/ci-guards.yml
#	.github/workflows/ci-health-report.yml
#	.github/workflows/ios-appstore-upload.yml
#	.github/workflows/ios-streamed-validate.yml
#	.github/workflows/iroh-release-gate.yml
#	.github/workflows/nightly.yml
#	.github/workflows/test-depot.yml
#	.github/workflows/test-e2e.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant