Skip to content

acpmux: chat window rows carry their label and place (Rust migration A7, list 1) - #31798

Open
lawrencecchen wants to merge 4 commits into
feat-cmux-nextfrom
rust-migration/chat-lists
Open

lawrencecchen wants to merge 4 commits into
feat-cmux-nextfrom
rust-migration/chat-lists

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Rust migration A7 (rm-chat-lists), list 1 of the chat-list moves: the text a chat list row shows.

What changes

Each row of acpmux's _acpmux/chats_window now carries two fields:

  • label: the row's shown name. It is the chat's title. With no title, it is the window's untitled word (the palette sends "New chat"), else the folder's last part (with lastPathComponent rules: a trailing / is dropped, / stays /), else the harness name as the row serializes it.
  • place: the chat's folder, with ~ for the home folder. This is the same function that the palette ranker uses for its subtitle, so a palette row shows the text that the ranker matched.

The Swift copies of these rules are deleted:

  • AgentChatsPalettePage.items: title ?? untitled and abbreviatingWithTildeInPath. They were also a second copy of the ranker's rank_entry title and subtitle (layer-ownership.md rule 4).
  • SidebarChatHistoryFeed.historyChat: title ?? lastPathComponent ?? harness.

Swift keeps a raw-field fallback (label ?? title, place ?? cwd) only for a running daemon from before this change. The version handoff replaces such a daemon when the app starts.

The daemon change has no FFI pin step: acpmux is a daemon, not cmux-app-ffi.

Tests (fleet, run by me)

Changelog

none

Observed / Not run

Observed: the fleet results above (filled in when they end). prepush-gate.py: no new violations.

Not run:

  • No perf A/B: the change adds two short strings per window row in the daemon and removes string work from the main thread; no list here is on a per-frame path.
  • place equals Swift's abbreviatingWithTildeInPath for a folder at or under the home folder, and for every other folder. I did not compare the two on a Mac for unusual paths (for example .. parts); both leave such a path as it is, by reading.

… shows (red)

The sidebar history and the palette chats page each work out a chat's shown
title and folder in Swift (SidebarChatHistoryFeed.historyChat,
AgentChatsPalettePage.items), and the palette ranker in acpmux works out the
same text a second time to match it (window.rs rank_entry). This test asks
the window rows for that text: it fails until acpmux sends it.

Rust migration A7 (rm-chat-lists).
Each row of `_acpmux/chats_window` now carries the text its list shows:
`label` (the title; with none, the window's `untitled` word, else the
folder's name with `lastPathComponent` rules, else the serialized harness)
and `place` (the folder, `~` for the home folder). `place` is the same
function the palette ranker uses for its subtitle, so a palette row shows the
text the ranker matched. Clients stop working these out (layer-ownership.md
rules 1 and 4: one copy of a rule, in Rust).

Rust migration A7 (rm-chat-lists).
… label and place

AgentChatsPalettePage.items and SidebarChatHistoryFeed.historyChat no longer
work out a chat's shown title and folder: they show the row's `label` and
`place` from acpmux's window (previous commit). The Swift copies of the
title fallback (title, else the page's word / the folder's name / the
harness) and of the `~` folder text are deleted. A raw-field fallback stays
only for a daemon from before labels, until the version handoff replaces it.

Rust migration A7 (rm-chat-lists).
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 44c04c6: 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=pr31798 -f ref=rust-migration/chat-lists -f platform=macos -f backend_mode=local

web and size checks: green

Default changes

This pull request changes 1 default at b77d9b28cdcf (what everyone gets without configuring anything):

  • tunable-source.CmuxNextAgentLanes/SidebarChatHistoryFeed.swift:historyChat: "22b1ce3eb88be73456abad0f6f81fcfa43a91406ace1db0079ad268f97e9c3bb" -> "05e8b1e8ff5b857ef7fcefb3f1361a475438907043be9cdb6a7a78080833cb5e"

Needs a review. It passes with an independent review receipt on this commit: a comment or review whose first line is Review (<reviewer>): SHIP at b77d9b28cdcfcc878c3b52b010ee05695a9f4d4c, from a member, not a self-review (the newest verdict naming the head decides; a later BLOCK beats an earlier SHIP), or with an approving review on this commit from @austinywang or @teamleaderleo (the call owners in .github/defaults-call-owners.json, never the author). A receipt posted as a review re-runs this check; after a plain comment, re-run it. The needs a call label alone does not pass, and a new push needs a new receipt.

Regenerate the snapshot with python3 scripts/ci/defaults_guard.py snapshot.

@blacksmith-sh

blacksmith-sh Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
main.DeveloperIdAppTests.test_download_prints_the_active_profile_only/
test_download_prints_the_active_profile_only
View Logs
stubs: /System/Library/Frameworks/Security.framework/Versions/A/Security has no stub/
Security has no stub
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_CFRetain']/
CoreFoundation, missing 1: ['_CFRetain']
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_log2']/
CoreFoundation, missing 1: ['_log2']
View Logs
test_every_compile_job_names_its_warm_sources (main.ThisTree.test_every_compile_job
_names_its_warm_sources)/
test_every_compile_job_names_its_warm_sources (main.ThisTree.test_every_compile_job
_names_its_warm_sources)
View Logs
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)/
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)
View Logs

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

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Check classification at 44c04c6: fleet clippy step ae05c7dcd3cc28e147f1a91b failed on a base finding, not on this PR. The error is clippy::suspicious_open_options in cmux-tui/crates/cmux-tui/src/plugin_manager.rs:233 (PluginUpdateLock::acquire, from #31462 / 765d3c4). This PR does not change that file. The line is the same on base origin/feat-cmux-next ad01dee. The open fix is #31697. Log: https://github.com/manaflow-ai/cmuxterm-hq/actions/runs/41388477162. I will run clippy again after #31697 lands, so that acpmux is also checked to the end.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-reviewer-a7): SHIP at 44c04c6

I read the diff and the full files at the head. I compared them with origin/feat-cmux-next. I did not compile or run anything on my machine.

What I checked

Sidebar history label is the same as before (base: title ?? lastPathComponent(cwd) ?? harness, with Swift's decoder dropping an empty title and cwd). The history windows (earlierSpec, pinnedSpec) send no untitled, so label() uses the folder, then the harness.

  • Empty title: Rust filter(!is_empty) does what the Swift decoder does. Same.
  • nil or empty cwd: row_value filters out an empty cwd, so the label falls to the harness. Same.
  • /a/b/ and /a/b//: the trim gives b. lastPathComponent also gives b. Same.
  • / and //: Rust gives /, and so does lastPathComponent. Same.
  • A relative path like proj: gives proj in both. Same.
  • Harness: the label reads value["harness"], the serialized kebab-case string that Swift decoded as chat.harness. Same string.
  • activity.decorate adds only attention, preview and parent. It never changes title, so entry.title is the title Swift saw.

Palette title is the same as before. The palette spec always sends untitled: AgentChatsPaletteStrings.untitled, so the label is the title, else that word. That is the old title ?? untitled. The ranker's title is the same value, so a row now shows the text the ranker matched.

Palette subtitle. place() is the function the ranker already used for its subtitle, so the shown subtitle and the matched subtitle are now one value. For a folder at the home folder or under it, and for a folder beside it (/Users/meow against /Users/me), it gives what abbreviatingWithTildeInPath gives. The app passes HOME through to the daemon (AcpmuxDaemonLauncher), so the daemon's dirs::home_dir() is the app's home folder.

Every path builds the row the same way. The plain path and the keys path use decorated = row_value. The query path calls row_value before its top_level check, which is where chat_value was built before. So every row gets label and place from one function.

Diffs. A row value now depends on the chat, its activity, spec.untitled and the home folder. untitled is part of WindowSpec's equality, so a change to it starts a new window and sends every row again. The home folder does not change while the daemon runs. So an unchanged chat gives the same value and causes no diff (*sent == next, and the per-row != compare).

Old daemon. An older daemon sends no label or place, and its rows decode to nil for both. Swift then falls back to the raw fields. A daemon that is not attached gets replaced by AcpmuxVersionHandoff at launch. Clients that do not know the new fields ignore them, because chats_window_changed is untyped in acpmux-protocol.json and in the TS methods.

Tests.

  • The Rust integration test asserts the label and place values on the plain path (history), on a counted palette window and on a query window.
  • Your red run at c106c06 failed on its first assertion (all labels null). So the test fails without the fix.
  • The test runs hermetic (daemon_env::enter re-execs the process with HOME=<scratch>/home before main). The hub runs in the same process, so the dirs::home_dir() that the test reads is the one rows() reads.
  • The unit tests cover the cases of the old Swift rules.
  • The palette Swift test proves that the page reads place: the old code would show /Users/me/cmux for that row.

Binding rules. This change follows layer-ownership.md rule 1 and L1/L3, because the rule now lives in Rust and Swift only shows what the daemon decided. It also follows rule 4/L5: one place() is used by both the ranker and the row, and the Swift copies are deleted. It adds no new hand-written wire type. The AcpmuxChat decoder was already hand-written, which is existing L6 debt. There are no sleeps, no polling and no main-thread work.

Non-blocking notes

  1. AgentChatsPalettePage.swift:175: with an older daemon, an untitled chat now shows an empty title (?? ""), where it used to show "New chat". The sidebar history also loses the folder-name fallback. This happens only until the handoff replaces the daemon, and only for a daemon that is not attached (an attached daemon is never handed off). I accept it as a transition cost. If you want readable rows in that window of time, ?? AgentChatsPaletteStrings.untitled is one word.
  2. window.rs:175: rank_entry keeps its own title.filter(..).unwrap_or(untitled), which is a second copy of the first two branches of label(). For a window with no untitled, the ranker matches "" while the row shows the folder name. No such query window exists today. Calling label() from rank_entry would make them one rule.
  3. window.rs:351 against :233: rank_entry gets the cwd without the empty filter, so an empty cwd gives a ranker subtitle of "" and keyword "", while place is absent. This is harmless.
  4. Trailing slash: place("/work/p1/") keeps the slash. I could not confirm from the docs whether Darwin's abbreviatingWithTildeInPath removes it. Recorded working folders seldom end in /. The PR body already lists this as not run.
  5. rows() now calls home_folder() on each recompute of every window, not only for queries. With HOME set, this is an env read. Without it, dirs uses getpwuid_r. A OnceLock would remove the question (perf-lead D-rules: no I/O on a request path).
  6. Tests: the Swift history test's labels (cmux, claude-code) are the same as the old Swift rule's output, so that test alone does not prove that the feed reads label. A label that differs from the derived value would prove it. The Rust test does not cover the keys (pinned) path, but that path uses the same decorated closure.
  7. Docs: the Row comment (window.rs:148) and the row shapes in server/chat_windows.rs's module doc do not mention label and place yet.

Fleet

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Check classification at 44c04c6. This PR changes none of the files these checks name.

@lawrencecchen lawrencecchen 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 Nov 2, 2026
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 2, 2026 05:31 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 2, 2026 05:31 Inactive
@cursor

cursor Bot commented Nov 2, 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.

@lawrencecchen lawrencecchen removed the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 2, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-reviewer-a7): SHIP at b77d9b2

This head is cmux-bot's merge of feat-cmux-next (85ed087) into 44c04c6, which I reviewed before (#31798 (comment)). I did not compile or run anything on my machine.

What I checked:

  • The PR's own diff did not change. I compared git diff <merge-base 85ed0878a38> b77d9b28cdc with git diff 86faa171ca1f 44c04c69eb28. When I ignore the index and @@ lines, they are the same in the same 7 files (+256/-31).
  • The 64 feat commits that came in touch none of these: cmux-tui/crates/acpmux/src/chats/, server/chat_windows.rs, cmux-chat-index, tests/chats_window.rs, tests/support/, CmuxNextAgentLanes/, AcpmuxChat.swift and the two Swift test files.
  • cmux-next: list acpmux sessions with terminal agents (#16417) #31757 changes acpmux/src/deliver.rs and the cmux-tui agent roster. base red: acpmux process pages and retry delays without an index (panic ratchet 697 > 686, #30726, #31709) #31773 changes hub/processes.rs and hub/auto_resume/settings.rs. In CmuxNextAgentPane, only AgentPaneOwnerAvatars.swift and AgentPaneView+NewTab.swift changed. None of these touches chat_value, ChatActivity, the window rows or AcpmuxChat.
  • No incoming change adds a label or place field to the chat value, and none adds a new AcpmuxChat( caller. The new init arguments have defaults, so existing callers still compile.

My earlier findings stay as they were: none of them blocks the merge, and the non-blocking notes in the first comment still apply. The Swift suites step (3e4998cd051637a5030a6dbe) ran on 44c04c6. Its result still applies to this head, because the merge changed none of the files those suites test.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

For awareness: this PR conflicts with feat-cmux-next, so it can't land until it's rebased or base is merged in. Its current review verdict stays as is; a hand-resolved merge gets a fresh review at the new head.

This branch was previously deployed

3 inactive deployments
ci-routing-agent — b77d9b28 Deployed Nov 2, 2026 by github-actions[bot] via cmux-next macOS placement #61581
artifacts — b77d9b28 Deployed Nov 2, 2026 by cmux-bot[bot] via publish tree-addressed cmux-tui #99501
sentry-dsyms — b77d9b28 Deployed Nov 2, 2026 by cmux-bot[bot] via upload cmux-tui dSYMs to Sentry #99501
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants