acpmux: prompt bookmarks persist in the session

Merged
TE
23d ago
+1082-83

Bookmarks on prompts in the thread minimap used to live in one pane's localStorage, so they were lost on another pane, another Mac, or a moved chat. acpmux now keeps them with the session. They survive a restart, a fork, a move, and a remote (peer) session, and every attached pane sees each change.

What changed

  • acpmux (cmux-tui/crates/acpmux): SessionMeta.bookmarks is a set of the chat's user_message seqs. It defaults to empty, is skipped when empty, and old session.json files still load.
    • _acpmux/bookmark_get {sessionId} returns {bookmarks: [seq...]}.
    • _acpmux/bookmark_set {sessionId, seq, bookmarked} returns the new list. It rejects a seq that is not a prompt in that chat ("seq N is not a prompt in session …"), refuses during a move, and caps a chat at 1000 marks. The code is in hub/bookmarks.rs and server/requests/composer.rs, and both methods are advertised in initialize.
    • Broadcast: a change logs a mux bookmarks record, as set_tags does, so attached panes get it as _acpmux/event. The chat keeps its place in the session list because updatedAt is not bumped.
    • Fork: marks on the prompts the fork copies carry over under the fork's own seqs, because a fork renumbers its log. After that the two chats mark independently.
    • Move (transfer_*): marks travel in session.json and the log keeps its seqs, so they still point at the same prompts on the target. On commit the source clears them, like the draft, because the chat lives on the target now. An imported bundle is capped at 1000.
    • Peers: both methods are session-scoped and not in SESSION_SCOPED_EXCLUDED, so peer_forward.rs sends a peer's chat to its owner. Nothing is answered locally.
  • Pane relay (AcpmuxPaneMethods.swift, +Params.swift, and the Rust mirror cmux-agent-pane-policy/policy.json): both methods are allowlisted and sessionScoped. The only allowed params are sessionId (get) and sessionId, seq, bookmarked (set). No gesture is needed.
  • Web (threadMinimap/bookmarks.ts): the localStorage store is replaced by a per-session store backed by acpmux.
    • It loads marks with bookmark_get and toggles with bookmark_set. A legacy localStorage mark is removed only after its bookmark_set succeeds; a refused one is retried on the next attach. A toggle shows immediately and rolls back if acpmux refuses it.
    • It applies bookmarks records from other panes. The attach kinds now include bookmarks.
    • Marks are keyed by the prompt row's seq (user-<seq>). The fnv1a key scheme is dropped. Old localStorage marks for the chat are imported through bookmark_set.
    • The popover button shows only for prompts that acpmux has logged.
    • The mock daemon serves both methods. No tick marker was added, and the minimap UI is unchanged.

Relay authorization analysis

Per skills/cmux-socket-policy/references/remote-relay-authorization.md:

  • Local command or content execution: none. The params are a session id, an integer seq and a boolean. Nothing is spawned, opened, evaluated or sent to an agent, and no command-bearing param is accepted. The P1 params rule refuses command or any other extra key.

  • Access to unowned objects: both methods are in sessionScoped, so the relay refuses any sessionId outside the pane's scope, and also a frame with no sessionId. acpmux further accepts only seqs of user_message records in that same chat. On a peer, the owning daemon checks the seq against its own log.

  • Local-state exposure: the reply is the chat's own list of integers, which the pane already sees as row ids. It contains no paths, environment, tokens or other sessions.

  • Policy tests: AgentPaneBookmarksRelayTests (Swift, at the transport) covers:

    • allowed for an owned chat with no gesture;
    • refused for an unowned chat, a missing sessionId, a command param and an unknown param.

    The Rust mirror's cases cover the same rules: sessions.json for own versus other chat and a missing id, and params.json for known params versus command and cwd.

Red, then green

Red commit 26e37116af6f, fix b5e2f061cd92.

CheckRed (26e37116af6f)Green (b5e2f061cd92)
cmux-ci run --class isolated --script scripts/ci/cmux-tui-rust-check.sh --arg test --arg bookmarkhub_integration: 0 passed, 3 failed ("no such method"); cargo stopped before the peer test4 new tests pass: restart and prompt-only (drafts::bookmarks::a_bookmark_marks_only_a_prompt_and_survives_a_daemon_restart), fork, move, peer forward; no failures
run-swift-testing-suites.sh Packages/macOS/CmuxNextAgentPaneBookmarksRelayTests fails (frames 6/7 refused, 0 reached acpmux)passes; 290 passed, 1 failed
bun test test/thread-bookmarks.test.tsx (webviews)0 pass, 4 fail (setBookmarkTransport is not a function)3 pass, 0 fail

The one Swift failure on both heads is CmuxNextAppTests.NowPlayingCardTests, which this PR doesn't touch. Both suite steps also exit 1 from the same-tree skip guard (AcpmuxBundleReplacedTests skipped: the head's cmux-tui tree is not published).

Review fixes (red 898f172f163b, fix c13d6b810d60)

  • Concurrent sets: set_bookmark now checks for a move, changes the marks, logs the bookmarks record and saves meta in one section under the chat's move lock (session.moving). Two sets therefore log in the order they change the marks. The same lock closes the race with transfer_begin: a move begins either before a change or after it.
  • Log scan: the prompt check scans the log on spawn_blocking, not on an async worker.
  • Legacy marks: a localStorage mark is removed only once acpmux holds it.
CheckRed (898f172f163b)Green (c13d6b810d60)
cmux-tui-rust-check.sh test bookmarkconcurrent_sets_log_the_marks_in_the_order_they_change_them passed: 8 writers x 20 sets x 10 rounds did not hit the race on the fleet host, so this test guards against regressions but is not a deterministic repro5 bookmark tests pass
bun test test/thread-bookmarks.test.tsx3 pass, 1 fail (an old local mark leaves localStorage only once acpmux keeps it)4 pass

Swift is unchanged since b5e2f061cd92, so the Swift suites and release compile were not re-run. Crash ratchet: ok. Rust godfiles: ok. The webviews checks (ui-rules, the minimap tests, type-scale, system-blue, text-focus, vp fmt) give the same results as before.

Other checks on b5e2f061cd92

  • cmux-agent-pane-policy (cmux-tui-rust-check.sh test <name>): session_scope, params_rule and the_allowlist_is_what_the_page_sends pass, so policy.json matches what direct.ts sends
  • Release compile (scripts/cmux-next/check-release-compile.sh): xcodebuild exit 0
  • crash_ratchet.py: ok. check-no-godfiles.sh --only rust and --only swift on the touched files: ok (server/requests.rs stays line-neutral). check-app-size.sh: ok. check-test-notification-centers.sh: ok. check-system-blue.py: ok.
  • webviews: bun test test/ui-rules.test.ts test/thread-turn-keys.test.tsx test/thread-minimap-current.test.tsx passes. In bun run check, type-scale, system-blue, text-focus and vp fmt pass. Two failures are already on base, in files this PR doesn't touch: check:layers (summary/summary.css z-index) and one vp lint warning (SubagentStatusList.tsx).
  • Base reds in files this PR doesn't touch: check-concurrency.sh (ControlWorkspaceInfoCache.swift:59) and lint-ios-package-conventions.sh (PaneAnimationsSetting). This PR adds no public types.
  • Not run: the Windows cargo check. This PR touches only acpmux, with no platform code, and neither cmux-tui-core nor cmux-tui. No Testbox CLI was available on this host, so the PR's CI is the only coverage.
  • No new user-facing strings; the existing minimap.bookmark* strings are reused.

Changelog

  • Added: prompt bookmarks are saved with the chat and survive restart and remote sessions

🤖 Generated with Claude Code