fix(sidebar): make slot reconcile additive, rebalance on split#52
Merged
Conversation
ReconcileSlots used to silently ignore the second slot it found per window
(`if _, has := existing[wid]; !has { existing[wid] = pid }`), so any race that
created two slot-tagged panes in the same window left the duplicate haunting
the user forever. Now the extras are killed regardless of reserved/current
status, restoring the at-most-one-slot-per-window invariant on every
reconcile pass.
AddMissingSlots creates placeholder slots only where missing - never kills. Show.showSlotsMode and Follow.maintainSlotsBudget now use it instead of ReconcileSlots. The kill-pane cascade (after-kill-pane -> backgrounded pane_closed -> SweepOrphanWindows -> potentially more kill-pane) was saturating tmux's command queue and freezing input on every show / nav. Eviction and dedupe move out of these hot paths; they belong to the upcoming explicit `slots prune` subcommand. Regression test guards Show against any kill-pane call.
Adds MaybePromoteSlot: when the client switches session/window and the env var tracking the active sidebar is unset (or its pane is dead) and the destination window already has a slot, promote that slot to active sidebar via respawn-pane + WriteEnv + optional focus. Wired into the session_changed / window_focus handler so users who ran `demux sidebar hide` and later jumped sessions stop seeing a bare slot placeholder in the destination. Five gating checks (slots mode on, env unset/dead, any slot exists, current target resolvable, slot in current window) make this safe to call on every nav. Promote deliberately does no ReconcileSlots / EnsureSlotInWindow elsewhere to keep the nav hook off the kill-pane cascade path.
New explicit one-shot reconcile (PruneMRU + ComputeReserved + ReconcileSlots) for cleaning up corrupt slot state - duplicates from past races, stale slots in non-reserved windows. The only path that intentionally kills slots. Prune wraps the reconcile in withHooksSuppressed(after-kill-pane, pane-exited): captures current hook bodies via show-hooks -g, clears them with set-hook -gu, restores via defer + set-hook -ga. Each kill-pane during prune fires zero demux hooks, so the after-kill-pane -> pane_closed -> SweepOrphanWindows -> more kill-pane cascade that used to saturate tmux's command queue cannot fire. Also wraps every interactive sidebar command (show/hide/toggle/slots install/uninstall) and the client_attached auto-show in withEventLock for symmetry with the existing pane_closed/pane_exiting lock holders.
Two changes that close gaps in the slot-lifecycle invariants: 1. ensureSlots now uses AddMissingSlots instead of ReconcileSlots. The after-new-window event runs on every new window, so reconcile here was the last remaining kill-pane source in the hot path - directly contradicting the stated "only `slots prune` kills" invariant. 2. eventNewWindowCmd no longer calls Follow. A new window does not imply the client switched - tmux fires after-select-window separately, and that handler (runWindowFocus) is the right place. The extra Follow on new_window races against the after-select-window Follow and, during a multi-window session launch, against itself across the burst of after-new-window hooks - stranding the sidebar in the wrong window.
… launch
`demux session connect <config-session>` runs new-session + N x new-window +
switch-client in sequence. Each new-window fires `run-shell -b "demux event
new_window"` - backgrounded handlers that race with each other and with the
client-session-changed handler that follows switch-client. With three+
concurrent Follow / ensureSlots invocations thrashing swap-pane state, the
final sidebar location is non-deterministic - users observed the TUI stranded
in the source session.
Fix: wrap the launch sequence (NewSessionDetached + CreateSessionWindows) in
withHooksSuppressed("after-new-window"). Hooks are restored before
tmux.Connect so the session-changed handler still fires - one handler, no
race. Split LaunchAndConnectConfigSession into LaunchConfigSession + the
existing composed wrapper to enable caller-level suppression.
cmd/root.go: loadConfig becomes a var so the new ensureSlots regression
test can inject a slots-enabled config without touching disk.
Without rebalancing, tmux's default `split-window -f -h -b -l W` takes width from the rightmost pane, but on close (kill-pane) tmux gives it back to the adjacent pane. The asymmetry ratchets: every open/close cycle moves W columns from the rightmost pane to the adjacent one, shrinking the rightmost pane to nothing over time. Users with nvim + claude side-by-side hit it fast. splitSidebarPane snapshots pane_id / pane_left / pane_width per pane, runs the split, then resizes every original pane (rightmost-first by pane_left) back to its recorded width - skipping the panes in the originally-leftmost column, which absorb the sidebar's footprint. With open and close both taking from / giving to the same adjacent column, cycles are net-zero. Geometry-read failures are swallowed so the rebalance is best-effort. Show and EnsureSlotInWindow now go through this helper instead of issuing split-window directly.
PrepareWindow runs on every TUI session switch (model_keys.go:34) and was still calling ReconcileSlots, which fires kill-pane for stale or duplicate slots - the exact backgrounded after-kill-pane -> pane_closed -> SweepOrphanWindows cascade the prior fixes went to lengths to extinguish. Switch to AddMissingSlots so the TUI nav path matches Show / Follow / the after-new-window event. Cost: stale slots in non-reserved windows now linger until the user runs `demux sidebar slots prune`. Acceptable; the invariant is "only prune evicts," not "every nav reconciles." Regression test seeds duplicates + a stale slot and asserts zero kill-pane calls.
If `set-hook -gu <event>` failed, the snapshot for that event was still pushed onto the restore list. The deferred restore then re-appended every original hook on top of the still-installed originals, doubling every subsequent hook firing for the rest of the tmux server's lifetime. Push the snapshot only after the unset succeeds. Regression test extends stubTmux with a Run-override and asserts no `set-hook -ga` runs when the unset is scripted to fail. Also adds an inline comment to the existing ordering test explaining why preserving slice order matters (tmux's [N] bracket index is the registration order and the restore loop replays it).
LaunchAndConnectConfigSession had only one caller (dispatchConnect) and that caller now uses the launch/connect split + hook suppression from launchConfigSessionAndConnect. Keeping a thin wrapper around something the codebase no longer touches invites the next refactor to "fix" it back the wrong way. Also fixes a comment in maintainSlotsBudget that still claimed dedupe and eviction "live in `demux sidebar slots prune` and the after-new-window event" - after-new-window is now additive-only, so the only eviction path is prune.
Code reviewer flagged the absence of integration coverage for the actual race fix. Expose launchConfigSession / tmuxConnect as package vars (same pattern as stickyClient / openDB) so the test can inject fakes that record call order without spawning real tmux processes. Asserts: launch runs before connect; `set-hook -gu after-new-window` fires before launch; `set-hook -ga after-new-window` (the deferred restore) fires between launch and connect.
…aveat splitSidebarPane silently skipped the rebalance when listPaneGeometry errored. Log at debug level so future investigators can tell whether the rebalance actually ran instead of guessing. MaybePromoteSlot's respawn-pane will kill any other client's TUI process that happens to track the same slot. Acceptable for single-user multi-client setups (the demux target audience); document the caveat inline so it isn't a surprise.
ReconcileSlots dedup logic in the current window was keeping the first-listed duplicate and killing the rest. When the env-tracked active sidebar was not first, prune would yank the user's sidebar and cascade into UninstallSlots, orphaning slots in other windows. Fix: ReconcileSlots now takes a protectedPane parameter (the env-tracked sidebar for the invoking client). In the current-window dedup branch, if the incoming duplicate is the protected pane and the kept one is not, swap the survivor so the protected pane survives unconditionally. Caller (sidebar.go prune cmd): look up the env var for the invoking client's tty and pass it as protected. Best-effort fallback: missing tty / unset env / read failure all degrade to no protection, same behavior as before. Adds regression tests for both orderings: protected pane listed first vs second.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Sticky sidebar slots mode shipped with three latent hazards that surfaced under heavy multi-window use:
Show,Follow,PrepareWindow, and theafter-new-windowhook all reconciled the slot set on every fire, which meant a single kill-pane could fan out through backgroundedpane_closed/pane_exitinghandlers, each running its own server-wide sweep. Under bursts (e.g.demux session connect <multi-window-config>) the cascade saturated tmux's command queue and froze input.-bsplit is the rightmost pane, but its default victim on pane removal is the leftmost-adjacent. The asymmetry compounded — open shrinks rightmost, close grows leftmost — so repeatedly toggling the sidebar slowly ate the user's chat / claude column.demux session connectagainst a multi-window config stranded the sidebar. The new-window handler'sFollowraced the legitimateclient-session-changedhandler'sFollowduring the launch burst, leaving the sidebar in the wrong window.What does this PR do
AddMissingSlotsand routesShow,Follow,PrepareWindow,MaybePromoteSlot, and theafter-new-windowevent through it. Slot eviction and dedupe now live exclusively indemux sidebar slots prune.demux sidebar slots prunesubcommand. Reconciles the slot set on demand. Wraps the work inwithHooksSuppressed, which clearsafter-kill-pane/pane-exitedfor the duration of the reconcile so its kill-pane calls do not cascade into a sweep storm. Snapshot is taken only afterset-hook -gusucceeds (so a failed unset cannot leak doubled hooks on restore).ReconcileSlotsnow takes aprotectedPaneparameter; when duplicates exist in the current window and one of them is the env-tracked sidebar, the protected pane wins. Without this, prune could yank the user's sidebar and cascade intoUninstallSlotsvia the nextFollow.MaybePromoteSlot(called fromsession_changed/window_focushandlers) activates the current window's slot when the client has no live sidebar tracked, without going throughShow(which would Reconcile and kick off a cascade).splitSidebarPanesnapshots pre-split widths, then issuesresize-panecalls rightmost-first to restore them, so the leftmost original column absorbs the sidebar's footprint instead of the rightmost. Open/close cycles are now stable.launchConfigSessionAndConnectwraps only window creation inafter-new-windowsuppression, restores the hook beforetmux.Connect, so the legitimateclient-session-changedFollow still fires.LaunchAndConnectConfigSessionand tightens stale comments around the rename.Out of scope (known gaps, follow-ups)
internal/sticky/reconcile.go:56swallows errors fromEnsureSlotInWindowin the install loop. Self-heals on next nav viaAddMissingSlots, so left as-is.cmd/sidebar.go:240-251. Doc polish only.Manual QA
Prerequisites
1. tmux >= 2.6 attached with demux hooks installed.
```bash
tmux -V
demux hooks install
tmux show-hooks -g | grep demux
```
Expect demux entries on
after-new-window,after-kill-pane,pane-exited,client-session-changed,after-select-window,client-attached. If missing, rerundemux hooks install.2. Slots mode enabled in config.
```bash
grep -A2 "sidebar.sticky" ~/.config/demux/demux.toml
```
Expect
slots = trueunder[sidebar.sticky]. If not, edit the file or rundemux config initand toggle it.3. Built demux on this branch.
```bash
go build -o demux . && ./demux --version
```
Confirm the binary is from the current checkout.
Setup
Start with a clean slate so changes are observable:
```bash
demux sidebar hide # tear down any prior slots
tmux show-environment -g | grep DEMUX_STICKY
```
Expect no
DEMUX_STICKY_PANE_*and noDEMUX_STICKY_MRUentries.Scenario 1 — Sidebar open/close cycle preserves rightmost pane width
Reproduces the pre-fix ratchet on the rightmost pane.
```bash
tmux list-panes -F "#{pane_id} #{pane_left} #{pane_width}"
```
demux sidebar show— sidebar appears as leftmost column. Re-list panes; expected: claude (rightmost) still has its original width, nvim (leftmost-adjacent) isoriginal - sidebar_width.demux sidebar hide. Re-list panes; expected: claude unchanged, nvim restored to its original width.Scenario 2 — Multi-window session connect lands sidebar in the right window
Reproduces the session-connect race.
```toml
[[session]]
name = "qa-multi"
path = "~"
[[session.window]]
name = "a"
[[session.window]]
name = "b"
[[session.window]]
name = "c"
```
demux sidebar show), run:```bash
demux session connect qa-multi
```
c(last defined). Confirm sidebar is in windowc:```bash
tmux display-message -p "#{window_id} #{window_name}"
tmux list-panes -t :. -F "#{pane_id} #{@demux_slot} #{pane_title}"
```
Expected: at most one
@demux_slot=1row, the active pane is the user pane (not the sidebar).```bash
tmux list-panes -aF "#{window_id} #{pane_id} #{@demux_slot}" | awk '$3==1'
```
Expected: at most one slot per window.
Scenario 3 — Auto-promote slot on session change
AandBboth havingdemux sidebar slots installrun at least once, attach toA, rundemux sidebar show, then close the sidebar pane manually viaprefix x.B(prefix s, select B). Expected: the slot in B's current window automatically respawns as the active sidebar (you seedemux --stickyrunning in the slot pane).```bash
tmux show-environment -g | grep DEMUX_STICKY_PANE
```
Expected: one entry, pointing at the now-active slot pane.
Scenario 4 — slots prune preserves the active sidebar across dedup (F1 regression)
Forces the corrupt-state case where the current window has two slot-tagged panes and the env-tracked one is not first-listed.
```bash
tmux show-environment -g | grep DEMUX_STICKY_PANE
```
```bash
tmux split-window -h -d
tmux set-option -p -t :.+ @demux_slot 1
```
```bash
tmux list-panes -F "#{pane_id} #{@demux_slot}" | awk '$2==1'
```
demux sidebar slots prune.UninstallSlots(other windows still have their slots — checktmux list-panes -aF "#{window_id} #{@demux_slot}" | awk '$2==1').Scenario 5 — new-window burst does not freeze tmux input
Stress-tests the additive invariant.
```bash
for i in $(seq 1 10); do tmux new-window; done
```
```bash
tmux list-panes -aF "#{window_id} #{@demux_slot}" | awk '$2==1' | sort -u
```
Expected: at most one slot per MRU-reserved window, capped at
MRUMaxLen(8 placeholders + 1 active sidebar).Automated checks
```bash
go build ./...
go test ./...
gofmt -l .
```
All pass on this branch (204 tests in
internal/sticky+cmd).