Skip to content

fix(cmux NEXT sidebar): a move the daemon did not answer in time is sent again, never undone - #34785

Merged
teamleaderleo merged 4 commits into
feat-cmux-nextfrom
lane--optimistic-move-no-undo-on-timeout
Nov 4, 2026
Merged

teamleaderleo merged 4 commits into
feat-cmux-nextfrom
lane--optimistic-move-no-undo-on-timeout

Conversation

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Leo, on "Move did not apply: set-personal-workspace: timed out: workspace.place (no reply within 2.0 seconds)":

this is really bad 😞 we have the optimistic moving/animation which is good... THEN WE UNDO IT!!! WHICH IS BAD!!!

What changed

A missed deadline or a lost connection is not a refusal. Sidebar organization writes that set absolute values no longer roll back when the daemon is slow. That covers a workspace's group and place (drag, Move to Group, Remove from Group), and a group's order, name, color, pin and icon.

  • Optimistic state stays. DaemonService.performUntilAnswered sends the same request again, spaced by Backoff (0.25 s doubling to 5 s), until the daemon answers. No toast shows while it retries.
  • Rollback only on a definite refusal. Only then does the change roll back, with the existing plain-words toast.
  • Last write wins. PersonalWriteLedger records a claim for each workspace or group field when the person acts. A newer claim stops an older retry before its next send.
  • The placement queue stays serial. At the first unanswered try it releases its action-scope tickets (releaseHeld), so a CLI or script action ends and reads that the move may still apply, instead of waiting on retries.
  • Retries stop on an answer, a refusal, a newer write, a connection shutdown (quit or teardown), or cancellation.

Is it safe to repeat? Yes. set_personal_workspace_in (cmux-tui-core) writes the group column and moves the row to an absolute index, so sending it twice lands the same state. workspace_group order, name, color, pin and icon are absolute sets too.

Not retried: create-personal-group. A second create would make a second group. It keeps its rollback (follow-up below).

Why workspace.place took more than 2 s

#34628 (landed today) already fixed this. Journal maintenance's roster catch-up scanned every non-output journal entry each tick: 5.6M rows on the nightly registry, 5-8 s. That held up workspace.place behind it. The read is now a skip scan, 5 ms instead of 330 ms warm. This PR makes the client correct for any slow answer that remains.

Verification

  • Regression test PlacementWriteOrderTests/anUnansweredPlacementIsSentAgainNotUndone: the fake daemon drops the first set-personal-workspace. The test expects no notice, the moves kept, and all three landing in order after one resend.
    • Red at the test-only commit 1c94ccd: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=PlacementWriteOrderTests, step dd9332b5f79c, failed.
  • The disconnect mode now expects the rows to keep the moves and no notice to show.
  • PersonalWriteLedgerTests covers which errors count as unanswered and the supersede rule.
  • SidebarOptimisticSaveTests: the rename-with-no-daemon test now expects the name to stay with no error.

Observed / Not run

  • Observed: red on base for the new test (above). prepush-gate clean.
  • Not run yet: a green run at the head (one run at 330b9ac failed to compile on a missing import, fixed in 330b9ac→c529fee). This lands as a leo-named-fix, and anything CI finds gets fixed forward. Not tried in a DEV build.

Follow-ups (review)

  • A claim is superseded when any one of its keys is claimed again: a disconnected move of [a, b] followed by a move of a alone drops b's resend, so b snaps back with no notice.
  • Group reorders run outside the queue, so a late resend of group A's absolute index can shift group B.
  • While disconnected, the placement queue blocks and an ending group stays hidden until answered.
  • A notConnected scope failure reads mayHaveApplied: false even though the write is sent again.
  • create-personal-group could retry with a stable idempotency key.

Changelog

Fixed: A sidebar move or group edit no longer snaps back with "Move did not apply" when cmux's background service is slow to answer.

Agent session: agent session

🤖 Generated with Claude Code

teamleaderleo and others added 4 commits November 3, 2026 20:02
…ed on base)

Leo, 2026-11-03, on "Move did not apply: set-personal-workspace: timed out:
workspace.place (no reply within 2.0 seconds)".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
…ent again, never undone

Leo: "we have the optimistic moving/animation which is good... THEN WE UNDO
IT!!! WHICH IS BAD!!!", on "Move did not apply: set-personal-workspace:
timed out: workspace.place (no reply within 2.0 seconds)".

A missed deadline or a lost connection is not a refusal. Personal
organization writes that set absolute values (workspace group and place,
group order, name, color, pin, icon) now keep the optimistic state and go
again with backoff (0.25 s doubling to 5 s) until the daemon answers
(DaemonService.performUntilAnswered). Only a definite refusal rolls back
and toasts. A newer write to the same workspace or group field supersedes
a retry (PersonalWriteLedger, last write wins). The placement queue still
serializes, but releases its action tickets at the first unanswered try so
no script waits on retries. Retries stop at a connection shutdown.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
… error (review F1)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
@teamleaderleo teamleaderleo added leo-named-fix An obvious fix to something Leo named: lands on push, review runs in parallel risk:normal Risk tier (docs/ci/risk-tiers.md) labels Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

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

Review c529fee: 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=pr34785 -f ref=lane--optimistic-move-no-undo-on-timeout -f platform=macos -f backend_mode=local

Shipped in:

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (subagent, claude-opus-5-5 medium, correctness): SHIP at c529fee

Visual verdict: unchanged by this diff (no UI layout change; optimistic rows are now kept on an unanswered write).
Checked: retry loop stop conditions (supersede, shutdown, cancel), the queue hold release on an unanswered first send, the rollback only on a definite refusal, and the test updates (F1 fixed in c529fee). SidebarOptimisticSaveTests and PlacementWriteOrderTests pass at this head (cmux-ci step optimistic-move-head-c529fee).

Follow-ups:

  • F2 [follow-up] category:correctness outcome:pending — a multi-workspace claim counts as superseded when any one key is re-claimed, so b in a move of [a,b] followed by a move of a alone is dropped.
  • F3 [follow-up] category:correctness outcome:pending — group reorders sit outside the serial queue; a late absolute-index resend can shift another group.
  • F4 [follow-up] category:behavior outcome:pending — while disconnected the placement queue blocks and an emptied group stays hidden until answered.
  • F5 [follow-up] category:correctness outcome:pending — a notConnected scope failure reports mayHaveApplied: false while the write is still being resent.
  • F6 [follow-up] category:tests outcome:pending — no-connection tests leave the retry task alive after teardown.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex, router-review): SHIP at c529fee

Reviewer: codex gpt-6-astra high, fast tier.

Follow-ups: 5. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward.

SHIP at c529fee

Visual verdict: no images; Leo tries the build

  1. [P2] [follow-up] [category:correctness] [finding:F1] [outcome:pending] A multi-workspace claim is superseded when any one workspace is rewritten (PersonalWriteLedger.swift:25-40). Moving [a,b], then moving only a, cancels the first retry and settles its optimistic edit, causing b to snap back without a refusal or notice.

  2. [P2] [follow-up] [category:correctness] [finding:F2] [outcome:pending] Group-order retries run outside the serial placement queue (SidebarBridge+Personal.swift:63-71). A late absolute-index resend for group A can therefore run after group B was reordered and shift B to the wrong position.

  3. [P2] [follow-up] [category:reliability] [finding:F3] [outcome:pending] An initially disconnected placement treats notConnected as unanswered and retries indefinitely (DaemonService+UnansweredWrite.swift:31-41). The queue tail remains occupied (PersonalPlacementQueue.swift:51-55), so later placements and ending-group cleanup remain blocked until reconnection.

  4. [P2] [follow-up] [category:correctness] [finding:F4] [outcome:pending] The first notConnected attempt closes its action ticket with mayHaveApplied == false (DaemonService+UnansweredWrite.swift:31-37), because scopeFailure only marks timed-out errors as possibly applied (DaemonService+Scope.swift:38-44). Control clients can therefore receive a definitive daemon_error even though this write is being retried and may apply.

  5. [P2] [follow-up] [category:ux] [finding:F5] [outcome:pending] No after screenshots or gallery artifacts were available for visual comparison.

Follow-ups:

  • [finding:F1] Multi-workspace supersession can revert an unaffected workspace.
  • [finding:F2] Late group-order retries can corrupt neighboring group order.
  • [finding:F3] Disconnected placement retries can block the serial queue indefinitely.
  • [finding:F4] notConnected reports mayHaveApplied: false despite retrying.
  • [finding:F5] Visual comparison remains pending because no images were supplied.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Finding outcome: comment-6622939697/F1 fixed ecdf851
Finding outcome: comment-6622939697/F2 filed-follow-up #34822
Finding outcome: comment-6622939697/F3 filed-follow-up #34823
Finding outcome: comment-6622939697/F4 filed-follow-up #34824
Finding outcome: comment-6622939697/F5 dismissed screenshots never block a land and this diff changes no layout

@lawrencecchen

Copy link
Copy Markdown
Contributor

Swift logic follow-up (tip watcher swift-logic row, STATUS 13027 "SWIFT LOGIC CULPRITS"; lead order).

This PR raised swift_logic_lines by +152 at its merge commit 90a5d96: scripts/cmux-next/swift-logic-lines.py before and after the merge, with the same table (the tip's, c6be943). The largest declarations of the rise:

  • class PersonalWriteLedger in Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Store/PersonalWriteLedger.swift: +46 lines counted as Swift logic (no row in plans/cmux-next/swift-logic-classes.tsv)
  • extension DaemonService in Packages/macOS/CmuxNext/Sources/CmuxNextDaemonService/DaemonService+UnansweredWrite.swift: +44 lines counted as Swift logic (no row in plans/cmux-next/swift-logic-classes.tsv)
  • class PersonalPlacementQueue in Packages/macOS/CmuxNext/Sources/CmuxNextDaemonService/PersonalPlacementQueue.swift: +30 lines counted as Swift logic (its row counts it as logic)
  • extension DaemonService in Packages/macOS/CmuxNext/Sources/CmuxNextDaemonService/DaemonService+UnansweredWrite.swift: +30 lines counted as Swift logic (no row in plans/cmux-next/swift-logic-classes.tsv)

plans/cmux-next/layer-ownership.md says Rust owns every state, list and rule and Swift is the platform shell. Please move the new logic of this PR in a follow-up: to Rust (the crate that owns the area; the owner lists in STATUS 12147-12165 name the target per area), or at least out of the app into its package. If a declaration above is platform code (AppKit, Core Animation, OS calls, strings, tokens, DEBUG tooling), reply here and the table gets a PLATFORM row with your reason instead.

The swift-logic row is a report, not a gate: nothing blocks on this comment.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — c529fee8 Deployed Nov 4, 2026 by teamleaderleo via namespace autofix #13945
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

leo-named-fix An obvious fix to something Leo named: lands on push, review runs in parallel risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants