fix: make the optimistic Cmd-Y Cloud tests pass and keep their tree reveal

Merged
TE
1mo ago
+39-7

Main is red on CloudWorkspaceOptimisticShortcutTests, which landed in #18895 (Austin). That PR's own app-host unit tests (changed suites) job was already failing on these tests, and they have failed on every main run since. See #19109.

Three separate problems, one commit each:

  1. Fixture bindings went nowhere (test). CloudWorkspaceCreationSidebarFixture(useSharedCatalog: true) kept the rename service the host app delegate installed on SurfaceCatalog.shared. That service resolves workspaces through the host delegate's windows, which never contain the fixture's TabManager. So bindCloudWorkspace did nothing, and reservations never got a cloudVMBinding, which made :117 and :210 fail. The fixture now installs a rename service bound to its own manager while it is alive and restores the previous one on close. LastSurfaceClosePreferenceTests already uses this pattern.
  2. Double wait (test). optimisticSelection waited twice on the single entered signal in every case except providerDelay. Once the lookup passed, the stay case hung for the full 300s time limit. Only providerDelay waits again now.
  3. Reveal withdrawn for a workspace that stayed selected (product). Cmd-Y selects its pre-admitted reservation itself, and that selection advances the window's Cloud selection revision. When the create finished, focusCreatedCloudWorkspace saw the revision mismatch and returned nil, so revealing withdrew the Cloud tree reveal even though the user never left the workspace. Now a created workspace that is still selected in its key window keeps its reveal. A revision change still blocks selecting a workspace the user navigated away from, so the ⌘N away/awayAndBack reveal tests keep their meaning. The test's providerDelay case expected the original workspace to stay selected. That expectation predates the change to admit the reservation before provider lookup, so it now expects the same result as stay.

Merged with current main (#19145–#19148).

Not verified locally (app builds are not allowed on this machine). Full CI was dispatched on this branch.

🤖 Generated with Claude Code


[!NOTE]
Low Risk
Narrow focus/reveal guard around post-create selection; test-only catalog rename wiring with restore on teardown.

Overview
Fixes failing optimistic Cmd-Y (CloudWorkspaceOptimisticShortcutTests) coverage and a product bug where the Cloud sidebar reveal was withdrawn after creation even when the user never left the new workspace.

focusCreatedCloudWorkspace now returns immediately when the created workspace is still the active tab, instead of treating the selection-revision bump from pre-admitting the reservation as “user navigated away.” Revision checks still block re-selecting a workspace the user left during the async create.

Test harness: CloudWorkspaceCreationSidebarFixture(useSharedCatalog: true) temporarily replaces SurfaceCatalog.shared’s rename service with one bound to the fixture’s TabManager (restored on close), so bindCloudWorkspace and cloudVMBinding work under the shared catalog.

Tests: optimisticSelection no longer waits twice on the single entered signal (which hung stay), treats providerDelay like stay for selection and reveal expectations, and only re-waits on entered in the provider-delay path.

Reviewed by Cursor Bugbot for commit 588c68d7cfdb80202b79fdc4bb6a520556780980. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes
    • Cloud workspace creation now preserves the newly created workspace as the selection when it is already selected, even if other selection changes occurred during creation. If another workspace is selected instead, creation only changes the selection when no intervening selection change has occurred.
    • While a workspace provider lookup is delayed, the pending workspace remains selected and its reveal stays active until creation completes. This keeps the in-progress workspace visible without overriding a different selection made during the delay.