Skip to content

cmux NEXT: GUI Delete Group and group edits name the group's sidebar document item - #23059

Merged
austinywang merged 8 commits into
feat-cmux-nextfrom
feat-cmux-next-22996-group-delete
Oct 21, 2026
Merged

austinywang merged 8 commits into
feat-cmux-nextfrom
feat-cmux-next-22996-group-delete

Conversation

@austinywang

@austinywang austinywang commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #22996

On the sidebar document, Delete Group from the GUI said "The sidebar did not make this move", and Rename and the other group edits failed the same way. The CLI's cmux workspace group <group> delete worked for the same group.

Cause. The daemon lists a personal group (grp_…) as the document item itm_g_grp_…. The app's group edits put whatever group id they were given into the sidebar.apply op. When that was the personal id, the reducer refused it unknown_item, and the app shows any refusal other than cross-machine or provider-owned as "The sidebar did not make this move". The CLI writes the personal row instead, and the daemon syncs that row into the document.

Reproduced on an isolated throwaway headless daemon from the installed cmux NEXT (its own session, HOME and socket; never the user's session): one group with one member.

  • sidebar.apply [{op: group.update, id: itm_g_grp_…, patch: {name}}] and then group.ungroup with the same id: both commit.
  • group.ungroup with id: grp_…: validation.invalid, reason: unknown_item.

Fix. SidebarDocStore.item(group:) resolves a group id to the document item it names: the id itself when the document holds a group under it (a group the document made, or an item id already), else itm_g_<id>. Every op that names a group goes through it: Ungroup/Delete, Rename, color, icon, fold (both paths), pin, move into a group, and group reorder.

Tests. SidebarDocGroupEditTargetTests (CmuxNextAppTests) runs on a shortened copy of a real sidebar.get document: a personal id resolves to its itm_g_ item, an item id and a group the document made keep their own id, and a non-group or missing id is passed through unchanged.

No capture: no pixels change. The diff is sidebar-document store logic (which group id the ops name). The visible effect is that Delete Group, Ungroup and Rename now succeed instead of showing a refusal; that behavior is checked in the tagged-build GUI run in the PR comments.

Changelog

Fixed: In cmux NEXT, Delete Group, Ungroup, Rename and the other group edits from the sidebar work again instead of saying "The sidebar did not make this move".

Observed / Not run

Observed (head 49a2f3a; the PR diff is unchanged since c75bf8a, the later commits only merge feat-cmux-next):

  • Live daemon repro on the installed cmux NEXT (isolated session g22996, then stopped): sidebar.apply with itm_g_grp_… commits; with grp_… it returns validation.invalid / unknown_item.
  • Focused suites on cmux-ci (step da2983250a9823b6d3e188dc, aws-m4pro-7, Xcode 26.6): package-test-lane.sh suite Packages/macOS/CmuxNext SidebarDocGroupEditTargetTests,SidebarDocSubgroupPersonalParentTests,SidebarDocGroupTargetTests,SidebarDocShownGroupIDTests,SidebarDocGroupEditorTargetTests. All 5 suites passed.
  • Tagged build 22996-gui-delete-group (job a5d0cd0a0b102524f4f90819, c75bf8a). In the GUI on throwaway groups, Delete Group, Ungroup and Rename worked with no refusal (details in the comment below).
  • scripts/cmux-next/prepush-gate.py: no new violations.
  • Correctness review by a subagent: PASS. Its two follow-ups are fixed in c75bf8a.

Not run: the full CmuxNext suite. CI's cmux-next swift test covers it.

Red/green: the new test calls SidebarDocStore.item(group:in:), which the base does not have, so on the base it fails to compile rather than failing an assertion. The behavioral red on the base is the daemon repro above.

🤖 Generated with Claude Code

…ent item (#22996)

Delete Group, Ungroup, Rename, color, icon, fold, pin, move-into and
reorder of a group sent the group id they were given. When that was the
personal id (grp_…), the daemon lists the group as itm_g_grp_… and the
reducer refused unknown_item, which the app showed as "The sidebar did not
make this move". The CLI writes the personal row and worked.

SidebarDocStore now resolves a group id to the document item it names (the
id itself when the document holds a group under it, else itm_g_<id>)
before it builds the ops.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 24cbbf2: 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=pr23059 -f ref=feat-cmux-next-22996-group-delete -f platform=macos -f backend_mode=local

Unclear: GhosttyKit release check failed: no known signature; failed step: Verify published GhosttyKit release and checksum.

CI failed on 24cbbf2208 (run 39671805819 attempt 1): 1 unknown.
macOS jobs did not run: compile and app tests were skipped: Fast static checks skipped.

Job Verdict Why
GhosttyKit release check unknown no known signature; failed step: Verify published GhosttyKit release and checksum

Not re-run automatically: GhosttyKit release check is not a machine failure.

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); base red: the same failure on the base d10176f; does not block

CI fast guards failed on 24cbbf2208 (https://github.com/manaflow-ai/cmux/actions/runs/39671805819). It does not block the merge; a red guard merged into main breaks it for every open PR.
The log named no failed step; see the run.

Agents: python3 scripts/ci/guard_attribution.py fix applies the mechanical fixes locally.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Bead cx-hi7g8 created for this PR by the cmux coordinator.

@austinywang austinywang added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 21, 2026
… review)

SidebarDocStore.item(group:) reuses SidebarDocRecords.groupItem(personal:),
and New Subgroup named by a personal group id plans under its document
item instead of refusing "group required".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@austinywang

Copy link
Copy Markdown
Contributor Author

Correctness review (subagent) on c023e11: PASS

The reviewer read the code only; nothing was built.

  • Compile: no shadowing or isolation problem. SidebarDocStore is @MainActor, groupItemID is nonisolated, and the test's document decodes with SidebarDocRecords(document:).
  • Holds up: a just-created group (in the mirror, not yet in records) keeps its own id, matching its group.create. Pinned groups, the fold path and a non-group id that is passed through also hold.
  • Finding 1 (why the bare id reaches the store): SidebarModel.groupID(personal:) falls back to the bare grp_… when the model finds no row (for example a nested group). Resolving at the store boundary covers every caller. No change needed.
  • Finding 2 (New Subgroup with a personal-id target was refused "group required"): fixed in c75bf8a. SidebarDocSubgroupPlan resolves the parent through groupItem(personal:). Test: SidebarDocSubgroupPersonalParentTests.
  • Finding 3 (two id rules in different orders): fixed in c75bf8a. SidebarDocStore.item(group:) now reuses SidebarDocRecords.groupItem(personal:).
  • Finding 4 (reorderGroup matches the model's id): not a regression, since drags carry the shown id. No change.

@austinywang

Copy link
Copy Markdown
Contributor Author

Check classification at c75bf8a: defaults-snapshot (cmux-next guards) fails in check-menu-actions.py on Packages/macOS/CmuxNext/Sources/CmuxNextAgentLanes/ChiefPaneTools.swift, a file this PR does not touch. It is a base finding: the same guard is red on feat-cmux-next (100ec72, 3020a01, 3aff6ed). Not blocking.

@austinywang

Copy link
Copy Markdown
Contributor Author

GUI check on the tagged build 22996-gui-delete-group (cmux-ci job a5d0cd0a0b102524f4f90819, head c75bf8a)

The app ran in the background on this Mac (open -g, CMUX_NEXT_NO_ACTIVATE=1, its own tagged session) and was quit afterwards. Each step used throwaway groups and ran the group header menu's action with the person's origin and the id the sidebar row shows. The harness is a copy of scripts/cmux-next/sidebar-group-delete-live.py, which another lane wrote for this issue.

Step Result
Delete Group on a group the CLI made (itm_g_grp_…) PASS: group gone, member stays ungrouped, no refusal
Ungroup on a group the CLI made PASS
Rename on a group the app made (New Group) PASS: the document has the new name
Delete Group on a group the app made PASS
Pin a group, Rename it in Pinned PASS
Ungroup and Delete Group on a pinned group Group gone, no refusal. The harness marks FAIL because its row lookup does not read Pinned's block rows. The screenshot shows the former member still pinned (Delta-48c1 in Pinned), which is the expected place.

No step showed "The sidebar did not make this move".

@austinywang

Copy link
Copy Markdown
Contributor Author

Check classification at 87babe7 (after merging origin/feat-cmux-next to pick up #23018 test-target compile fix):

@cursor

cursor Bot commented Oct 21, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@austinywang austinywang added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 21, 2026
@austinywang

Copy link
Copy Markdown
Contributor Author

Check classification, cmux-next swift test at 49a2f3a (run 39564060520, job 119106822330): not caused by this PR.

  • 16 red tests in AccountsCmuxRow, ActionRunCreated, AgentCursorNameTag/SoftStyle/TagLink, HomeChiefSettingsView, HomeMessageRequests, LegacyHistoryFile, NativeTooltipGuard, NowPlayingCardDismiss, PaneHorizontalScrollClaim, RowCloseMotion, SidebarRowGutter and TabCloseWidthChange. None of them goes through the code this PR changes, which is only group-id resolution in SidebarDocStore's group ops and SidebarDocSubgroupPlan.
  • The run also ended with 274 tests started and never finished (crash, kill or truncation), so it is not a clean result. The one group test it names, theMoreButtonOpensTheGroupEditorUnderTheButton, is in that never-finished list, not the red list.
  • This PR's own suites pass on the fleet at this head (step da2983250a9823b6d3e188dc). I will rerun the failed job on the same SHA once the run completes.

@austinywang austinywang added ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds and removed ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds labels Oct 21, 2026
@blacksmith-sh

This comment has been minimized.

@austinywang austinywang added ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds and removed ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds labels Oct 21, 2026
@austinywang

Copy link
Copy Markdown
Contributor Author

Check classification at 027e13e. None of these reds is caused by this PR:

Merged feat-cmux-next again. The new head is 97584f3, and this PR's own diff is unchanged (5 files). ready-to-land is reapplied.

@austinywang

Copy link
Copy Markdown
Contributor Author

Review (subagent, correctness first): SHIP at 97584f3

This review read the PR's own 5 files, diffed against the current feat-cmux-next. Nothing was built.

  • Compiles against the current base. SidebarDocRecords.groupItem(personal:) is nonisolated and is called correctly from the @MainActor store, from the static item(group:in:) and from the nonisolated SidebarDocSubgroupPlan.
  • No regressions. A group the document made keeps its own id, and a just-created group that is not in records yet keeps its raw id. Nested and pinned groups are covered, and so are the toggle-collapse and reorder paths.
  • The tests match the code: itm_g_<id> is checked before <id>, and the subgroup parent resolves to itm_g_grp_1.
  • Minor, not blocking: groupItemID(personal:) and groupItemPrefix in SidebarDocStore+Groups.swift duplicate the prefix in SidebarDoc.swift. The new tests live in CmuxNextAppTests but cover CmuxNextSidebar code.

My own check at this head: the PR diff is unchanged since c75bf8a, which the first review's follow-ups produced. The later commits only merge feat-cmux-next. The focused suites passed at 49a2f3a (same PR diff), and the GUI check ran on the tagged build.

@austinywang

Copy link
Copy Markdown
Contributor Author

Review (subagent, correctness first): SHIP at 0717db1

0717db1 only merges feat-cmux-next into 97584f3, which the subagent review passed (SHIP, comment above). The PR's own diff is byte-identical between the two heads: 82 changed lines, sha1 f2864eb59d111a9aaa45123417d6dced845c5f3b for both.

Check classification at 97584f3:

  • defaults-snapshot: check-menu-actions.py flags FileExplorer/FileExplorerOpenWithMenu.swift and FileExplorerSectionView.swift. Those are new files on the base (dd018d4), not in this PR. On the merged tree at 0717db1 the guard passes locally (ok: 41 inline menu rows in 31 baselined files, none new).
  • cmux-next swift test: 6 red tests (AccountsCmuxRow, AgentCursorTagLink, HomeChiefSettingsWords, NowPlayingCardDismiss, PaneHorizontalScrollClaim, TabCloseWidthChange), none of them on this PR's code path. 1037 tests started and never finished (crash, kill or truncation). The one group test the annotation names is only in that never-finished list. This PR's suites pass on the fleet (step da2983250a9823b6d3e188dc).

@austinywang austinywang added ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds and removed ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds labels Oct 21, 2026
@austinywang

Copy link
Copy Markdown
Contributor Author

Check classification at 0717db1: cmux-next generated files (job 119355998418) failed during Set up runner: glaeda-cmux-runner-hook: refused: a fleet build is waiting for the host. No step of the check ran, so this is an infrastructure refusal, not a red from this PR. I will rerun that job on the same SHA once run 39638487840 completes (GitHub refuses a job rerun while the run is in progress).

@austinywang

Copy link
Copy Markdown
Contributor Author

cmux-next swift test at 0717db1 (job 119355998255): 0 red tests. The helper ended with no Test run with summary and 1798 tests started and never finished (crash, kill or truncation). That is no assertion result, so it is not a red from this PR. I will rerun it with generated files (infra refusal) once Release compile finishes and the run completes.

@austinywang

Copy link
Copy Markdown
Contributor Author

Review (subagent, correctness first): SHIP at 24cbbf2

The head moved from 0717db1 only through a merge of feat-cmux-next (24cbbf2). The PR's own diff is byte-identical (5 files, +72/−10; the hash of the changed lines is f2864eb5 at both heads), so the earlier SHIP review carries over. The dev build 22996-gui-delete-group (c75bf8a) has the same diff.

@austinywang austinywang added ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds and removed ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds labels Oct 21, 2026
@blacksmith-sh

blacksmith-sh Bot commented Oct 21, 2026

Copy link
Copy Markdown

Found 9 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmp03p0d94l/summary-fail.json (live):/summary-fail.json (live): View Logs
/tmp/tmp22flh5qh/summary-missing.json (live):/summary-missing.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
convention violations found (ERROR lines above)./
convention violations found (ERROR lines above).
View Logs
test_a_file_a_test_reads_selects_that_test (main.PullRequestTiers.test_a_file_a_tes
t_reads_selects_that_test)/
test_a_file_a_test_reads_selects_that_test (main.PullRequestTiers.test_a_file_a_tes
t_reads_selects_that_test)
View Logs
test_a_shared_module_selects_its_dependents (main.PullRequestTiers.test_a_shared_mo
dule_selects_its_dependents)/
test_a_shared_module_selects_its_dependents (main.PullRequestTiers.test_a_shared_mo
dule_selects_its_dependents)
View Logs

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

@austinywang
austinywang merged commit b2f5b94 into feat-cmux-next Oct 21, 2026
@austinywang
austinywang deleted the feat-cmux-next-22996-group-delete branch October 21, 2026 21:18
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 24cbbf22 Deployed Oct 21, 2026 by teamleaderleo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants