base red: Cmd-N row slot tests (a placement's own rows pass drew the new row at the end)

Merged
LA
5d ago
+17-17

Summary

Base red on feat-cmux-next (priority zero). Three tests check that a Cmd-N workspace row draws only at its slot from the first frame. On the base they fail often but not every time:

  • CmdNRowSlotTests.cmdNDrawsTheNewRowOnlyAtItsSlot (:79)
  • CmdNInGroupRowSlotTests.cmdNInAGroupDrawsTheNewRowOnlyAtItsSlotInTheGroup (:209)
  • ColdCmdNRowSlotTests.firstColdCmdNWithRestoredFollowingRowsStaysAfterCurrent (:148)

What the person sees: after Cmd-N, the new row draws at the end of the sidebar for one frame, then jumps to its slot.

Cause (an old race, not a recent commit). A temporary event-order trace (scratch branch, job c8b7e3ff4e954c8e515b260c) shows this order in every failing run:

  1. The press draws the pending row at its slot.
  2. Reconcile: the daemon's workspace is now live. applyPendingPlacements takes the placement and clears its entry.
  3. The placement's own refresh (placementSections → sameTurn.syncNow) runs a rows pass before the move is sent. The sidebar document does not place the item yet and does not lag (version == shownVersion). So PendingWorkspaceRows.listing lists the workspace's own row at the daemon's place (the end), forget drops the pending row, and that frame is drawn.
  4. bridge.place sends the move, and the next frame has the row at its slot.

A run passes only when the store still shows the stand-in at step 3. The tests also fail at #27014's merge (58af48e51d8) and at every later point tried (jobs 501830b7, 76d6a7e9, fdc057e7, 989300c4). No single commit caused it, so there is nothing to revert.

Fix. A workspace whose slot WindowManager.pendingPlacements holds keeps its pending row at that slot until the placement is sent:

  • The entry is now cleared after the placement's refresh, right before the move. It used to be cleared before the refresh.
  • PendingWorkspaceRows.unsentSlots, observed and kept per id from pendingPlacements' didSet, makes listing keep the pending row while the id is in the set. When the id leaves the set, the rows pass runs again, also for a placement that sends no move.
  • An anchor wait now updates its entry in place.

There is no wait, retry or delay. The change adds no lines to CmuxNextApp (arch-ratchet app_lines).

The same fix makes NewInNewGroupRowSlotTests pass: it is red at the base and green at the head.

Testing

All runs are on the fleet (scripts/ci/package-test-lane.sh suite Packages/macOS/CmuxNext …). Each suite runs in its own process, by filter CmuxNextAppTests.<Suite>.

The base has an unrelated compile red: AgentSessionAttachClientCloudRepositoryTests from #30832, being reverted in #31023. Because of it, red and green ran on a scratch pair that is never merged:

  • base b33ff910 = origin/revert/30832-cloud-repo-tests
  • head bf5d0de3 = this branch merged with that revert
  • The diff between the two is exactly this PR's 3 files.
  • Compile 568bf412e4357b6fb3679c3a passed (CmuxNext, app, cmux-tui).
suite (runs)base b33ff910, job 569b74b326d9215319079152head bf5d0de3, job d6325ac61aa65ab5f2b77b46
CmdNRowSlotTests (3)1 red (:79)3 green
CmdNInGroupRowSlotTests (3, 4 cases each)3 red (:209, afterCurrent, both instantPage)3 green
ColdCmdNRowSlotTests (5)1 red (:148)5 green

More runs:

  • Head 42e34005985a404cb3bcb9db: ColdCmdN 4 of 4 green (9 of 9 at head in total); CmdNInGroup 2 of 2 green isolated.
  • Base 9c948a88edacbc16cef0a662: cmdNDraws red (:79).
  • Earlier reds at feat commits: 396855a8a312466572ff426d and 7adc0ba81250c7e855359687 (ColdCmdN 3 of 5 red), both at e25bc25a. Isolated at feat tip 86989822: 78e8567a77c1016de04b3237.

Neighbour suites (17), base 5a165f5875349b38b22b60ac vs head 05a772b114c67a826903c257:

  • 15 pass at both (CmdNTopInGroupFrames, ColdCmdNWindowRows, NewWorkspacePlacementRace, PlacementStaleRows, PlacementWriteOrder, PlacementDocumentWriteOrder, RapidWorkspaceCreate, SidebarDocNewWorkspaceTop, AgentSessionGroupPlacement, AgentSessionGroupDirectPlacement, ChiefSidebarPlacement, CmdNSlowDaemonKeepsWorkspace, CmdNAlwaysNewWorkspace, LinkOpenPlacement, SidebarInsertBurst as run without a display).
  • NewInNewGroupRowSlotTests is red at base and green at head.
  • SidebarCmdNumberTwiceTests is red at both: a base finding, not caused by this PR.

Observed / Not run

  • Observed: the table above. In the 3-suite process run at head (42e34005), one case failed at CmdNRowSlotTests.swift:162: the test's own New Group setup did not form within 30 s. That is a different check from the slot. The same :162 failure shows at base commits without this change (34129b0c at d2b6b65d, 55865c40 at 49f371cc), so it is a separate intermittent base finding.
  • Not run: SidebarInsertBurstTests on a display worker (it skips without one); a tagged dogfood build. Not measured: frame timing (the change removes one rows pass's wrong frame; it adds no work on the main thread except a set update when a placement is added or removed).

Changelog

Fixed: A new workspace's sidebar row no longer appears at the bottom for a moment before it moves to its place after Cmd-N.

Proof

No capture: the bug lasts one frame and depends on timing. The frame-by-frame row checks in the three tests above are the evidence. A still image cannot show a one-frame jump.


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


Summary by cubic

Fixes a one-frame race where, after Cmd-N, a new workspace row drew at the end of the sidebar before jumping to its slot. A workspace whose slot WindowManager.pendingPlacements holds now keeps its pending row at that slot until the placement is sent, so the row draws correctly on the first frame.

Bug Fixes

  • The placement entry is cleared after the rows pass, right before the move, instead of before it.
  • PendingWorkspaceRows.unsentSlots keeps the pending row listed while the placement is unsent, and re-runs the rows pass when the placement goes, even for placements that send no move.
  • Anchor waits now update their entry in place instead of clearing and resetting it.
  • No wait, retry, or delay is added; the change stays line-neutral in CmuxNextApp.