Skip to content

Fix virtual sleep determinism detection - #8541

Closed
austinywang wants to merge 254 commits into
mainfrom
issue-8529-workflow-guard-sleep-assert
Closed

austinywang wants to merge 254 commits into
mainfrom
issue-8529-workflow-guard-sleep-assert

Conversation

@austinywang

@austinywang austinywang commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Closes #8529

Root cause

The determinism checker treated sleep-shaped syntax too broadly. Swift virtual-clock enum values and arbitrary receiver methods could be classified as real wall-clock delays, while real sleep APIs in other lexical and language forms were not modeled consistently. That made the workflow guard noisy and left causal-ordering gaps.

What changed

  • Reworked the checker around language-aware, high-confidence real-delay detection for Swift, Python, JavaScript and TypeScript, and shell.
  • Added lexical binding and shadowing resolution so real standard clocks and timer aliases are detected without flagging injected or virtual clocks.
  • Preserved causal ordering: real sleep followed by an assertion is rejected, assertion before sleep and deadline-bounded polling remain allowed.
  • Added a 23-test behavioral CLI suite and expanded the built-in self-test to 23 positive plus 33 negative fixtures.
  • Moved the determinism CLI suite, self-test, and strict scan into workflow-guard-tests before the later Python 3.9-only nightly-prune setup, so parsing uses the same interpreter that validates repository syntax.
  • Restored direct virtual sleep-event expectations in the Iroh refresh test.
  • Kept the mobile terminal transition test asserting the exact synthesized duration value after merging current main.

The branch retains test-only red commits followed by their corresponding fixes, so the regression history demonstrates that coverage catches the defects.

Verification

Exact head: b625a30
Base: 252aa38

  • python3 tests/test_check_test_determinism.py: 23 tests passed.
  • python3 scripts/check-test-determinism.py --self-test: 23 positive plus 33 negative fixtures passed.
  • python3 scripts/check-test-determinism.py --strict: zero findings.
  • Ruff, Python 3.9 and 3.11 compilation, git diff --check, and cmux policy check passed.
  • Both touched Swift package tests executed one test and passed: scheduledRefreshReplacesCredentialWithoutChangingEndpointIdentity and zeroDurationSynthesizesDuration.
  • Three warm strict-scan samples: main median 4.27s, head median 4.47s, plus 4.7 percent. Limit: 5.34s relative and 20.29s absolute.
  • Tagged Debug build succeeded with ./scripts/reload.sh --tag pr8541-verify.
  • Tagged runtime smoke passed against the isolated socket: ping, version, topology, workspace status-none setup, terminal send, and read-screen. The terminal returned the unique marker and full head SHA.
  • Manually dispatched exact-head CI run 30193492950 completed all 18 jobs successfully. workflow-guard-tests executed the 23-test CLI suite, 56-fixture self-test, and strict zero-finding scan.
  • Canonical autoreview completed cleanly with no accepted or actionable findings; merge-conflict and cmux policy gates were clean.

Demo Video

Demo Video: N/A. This changes an internal static workflow guard; the tagged app was verified through its isolated socket instead.

Localization

Not applicable. The final five-file PR diff changes internal CI, checker, and test surfaces only; no user-facing strings or localization catalogs changed.

@cursor

cursor Bot commented Jul 21, 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 Jul 21, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The determinism checker now distinguishes real clock sleeps from fake-clock event data, and CI validates the checker before strict enforcement. The relay credential refresh test directly checks emitted clock events after each simulated refresh advance.

Changes

Determinism-safe refresh testing

