Skip to content

fix(sidebar): make slot reconcile additive, rebalance on split#52

Merged
rtalexk merged 13 commits into
mainfrom
fix/sidebar-slot-cascade-and-rebalance
May 25, 2026
Merged

fix(sidebar): make slot reconcile additive, rebalance on split#52
rtalexk merged 13 commits into
mainfrom
fix/sidebar-slot-cascade-and-rebalance

Conversation

@rtalexk

@rtalexk rtalexk commented May 25, 2026

Copy link
Copy Markdown
Owner

Context

Sticky sidebar slots mode shipped with three latent hazards that surfaced under heavy multi-window use:

  1. Kill-pane cascades during hot paths. Show, Follow, PrepareWindow, and the after-new-window hook all reconciled the slot set on every fire, which meant a single kill-pane could fan out through backgrounded pane_closed / pane_exiting handlers, 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.
  2. Rightmost pane ratcheted smaller on every sidebar open/close cycle. Tmux's default victim for a left -b split 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.
  3. demux session connect against a multi-window config stranded the sidebar. The new-window handler's Follow raced the legitimate client-session-changed handler's Follow during the launch burst, leaving the sidebar in the wrong window.

What does this PR do

  • Additive-only slot reconciliation. Adds AddMissingSlots and routes Show, Follow, PrepareWindow, MaybePromoteSlot, and the after-new-window event through it. Slot eviction and dedupe now live exclusively in demux sidebar slots prune.
  • demux sidebar slots prune subcommand. Reconciles the slot set on demand. Wraps the work in withHooksSuppressed, which clears after-kill-pane / pane-exited for the duration of the reconcile so its kill-pane calls do not cascade into a sweep storm. Snapshot is taken only after set-hook -gu succeeds (so a failed unset cannot leak doubled hooks on restore).
  • Preserves env-tracked active sidebar across prune dedup. ReconcileSlots now takes a protectedPane parameter; 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 into UninstallSlots via the next Follow.
  • Auto-promote slot on session change. MaybePromoteSlot (called from session_changed / window_focus handlers) activates the current window's slot when the client has no live sidebar tracked, without going through Show (which would Reconcile and kick off a cascade).
  • Rebalance pane widths after sidebar split. splitSidebarPane snapshots pre-split widths, then issues resize-pane calls 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.
  • session-connect race fix. launchConfigSessionAndConnect wraps only window creation in after-new-window suppression, restores the hook before tmux.Connect, so the legitimate client-session-changed Follow still fires.
  • Drops dead LaunchAndConnectConfigSession and tightens stale comments around the rename.

Out of scope (known gaps, follow-ups)

  • internal/sticky/reconcile.go:56 swallows errors from EnsureSlotInWindow in the install loop. Self-heals on next nav via AddMissingSlots, so left as-is.
  • README troubleshooting does not link the slot placeholder SIGHUP / pty-leak rationale documented in cmd/sidebar.go:240-251. Doc polish only.

Manual QA

Sticky sidebar feature is per-client tmux state. Verify by opening tmux, attaching demux, and exercising each scenario against a live tmux server. Run all scenarios with [sidebar.sticky] slots = true set in ~/.config/demux/demux.toml.

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, rerun demux hooks install.

2. Slots mode enabled in config.

```bash
grep -A2 "sidebar.sticky" ~/.config/demux/demux.toml
```

Expect slots = true under [sidebar.sticky]. If not, edit the file or run demux config init and 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 no DEMUX_STICKY_MRU entries.

Scenario 1 — Sidebar open/close cycle preserves rightmost pane width

Reproduces the pre-fix ratchet on the rightmost pane.

  1. In a fresh tmux window with two horizontal panes (nvim left, claude right, both ~80 cols), record widths:
    ```bash
    tmux list-panes -F "#{pane_id} #{pane_left} #{pane_width}"
    ```
  2. demux sidebar show — sidebar appears as leftmost column. Re-list panes; expected: claude (rightmost) still has its original width, nvim (leftmost-adjacent) is original - sidebar_width.
  3. demux sidebar hide. Re-list panes; expected: claude unchanged, nvim restored to its original width.
  4. Repeat steps 2-3 four more times. Expected: claude width stays constant across every cycle. Pre-fix: claude shrinks by a few columns each cycle.
Scenario 2 — Multi-window session connect lands sidebar in the right window

Reproduces the session-connect race.

  1. Add a multi-window session config:
    ```toml
    [[session]]
    name = "qa-multi"
    path = "~"
    [[session.window]]
    name = "a"
    [[session.window]]
    name = "b"
    [[session.window]]
    name = "c"
    ```
  2. From a tmux client with the sidebar already shown (demux sidebar show), run:
    ```bash
    demux session connect qa-multi
    ```
  3. After connect settles, the active window should be c (last defined). Confirm sidebar is in window c:
    ```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=1 row, the active pane is the user pane (not the sidebar).
  4. Confirm no stranded duplicate slots elsewhere:
    ```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
  1. With two demux-configured sessions A and B both having demux sidebar slots install run at least once, attach to A, run demux sidebar show, then close the sidebar pane manually via prefix x.
  2. Switch to session B (prefix s, select B). Expected: the slot in B's current window automatically respawns as the active sidebar (you see demux --sticky running in the slot pane).
  3. Verify env tracking:
    ```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.

  1. With sidebar shown, capture the active sidebar pane id:
    ```bash
    tmux show-environment -g | grep DEMUX_STICKY_PANE
    ```
  2. Manually tag a second pane in the same window as a slot (simulating corruption):
    ```bash
    tmux split-window -h -d
    tmux set-option -p -t :.+ @demux_slot 1
    ```
  3. Confirm two slot panes in current window, env-tracked one second:
    ```bash
    tmux list-panes -F "#{pane_id} #{@demux_slot}" | awk '$2==1'
    ```
  4. Run demux sidebar slots prune.
  5. Re-list slot panes; expected: exactly one slot pane in the current window, and it is the same pane id captured in step 1 (the env-tracked one).
  6. Navigate to another window and back. Expected: sidebar follows correctly, no cascade into UninstallSlots (other windows still have their slots — check tmux 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.

  1. With slots mode active and an existing sidebar, rapidly create windows:
    ```bash
    for i in $(seq 1 10); do tmux new-window; done
    ```
  2. Immediately try to type / send keys. Expected: tmux remains responsive throughout. Pre-fix: input could freeze for several seconds while the cascade drained.
  3. After the burst settles, confirm slots populated additively:
    ```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).

rtalexk added 13 commits May 24, 2026 16:45
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.
@rtalexk rtalexk merged commit e6534aa into main May 25, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant