Skip to content

fix(cmux-next): VNC reconnect test helpers are nonisolated (CmuxNextAppTests compile red) - #27862

Merged
teamleaderleo merged 1 commit into
feat-cmux-nextfrom
fix-vnc-test-isolation
Oct 30, 2026
Merged

teamleaderleo merged 1 commit into
feat-cmux-nextfrom
fix-vnc-test-isolation

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

#26557 landed RemoteVNCReconnectTests (carried from #26697). Under the CmuxNext test target's .defaultIsolation(MainActor.self), the private AttemptCounter is main-actor isolated, so the @Sendable transport factory cannot call increment():

RemoteViewVNCTests.swift:130:22: error: call to main actor-isolated instance method 'increment()' in a synchronous nonisolated context

CmuxNextAppTests does not compile on feat-cmux-next at 0ce3c31, so every CmuxNext suite run fails at build. This marks the two lock-guarded test helpers nonisolated, as #27811 already writes them (that PR now conflicts with base). Two lines, tests only.

Observed / Not run

  • Fleet focused run at feat-cmux-next 57b38a2-based head: test target fails to compile with the error above.
  • Fleet focused run at this head (RemoteVNCReconnectTests, RemoteVNCMappingTests, PaneTabBarControlsTests, NewTabMenuTriggerTests): result posted in a comment below.

Changelog

none

Agent session: agent session

🤖 Generated with Claude Code


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


Summary by cubic

Fixes the CmuxNextAppTests compile failure on feat-cmux-next by marking two VNC reconnect test helpers as nonisolated.

Under the test target's MainActor default isolation, the @Sendable transport factory could not call AttemptCounter.increment(), breaking the build for every CmuxNext suite run. The lock-guarded CountingRFBTransport and AttemptCounter helpers are now nonisolated; test behavior is unchanged.

Written for commit 4a4c593. Summary will update on new commits.

View guided diff Turn on auto-fix

…et compile red)

#26557 added RemoteVNCReconnectTests; under the test target's MainActor
default isolation, AttemptCounter.increment() is main-actor isolated and
the @sendable transport factory cannot call it, so CmuxNextAppTests does
not compile on feat-cmux-next. Both lock-guarded helpers are nonisolated,
as #27811 already writes them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo teamleaderleo added the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · session

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 4a4c593: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr27862 -f ref=fix-vnc-test-isolation -f platform=macos -f backend_mode=local

Not verified: macOS jobs did not run: compile and app tests were skipped on 4a4c593e20; ci-status is green without them.

CI passes on 4a4c593e20 (run 40993713808 attempt 1).
macOS jobs did not run: compile and app tests were skipped: Fast static checks skipped (the macos job needs both to succeed). Re-run CI once they pass, before merging.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on c033155 to compare; do not merge until green; see lane-rules

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra high fast, independent box): SHIP at 4a4c593

SHIP

No findings. Both helpers safely support nonisolated access: the transport is stateless, and the counter protects all mutable state with a lock.

Tests could not run because Swift is unavailable.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex-review-box/gpt-6-astra high, lane-place): SHIP at 4a4c593

SHIP

No findings. Both helpers safely support nonisolated access: the transport is stateless, and the counter protects all mutable state with a lock.

Tests could not run because Swift is unavailable.

@teamleaderleo teamleaderleo removed the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Found 8 test failures on Blacksmith runners:

Failures

Test View Logs
--from must install a complete prebuilt tree/
--from must install a complete prebuilt tree
View Logs
/tmp/tmpiz6yg9qa/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmpo1kfruxo/summary-fail.json (live):/summary-fail.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)/
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)
View Logs

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

@cmux-bot cmux-bot 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.

cmux-bot approves 4a4c593: an independent review passed this exact head (review box review-27862-4a4c receipt by teamleaderleo). A push needs a new review; a later BLOCK at this head dismisses this approval.

@github-actions github-actions Bot added the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@teamleaderleo
teamleaderleo merged commit b0b87c9 into feat-cmux-next Oct 30, 2026
175 of 192 checks passed
@teamleaderleo
teamleaderleo deleted the fix-vnc-test-isolation branch October 30, 2026 02:54
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Fleet focused run at 4a4c593 (step fc0c7d0c99ce6df26bc51880): RemoteVNCReconnectTests, RemoteVNCMappingTests, PaneTabBarControlsTests and NewTabMenuTriggerTests all passed. On current feat-cmux-next the same test target does not compile (RemoteViewVNCTests.swift:130:22, a call to main actor-isolated 'increment()'; step 7e6da2ca3ec136fbda9cad5c). The 'cmux-next checks' and release-notary reds here also fail on base e430ffd; prepush-gate on this commit reports no new violations.

@github-actions

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/40994392464 at b0b87c9 failed: cmux-next checks (god files, concurrency, crash safety, l10n).
Those jobs last passed at 7c1b653. This pull request is one of 1 merged since: #27862.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

@github-actions

Copy link
Copy Markdown
Contributor

Shipped in:

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

lawrencecchen added a commit that referenced this pull request Oct 30, 2026
…ice 1 merge seam)

Slice 1 switch/conflict review (2026-10-30), finding 1: feat's tests merged
without a text conflict but call APIs perf changed, so CmuxNextAppTests does
not compile.
- PaneTabBarControlsTests: TabStripView.testing(model:) (perf makes input:
  required) and strip.scroller.settle() (perf moved feat's settleScroll() into
  TabStripScroller.settle(), the same body).
- RemoteViewVNCTests (#27862): RemoteVNCSession(cloudRecord:makeTransport:)
  now also takes perf's work:, mainWork: and signIns:; the test passes its own
  lane, scheduler and in-memory sign-ins, as RemoteVNCSignInReadTests does.
No assertion changes. Proof: the fleet compile of CmuxNextAppTests on this head.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 4a4c593e Deployed Oct 30, 2026 by teamleaderleo via wait for same-key tree publisher #79836
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:low Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant