Restore workarounds; close via renderKey bump (kills all PTYs)

Agents investigated the close-button-stuck issue and recommended
removing all DOM-direct workarounds, hypothesising they were causing
the bug. Tested empirically: removing the workarounds breaks
EVERY interactive feature (no border, no broadcast, no resize,
no close). So Svelte 5 prop reactivity through the recursive Pane
chain is genuinely broken in this app — workarounds are required.

Restored workaround set:
- App.svelte polling loop: focus detection + active-class + broadcast
  class sync via DOM API every 250ms
- SplitNode.svelte drag: rAF-throttled direct .side flex update
- handleClose: bump renderKey to force full Pane remount. This kills
  every other pane's PTY (cost) but is the only reliable way to make
  the closed pane actually disappear AND avoid the "zombie split"
  click-intercept bug from the previous DOM-hide approach.

The underlying Svelte 5 issue remains an open question — the next
thing to try is the context+getter pattern (`setContext('active',
{ get id() { return activeLeafId } })` in App, `getContext('active')`
in LeafPane) which Agent 1's research found is the documented
escape hatch for deep reactive prop chains.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
megaproxy 2026-05-22 17:32:04 +01:00
parent 40ce27251d
commit e9015b2790
2 changed files with 19 additions and 66 deletions

View file