Layer / File(s) Summary
Sleep detector matching and validation
scripts/check-test-determinism.py, .github/workflows/ci.yml
The detector recognizes unqualified and locally bound real-clock sleeps, expands positive and negative fixtures, and runs self-validation before strict CI checking.
Relay refresh clock-event assertions
Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayCredentialCoordinatorTests+Refresh.swift
Refresh assertions inline checks for the initial and replacement sleep events after advancing the test clock.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: azooz2003-bit, lawrencecchen

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR addresses #8529 by distinguishing virtual clock .sleep(...) events from real delays and restoring the Iroh test expectations.
Out of Scope Changes check ✅ Passed The extra CI and test updates support the determinism fix and regression coverage; no unrelated changes are evident.
Cmux Swift Actor Isolation ✅ Passed PR only changes a Swift test plus Python/CI; no production Swift isolation debt is introduced or worsened.
Cmux Swift Blocking Runtime ✅ Passed The Swift diff is test-only and only inlines async clock expectations; it adds no blocking waits, sleeps, locks, or sync coordination in production code.
Cmux Browser Automation Off-Main ✅ Passed Diff only touches determinism script, one Iroh test, and CI; no browser-automation routing, WebKit/AppKit, or socket-worker code changed.
Cmux Expensive Synchronous Load ✅ Passed Diff only touches a test, the Python checker, and CI workflow; no production Swift load path or main-actor sync load was added.
Cmux Cache Substitution Correctness ✅ Passed The PR only changes a test Swift file, a Python checker, and CI workflow; no production Swift/TS/JS cache/persistence path was swapped to a cache.
Cmux No Hacky Sleeps ✅ Passed The diff only updates a Swift test, a checker regex/self-test fixture, and a workflow step; no production TS/JS/shell/runtime sleep or polling was added.
Cmux Algorithmic Complexity ✅ Passed Diff is test-only plus CI tooling; no production Swift/JS/shell path adds scalable nested collection scans or hot-path re-sorts.
Cmux Swift Concurrency ✅ Passed The only Swift change is an assertion simplification in a test; it adds no DispatchQueue/Combine/completion-handler/fire-and-forget Task pattern.
Cmux Swift @Concurrent ✅ Passed The only Swift change is a test assertion simplification; no @concurrent, nonisolated async, or UI-bound heavy async code was added or altered.
Cmux Swift Package Boundaries ✅ Passed Only test Swift, script, and CI files changed; no production Swift code or package-boundary-sensitive logic was moved, and test fixtures are allowed.
Cmux Swiftpm Lockfiles ✅ Passed No .gitignore, Package.resolved, or Xcode package-reference changes appear in the PR diff; the workflow edit is unrelated to SwiftPM lockfiles.
Cmux Swift Logging ✅ Passed Only a Swift test changed, and it contains no added or modified print/debugPrint/dump/NSLog or secret-exposing logs.
Cmux User-Facing Error Privacy ✅ Passed Changes touch a test, a CI workflow, and an internal determinism script; no user-facing errors, alerts, API bodies, or recovery copy were added or exposed.
Cmux Full Internationalization ✅ Passed Only test, script, and CI workflow files changed; no user-facing Swift text, catalogs, web messages, or metadata were touched.
Cmux Swiftui State Layout ✅ Passed No SwiftUI view/state code changed; diff only touches a test, a Python checker, and CI config, so the SwiftUI layout rule is not applicable.
Cmux Architecture Rethink ✅ Passed Test-only Swift change removes a false-positive virtual clock expectation; it adds no new timing, locks, observers, or split ownership patterns.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: the diff only updates tests, determinism detection, and CI; no NSWindow/WindowGroup/cmuxAuxiliaryWindowIdentifiers changes, so the rule doesn't apply.
Cmux Source Artifacts ✅ Passed Changed paths are a test, a checked-in script, and a workflow config; the policy allows source/tests/scripts/configs and no artifact dirs appear.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Diff touches only tests, CI, and a script; no **/Sources/** Swift files changed, so no production test/debug seam was introduced.
Cmux No Ambient Global State ✅ Passed No new production Swift ambient globals or singletons were introduced; the Swift diff only rewrites test assertions.
Title check ✅ Passed The title is concise and accurately reflects the main change: fixing virtual sleep determinism detection.
Description check ✅ Passed The description covers the change, rationale, and verification, but omits the template's review trigger and checklist sections.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-8529-workflow-guard-sleep-assert

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.

@greptile-apps

greptile-apps Bot commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes the test-determinism gate distinguish real delays from virtual or shadowed sleep APIs. The main changes are:

  • Adds language-aware sleep detection for Swift, Python, JavaScript, TypeScript, and shell.
  • Adds lexical binding, shadowing, source masking, and causal-order handling.
  • Expands the CLI and built-in behavioral test suites.
  • Runs the determinism checks with the workflow's repository-syntax interpreter.
  • Restores precise expectations in two Swift tests.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
scripts/check-test-determinism.py Adds language-specific sleep resolution, source masking, scope tracking, and causal-order checks.
tests/test_check_test_determinism.py Adds behavioral coverage for real delays, virtual clocks, shadowing, parser behavior, and assertion ordering.
.github/workflows/ci.yml Moves the determinism suite before the later Python 3.9 setup and runs all three gate checks.
Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayCredentialCoordinatorTests+Refresh.swift Restores direct virtual sleep-event expectations in the relay refresh test.
Packages/iOS/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalDockKeyboardTransitionPlannerTests.swift Checks the synthesized animation case before comparing its exact duration.

Reviews (128): Last reviewed commit: "Merge branch 'main' of https://github.co..." | Re-trigger Greptile

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/check-test-determinism.py`:
- Line 196: The sleep-detection regex at scripts/check-test-determinism.py lines
196-196 must exclude Swift enum cases while matching unqualified and
method-called sleeps; replace its current boundary pattern with the specified
refined matching logic. Remove the _is_assertion_line workaround at
scripts/check-test-determinism.py lines 449-453, leaving assertion lines
eligible for real delay detection because the regex now excludes virtual-clock
.sleep( cases.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 855c1b14-efa8-4cf1-ad19-5c2475636e7d

📥 Commits

Reviewing files that changed from the base of the PR and between 157c91e and 0f23ffa.

📒 Files selected for processing (1)
  • scripts/check-test-determinism.py

Comment thread scripts/check-test-determinism.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/check-test-determinism.py`:
- Around line 455-461: Update _SLEEP_CALL to use the specified prefix pattern so
it matches unqualified sleeps and method calls while excluding virtual clock
enum cases written as .sleep(. Remove _IMPLICIT_MEMBER_SLEEP_VALUE and delete
the candidate_line manipulation in the sleep-detection flow, using the original
line directly for _SLEEP_CALL matching.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 169bd566-3569-4894-ba9f-0a4e9f5d16ce

📥 Commits

Reviewing files that changed from the base of the PR and between 0f23ffa and 4d9e696.

📒 Files selected for processing (1)
  • scripts/check-test-determinism.py

Comment thread scripts/check-test-determinism.py Outdated
@cursor

cursor Bot commented Jul 21, 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 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/check-test-determinism.py`:
- Around line 461-478: Update the named-clock resolver around the backward scan
to track lexical scope boundaries, including private/static functions and
closures/blocks, so outer bindings cannot leak into nested scopes. Resolve typed
function parameters such as ContinuousClock before returning, while retaining
nearest-binding precedence and recognizing real-clock declarations or
initializers. Ensure injected or virtual-clock fixtures are not classified as
real, and make the added determinism self-tests pass.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c31b5a9f-ff28-4296-92d6-3da7baceff74

📥 Commits

Reviewing files that changed from the base of the PR and between 1ad7e4b and 9406d1d.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • scripts/check-test-determinism.py

Comment thread scripts/check-test-determinism.py Outdated
@cursor

cursor Bot commented Jul 21, 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.

@cursor

cursor Bot commented Jul 21, 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.

@cursor

cursor Bot commented Jul 28, 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.

@cursor

cursor Bot commented Jul 28, 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.

@cursor

cursor Bot commented Jul 28, 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.

@cursor

cursor Bot commented Jul 28, 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.

@lawrencecchen lawrencecchen added the stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening. label Sep 23, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

workflow-guard-tests fails on main: sleep-then-assert findings in CmxIrohRelayCredentialCoordinatorTests+Refresh

3 participants