Skip to content

CLI vm tests use the app's responses, and a stale resize test goes away - #12601

Closed
ejc3 wants to merge 4 commits into
manaflow-ai:mainfrom
ejc3:fix/cli-vm-test-expectations
Closed

ejc3 wants to merge 4 commits into
manaflow-ai:mainfrom
ejc3:fix/cli-vm-test-expectations

Conversation

@ejc3

@ejc3 ejc3 commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two CLI vm tests fail, and one of them crashes the test host.

testVMSSHAliasUsesCmuxRemoteWhenProviderSSHIsUnmanaged expects cmux vm ssh to fall back to cmux-remote when provider SSH is unmanaged. Three parts of its mock no longer match what the app sends or what the CLI asks for.

  • The CLI falls back when the error's data.backend_code is vm_attach_transport_unsupported. The app sends that code inside data, under a vm_error code. The mock put the provider code at the top level with no data, so the CLI printed the error and exited 1.
  • Since Cloud attach: trust the private network, drop cmux-tui device enrollment #12042, the CLI only dials a machine it has not opened before when vm.cmux_remote_info reports trusted_carrier: true. The mock left that out, so after the fallback the CLI stopped with "The Cloud machine is still preparing remote access".
  • Since Cloud tree: flatten terminal tabs and surface Displays #12227, the CLI reads surface.catalog before deciding whether to project an existing terminal or open a new one. The mock answered it with "Unexpected method". The mock now reports a connected machine with no remote workspaces, which keeps the test on the new-terminal path it was written for, and the expected request sequence includes the catalog request.

After the CLI exited early, the test read bindCommands[0] from an empty list and crashed the host with "Index out of range". The mock now sends the app's response shapes, and the test fails instead of crashing if the bind requests are missing.

testVMResizeIsNoLongerAVerb was added when vm resize was removed. #12442 brought the verb back on purpose and covers it in tests/test_cli_vm_resize.py, so the test now fails by design and is removed.

Testing

  • Ran CLINotifyProcessIntegrationRegressionTests/testVMSSHAliasUsesCmuxRemoteWhenProviderSSHIsUnmanaged on an EC2 Mac (Xcode 26.6, macOS 15.7) through scripts/ci/run-app-host-xcodebuild.sh, the way CI runs app-host tests. At main 4638e5b1ea plus the compile fixes in Fix the compile errors that keep main and its unit tests from building #12584, it records five errors and then crashes the test host with "Index out of range". With this change on the same base, the test passes, and the host does not crash.
  • At the same base, testVMResizeIsNoLongerAVerb fails, because cmux vm resize now reaches the app as vm.resize.

Demo Video

Not applicable. The change only affects unit tests.

Review Trigger (Copy/Paste as PR comment)

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

Checklist

  • I tested the change locally
  • I added or updated tests for behavior changes
  • I updated docs/changelog if needed
  • I requested bot reviews after my latest commit (copy/paste block above or equivalent)
  • All code review bot comments are resolved
  • All human review comments are resolved

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

Updates the CLI VM integration tests to match the app's current responses, so the unmanaged-provider SSH fallback test passes instead of exiting early and crashing the test host. Removes the obsolete vm resize rejection test because the verb is supported again and covered by tests/test_cli_vm_resize.py.

  • Mocks provider SSH errors as vm_error with data.backend_code, reports remote access as trusted, and answers surface.catalog with a connected machine and no remote workspaces.
  • Adds the catalog request to the expected request sequence and makes missing bind requests fail the test instead of crashing with an out-of-range error.
  • No production code changes; only test fixtures and assertions.

Written for commit 38ba217. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved VM SSH fallback behavior when provider-managed SSH is unavailable.
    • Provides clearer error details and trusted remote connection information.
    • Automatically opens a new terminal when no remote workspace is available.
  • Tests

    • Updated VM SSH coverage for the revised fallback flow.
    • Removed obsolete coverage for the retired VM resize command.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 3a45fe3d-188c-45ce-a40e-9ded76f63d48

📥 Commits

Reviewing files that changed from the base of the PR and between 65eee0e and b4e188b.

📒 Files selected for processing (2)
  • cmuxTests/CLIVMLayoutEnvTests.swift
  • cmuxTests/VMSSHCommandTests.swift
💤 Files with no reviewable changes (1)
  • cmuxTests/CLIVMLayoutEnvTests.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The PR removes coverage for the deleted vm resize command and updates VM SSH fallback tests for revised error data, trusted carrier information, surface catalog discovery, and terminal creation.

Changes

VM CLI test updates

Layer / File(s) Summary
Remove obsolete resize regression coverage
cmuxTests/CLIVMLayoutEnvTests.swift
Removes the test for rejection and help-text omission of the deleted vm resize command.
Update VM SSH fallback coverage
cmuxTests/VMSSHCommandTests.swift
Updates mocked error data, adds trusted carrier and surface catalog responses, and verifies the revised RPC sequence and bind-count handling.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: lawrencecchen

Merge Risk: ⚪ Minimal · up to b4e18

This PR updates VM CLI test fixtures and removes obsolete coverage; no actionable merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the two main changes: updating CLI VM tests to match app responses and removing the obsolete resize test.
Description check ✅ Passed The description includes the required summary, testing details, demo video status, review trigger, and checklist. It explains the test failures, fixes, and verification results.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed PASS. The authoritative diff changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. It removes one test and updates test mock responses and assertions. The custom …
Cmux Swift Blocking Runtime ✅ Passed PASS. The authoritative diff changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. It removes one test and updates deterministic mock responses and assertions in …
Cmux Browser Automation Off-Main ✅ Passed PASS. The pull request changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. The diff removes a VM resize test and updates VM SSH mock responses and assertions. I…
Cmux Expensive Synchronous Load ✅ Passed PASS: The pull request changes only two files under cmuxTests/. The diff updates test mocks, request assertions, and removes an obsolete test. It adds or moves no production Swift code and no synchr…
Cmux Cache Substitution Correctness ✅ Passed PASS. The reviewed range changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. The diff updates mock responses and request assertions, and removes an obsolete tes…
Cmux No Hacky Sleeps ✅ Passed PASS. The PR changes only two Swift test files. It adds no fixed sleeps, timers, polling, delayed dispatch, or wall-clock waits. The custom check applies to non-Swift production/runtime changes, and t…
Cmux Algorithmic Complexity ✅ Passed The pull request changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. The diff removes one test and updates mock responses, request expectations, and assertions …
Cmux Swift Concurrency ✅ Passed PASS: The pull request changes only two XCTest files. The added lines update mock response data, add a surface.catalog case, update expected request order, and guard the bind-request assertion. No a…
Cmux Swift @Concurrent ✅ Passed PASS: The pull request changes only two Swift test files. The added and edited test code is synchronous (throws), and the diff introduces no async, nonisolated, @concurrent, or @MainActor de…
Cmux Swift Package Boundaries ✅ Passed PASS: The pull request changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. These are test-target files, not production Swift files. The boundary policy also exp…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The reviewed range changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. The diff contains no Package.swift, Package.resolved, .gitignore, workflow, X…
Cmux Swift Logging ✅ Passed PASS. The PR changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. It removes one test and updates mock responses, request expectations, and assertions. It adds n…
Cmux User-Facing Error Privacy ✅ Passed PASS: The authoritative diff changes only two files under cmuxTests/. It updates XCTest mock responses, request assertions, and removes an obsolete test. The repository rule explicitly allows tests …
Cmux Full Internationalization ✅ Passed PASS. The authoritative diff changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. It removes one test and updates mock responses, comments, and assertions. The c…
Cmux Swiftui State Layout ✅ Passed PASS: The pull request changes only two XCTest files. The diff removes one obsolete CLI regression test and updates mock RPC responses, request expectations, and failure handling in `VMSSHCommandTests…
Cmux Architecture Rethink ✅ Passed PASS: The pull request changes only two Swift test files. The diff removes one obsolete test and updates mock responses, request expectations, and an assertion guard. It adds no timing or blocking rep…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The authoritative PR diff changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. It removes one test and updates test-only mock responses, request expectatio…
Cmux Source Artifacts ✅ Passed PASS. The authoritative diff changes only two hand-written Swift test files under cmuxTests/. It removes an obsolete test and updates mock responses, request expectations, and assertions in an exist…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The authoritative PR diff changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. No Swift file under a production Sources/ path is changed, and the added c…
Cmux No Ambient Global State ✅ Passed PASS: The authoritative diff changes only cmuxTests/CLIVMLayoutEnvTests.swift and cmuxTests/VMSSHCommandTests.swift. The changes remove one test and update mock responses, request expectations, an…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@github-actions

Copy link
Copy Markdown
Contributor

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

@ejc3
ejc3 marked this pull request as ready for review September 14, 2026 18:17
`testVMSSHAliasUsesCmuxRemoteWhenProviderSSHIsUnmanaged` expects `cmux vm ssh`
to fall back to cmux-remote when provider SSH is unmanaged. Three parts of its
mock no longer match what the app sends or what the CLI asks for:

- The CLI falls back when the error's `data.backend_code` is
  `vm_attach_transport_unsupported`, and the app sends that code inside `data`
  under a `vm_error` code. The mock put the provider code at the top level with
  no `data`, so the CLI never fell back.
- Since manaflow-ai#12042, the CLI only dials a machine it has not opened before when
  `vm.cmux_remote_info` reports `trusted_carrier: true`. The mock left that out,
  so the CLI stopped with "The Cloud machine is still preparing remote access".
- Since manaflow-ai#12227, the CLI reads `surface.catalog` before choosing between
  projecting an existing terminal and opening a new one. The mock did not answer
  it. The mock now reports a connected machine with no remote workspaces, which
  keeps the test on the new-terminal path it was written for, and the expected
  request sequence includes the catalog request.

After the first mismatch the test read `bindCommands[0]` from an empty list and
crashed the test host. Send the app's response shapes, and fail instead of
crashing if the bind requests are missing.

`testVMResizeIsNoLongerAVerb` came in when `vm resize` was removed. manaflow-ai#12442
restored the verb on purpose and covers it in `tests/test_cli_vm_resize.py`,
so this test now fails by design. Remove it.
@cursor

cursor Bot commented Sep 19, 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.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Thank you for this! You had it first, and main has the same fix now (a99d2de), so I'm closing this one as done. Appreciate it :)

@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 25, 2026
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.

3 participants