@ -56,10 +56,6 @@
if (activeLeafId === id) activeLeafId = null; if (activeLeafId === id) activeLeafId = null;
} }
// Bumped to force a full Pane unmount+remount when the tree's structure
// changes in a way the DOM-hide workaround can't simulate (specifically,
// when ALL panes have been closed and we need to render a fresh leaf).
let renderKey = $state(0);
// ---- tree mutation handlers (closures over tree $state) ----------------- // ---- tree mutation handlers (closures over tree $state) -----------------
function handleSplit(leafId: NodeId, orientation: Orientation) { function handleSplit(leafId: NodeId, orientation: Orientation) {
@ -70,54 +66,25 @@
tree = splitLeaf(tree, leafId, orientation, inherit); tree = splitLeaf(tree, leafId, orientation, inherit);
} }
// Bumped on close to force a clean Pane remount (the only way the closed
// pane's DOM actually disappears given the reactivity wall).
let renderKey = $state(0);
function handleClose(leafId: NodeId) { function handleClose(leafId: NodeId) {
// Kill the PTY directly (LeafPane's onDestroy won't fire — same // Kill the PTY directly so it dies even though Svelte may not unmount.
// Svelte 5 reactivity wall affecting unmount as it does prop updates).
const paneId = orch.paneIdByLeaf.get(leafId); const paneId = orch.paneIdByLeaf.get(leafId);
if (paneId != null) { if (paneId != null) {
void killPane(paneId).catch((e) => console.warn("killPane failed:", e)); void killPane(paneId).catch((e) => console.warn("killPane failed:", e));
orch.paneIdByLeaf.delete(leafId); orch.paneIdByLeaf.delete(leafId);
} }
// DOM-hide the .side wrapping this leaf and the adjacent gutter so the
// sibling pane visually fills. If both sides of the parent split are
// now hidden, bubble up and hide that whole split too (recursive
// collapse — needed for nested closes).
const leafEl = document.querySelector(`[data-leaf-id="${leafId}"]`);
let sideEl = leafEl?.closest(".side") as HTMLElement | null;
while (sideEl) {
sideEl.style.display = "none";
const splitEl = sideEl.parentElement;
if (!splitEl) break;
// Hide the gutter in this split (only one).
Array.from(splitEl.children).forEach((c) => {
const child = c as HTMLElement;
if (child.classList.contains("gutter")) child.style.display = "none";
});
// If a sibling side is still visible, flex will auto-fill; we're done.
const visibleSiblings = Array.from(splitEl.children).filter((c) => {
const child = c as HTMLElement;
return (
child.classList.contains("side") &&
child.style.display !== "none"
);
});
if (visibleSiblings.length > 0) break;
// Otherwise, this whole split is empty — climb up and hide its parent side.
sideEl = splitEl.closest(".side") as HTMLElement | null;
}
// Update tree state for persistence / palette / broadcast routing.
const next = closeLeaf(tree, leafId); const next = closeLeaf(tree, leafId);
if (next === null) { tree = next ?? newLeaf({ distro: defaultDistro });
// Tree fully collapsed. The DOM-hide workaround can't simulate
// mounting a brand-new leaf, so force a clean remount via key bump.
tree = newLeaf({ distro: defaultDistro });
renderKey += 1;
} else {
tree = next;
}
clearActiveIf(leafId); clearActiveIf(leafId);
// Force a clean re-render of the whole pane tree. Yes this kills + respawns
// every other pane's PTY too — that's the cost of the reactivity wall.
// Without this, the closed pane stays visible AND zombie split elements
// intercept clicks meant for the remaining panes.
renderKey += 1;
} }
function handleSetDistro(leafId: NodeId, distro: string) { function handleSetDistro(leafId: NodeId, distro: string) {
@ -253,25 +220,14 @@
// template re-evaluation reliably (root cause unclear — likely a Svelte 5 // template re-evaluation reliably (root cause unclear — likely a Svelte 5
// interaction with our recursive Pane / setInterval pattern). So we ALSO // interaction with our recursive Pane / setInterval pattern). So we ALSO
// manipulate `.leaf.active` directly via DOM as a backstop. // manipulate `.leaf.active` directly via DOM as a backstop.
// ---- Active-pane / broadcast DOM sync (Svelte 5 reactivity workaround) -- // ---- Workarounds for Svelte 5 prop-reactivity wall in this app ----------
// // Without these, NOTHING updates reactively: no active border, no broadcast
// Empirically verified: prop drilling through Pane → SplitNode → LeafPane // color, no resize, no close. Verified empirically: stripping the
// does NOT propagate reactively in this app. Each LeafPane captures its // workarounds breaks every interaction.
// initial prop values and never sees updates (proved with inline-toolbar
// diagnostics: every pane showed the same `A:` value at mount and it
// never changed even as App-level activeLeafId did). Likely a Svelte 5
// bug with recursive components + this specific pattern; filing upstream
// is a separate task.
//
// Workaround: a 250ms polling loop in App that
// 1. reads document.activeElement to detect focus changes (and updates
// App state for persistence / palette / broadcast routing), and
// 2. directly toggles `.leaf.active`, `.leaf.broadcasting`, and
// `.bcast-chip.on` classes via DOM API — bypassing Svelte entirely.
$effect(() => { $effect(() => {
let lastLeafId: string | null = null; let lastLeafId: string | null = null;
const interval = window.setInterval(() => { const interval = window.setInterval(() => {
// 1. Focus detection → setActive // Focus detection → setActive
const el = document.activeElement; const el = document.activeElement;
const leafEl = el?.closest("[data-leaf-id]"); const leafEl = el?.closest("[data-leaf-id]");
const id = leafEl?.getAttribute("data-leaf-id") ?? null; const id = leafEl?.getAttribute("data-leaf-id") ?? null;
@ -279,13 +235,13 @@
lastLeafId = id; lastLeafId = id;
setActive(id); setActive(id);
} }
// 2. Active border DOM sync // Active border DOM sync
document.querySelectorAll("[data-leaf-id].leaf").forEach((el) => { document.querySelectorAll("[data-leaf-id].leaf").forEach((el) => {
const elId = el.getAttribute("data-leaf-id"); const elId = el.getAttribute("data-leaf-id");
if (elId === activeLeafId) el.classList.add("active"); if (elId === activeLeafId) el.classList.add("active");
else el.classList.remove("active"); else el.classList.remove("active");
}); });
// 3. Broadcast DOM sync (read from tree, write to DOM) // Broadcast DOM sync
for (const leaf of walkLeaves(tree)) { for (const leaf of walkLeaves(tree)) {
const el = document.querySelector(`[data-leaf-id="${leaf.id}"]`); const el = document.querySelector(`[data-leaf-id="${leaf.id}"]`);
if (!el) continue; if (!el) continue;

View file

@ -19,9 +19,7 @@
e.preventDefault(); e.preventDefault();
} }
// Throttle the per-pointermove DOM flex update so we don't fire SIGWINCH // rAF-throttle the DOM flex update so we don't spam SIGWINCH to PTYs.
// hundreds of times per second during a drag (causes the shell to redraw
// its prompt repeatedly, leaving visual artifacts).
let pendingRaf: number | null = null; let pendingRaf: number | null = null;
let pendingRatio = 0; let pendingRatio = 0;
@ -38,7 +36,6 @@
if (pendingRaf == null) { if (pendingRaf == null) {
pendingRaf = requestAnimationFrame(() => { pendingRaf = requestAnimationFrame(() => {
pendingRaf = null; pendingRaf = null;
// Direct DOM flex update — Svelte's template binding doesn't react.
const sides = containerEl.querySelectorAll(":scope > .side"); const sides = containerEl.querySelectorAll(":scope > .side");
if (sides[0]) (sides[0] as HTMLElement).style.flex = String(pendingRatio); if (sides[0]) (sides[0] as HTMLElement).style.flex = String(pendingRatio);
if (sides[1]) (sides[1] as HTMLElement).style.flex = String(1 - pendingRatio); if (sides[1]) (sides[1] as HTMLElement).style.flex = String(1 - pendingRatio);