composer: a window you cannot reach, and an empty pipeline that filled itself

A floating window remembers its position per id, and only the DRAG handler
clamped. So a window moved to the right of a wide screen and reopened on a
narrow one opened entirely off the viewport: no titlebar on screen to grab,
nothing to drag back, and the position remembered -- unrecoverable without
clearing localStorage by hand. Reproduced at x=1400 in an 800px viewport.

The clamp lives in apply() now, so every path goes through it: opening,
restoring a remembered box, coming back from maximised, and the viewport
shrinking under an open window. Also guarded for a viewport narrower than
the grabbable strip, where the old bound went negative and pushed the window
further off rather than back on.

jsdom cannot see any of this -- getBoundingClientRect is zeros there, so a
window off the right edge measures identically to one in the middle. The
three tests are Playwright, and removing the clamp fails two of them.

And POST /api/pipeline could not accept an empty pipeline: `slots or
default()` resurrects the built-ins, because an empty list is falsy. A client
that removed every slot got them handed back. Absent means "use the default";
empty means "there is nothing", and only one of those is a fallback.

One of the tests I first wrote for this asserted len(x) < len(y) + 1, which
is true of almost anything. It counts the pipeline at every step now and
fails if it ever grows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

app/composer/elements/floating.js

@@ -36,7 +36,27 @@ define("floating", "window", ({
   // Bound, because another window coming to the front shifts this one's z.
   const layer = claimFor("floatingWindow", node);
 
+  // Enough of the window to grab: a strip of titlebar wide enough to hit and
+  // tall enough to see. Everything that positions the window goes through here,
+  // because the one path that did not was the one that lost it.
+  const GRABBABLE = 80;
+  const TITLEBAR = 32;
+
+  const clampIntoView = () => {
+    // Math.max(0, ...) matters on a viewport narrower than the reachable strip:
+    // without it the upper bound goes negative and the clamp pushes the window
+    // further off rather than back on.
+    box.x = Math.min(Math.max(box.x, -box.width + GRABBABLE),
+                     Math.max(0, window.innerWidth - GRABBABLE));
+    box.y = Math.min(Math.max(box.y, 0), Math.max(0, window.innerHeight - TITLEBAR));
+  };
+
   const apply = () => {
+    // Clamped on EVERY apply, not only while dragging. A position is remembered
+    // per window id, so a window moved to the right of a wide screen and reopened
+    // on a narrow one opened entirely off it -- no titlebar to grab, nothing to
+    // drag back, and the only way out was clearing localStorage by hand.
+    clampIntoView();
     node.style.left = `${box.x}px`;
     node.style.top = `${box.y}px`;
     node.style.width = `${box.width}px`;
@@ -89,14 +109,19 @@ define("floating", "window", ({
   const toFront = () => { layer.raise(); };
   node.addEventListener("pointerdown", toFront);
 
+  // A window open while the viewport shrinks has the same problem as one
+  // reopened on a smaller screen.
+  const onViewportResize = () => apply();
+  window.addEventListener("resize", onViewportResize);
+
   let start = null;
   const stopDrag = drag(titlebar, {
     onStart: () => { start = { ...box }; toFront(); },
     onMove: ({ dx, dy }) => {
-      // Clamped so a window can never be dragged somewhere it cannot be
-      // dragged back from.
-      box.x = Math.min(Math.max(start.x + dx, -box.width + 80), window.innerWidth - 80);
-      box.y = Math.min(Math.max(start.y + dy, 0), window.innerHeight - 32);
+      // apply() does the clamping, so a window can never be dragged somewhere
+      // it cannot be dragged back from -- and the rule lives in one place.
+      box.x = start.x + dx;
+      box.y = start.y + dy;
       apply();
     },
     onEnd: () => remember(id, box),
@@ -127,6 +152,7 @@ define("floating", "window", ({
       remember(id, box);
       dismissal.release();
       stopDrag(); stopResize();
+      window.removeEventListener("resize", onViewportResize);
       layer.release();
       unmount(node);
       if (onClose) onClose(reason);

app/tests-layout/window.spec.js

@@ -0,0 +1,87 @@
+// A floating window must always be reachable.
+//
+// Position is remembered per window id, so the dangerous case is not "where did
+// it open" but "where did it open GIVEN a screen it no longer fits". jsdom
+// cannot see this at all: getBoundingClientRect is zeros there, so a window
+// entirely off the right edge measures the same as one in the middle.
+
+import { test, expect } from "@playwright/test";
+
+test.beforeEach(async ({ page }) => {
+  await page.goto("/funpack/");
+  await page.waitForFunction(() => window.FunPack !== undefined);
+});
+
+async function openWindow(page, id) {
+  return page.evaluate(async (id) => {
+    const { composer } = await import("/funpack/app/composer/composer.js");
+    const handle = composer.floating.window({
+      id, title: "Attack", body: composer.text.md({ text: "content" }),
+    });
+    const box = handle.node.getBoundingClientRect();
+    return { x: box.x, y: box.y, w: box.width, h: box.height,
+             vw: innerWidth, vh: innerHeight };
+  }, id);
+}
+
+test("a window remembered off a wider screen still opens reachable", async ({ page }) => {
+  // Remember a position far to the right, as dragging on a wide screen would.
+  await page.evaluate(() => localStorage.setItem(
+    "cx.win.attack", JSON.stringify({ x: 1400, y: 40, width: 420, height: 320 })));
+
+  await page.setViewportSize({ width: 800, height: 600 });
+  await page.goto("/funpack/");
+  await page.waitForFunction(() => window.FunPack !== undefined);
+
+  const box = await openWindow(page, "attack");
+
+  // Something grabbable has to be on screen -- a titlebar with no pixels in the
+  // viewport cannot be dragged back, and the position is remembered, so the
+  // window would be lost until localStorage was cleared by hand.
+  expect(box.x, "the whole window is past the right edge").toBeLessThan(box.vw);
+  expect(box.x + box.w, "the whole window is past the left edge").toBeGreaterThan(0);
+  expect(box.y).toBeGreaterThanOrEqual(0);
+  expect(box.y, "the titlebar is below the bottom edge").toBeLessThan(box.vh);
+
+  // And enough of it to actually hit with a pointer.
+  const visible = Math.min(box.x + box.w, box.vw) - Math.max(box.x, 0);
+  expect(visible, "less than a grabbable strip is on screen").toBeGreaterThanOrEqual(60);
+});
+
+test("a window open while the viewport shrinks stays reachable", async ({ page }) => {
+  await page.setViewportSize({ width: 1400, height: 900 });
+  await page.goto("/funpack/");
+  await page.waitForFunction(() => window.FunPack !== undefined);
+  await openWindow(page, "shrink");
+
+  await page.setViewportSize({ width: 600, height: 500 });
+  const box = await page.evaluate(() => {
+    const node = document.querySelector(".cx-window");
+    const r = node.getBoundingClientRect();
+    return { x: r.x, y: r.y, w: r.width, vw: innerWidth };
+  });
+
+  expect(box.x).toBeLessThan(box.vw);
+  expect(box.x + box.w).toBeGreaterThan(0);
+});
+
+test("a window cannot be dragged somewhere it cannot be dragged back from", async ({ page }) => {
+  await page.setViewportSize({ width: 900, height: 700 });
+  await page.goto("/funpack/");
+  await page.waitForFunction(() => window.FunPack !== undefined);
+  await openWindow(page, "dragged");
+
+  const bar = page.locator(".cx-window-bar").first();
+  await bar.hover();
+  await page.mouse.down();
+  await page.mouse.move(5000, 5000, { steps: 5 });   // far past both edges
+  await page.mouse.up();
+
+  const box = await page.evaluate(() => {
+    const r = document.querySelector(".cx-window").getBoundingClientRect();
+    return { x: r.x, y: r.y, w: r.width, vw: innerWidth, vh: innerHeight };
+  });
+  expect(box.x).toBeLessThan(box.vw);
+  expect(box.y).toBeLessThan(box.vh);
+  expect(box.x + box.w).toBeGreaterThan(0);
+});

core/routes.py

@@ -123,7 +123,12 @@ def register(routes, prefix=None):
         except Exception:  # noqa: BLE001
             return web.json_response({"problems": ["that is not JSON"]}, status=400)
 
-        slots = body.get("slots") or _pipeline()
+        # `or` would resurrect the default here: an explicitly empty pipeline is
+        # falsy, and a client that has removed every slot is entitled to be told
+        # it has none rather than handed the defaults back.
+        slots = body.get("slots")
+        if slots is None:
+            slots = _pipeline()
         action, slot_id = body.get("action"), body.get("slot")
 
         if action == "replace":

core/tests/test_routes_pipeline.py

@@ -147,3 +147,47 @@ def test_an_unknown_action_is_refused(server):
     status, body = _request(server, "POST", "/funpack/api/pipeline",
                             {"action": "explode", "slot": "model"})
     assert status == 400 and body["problems"]  # a malformed request, not an edit
+
+
+def test_an_explicitly_empty_pipeline_is_not_replaced_by_the_default(server):
+    """`slots or default()` resurrects the default, because an empty list is
+    falsy. A client that removed every slot is entitled to be told it has none
+    rather than handed the built-ins back."""
+    status, body = _request(server, "POST", "/funpack/api/pipeline",
+                            {"action": "check", "slots": []})
+    assert status == 200
+    assert body["slots"] == [], "an empty pipeline came back full"
+
+
+def test_omitting_slots_still_falls_back_to_the_default(server):
+    """The distinction that matters: absent means "use the default", empty means
+    "there is nothing". Only one of them is a fallback."""
+    status, body = _request(server, "POST", "/funpack/api/pipeline",
+                            {"action": "check"})
+    assert status == 200
+    assert body["slots"], "omitting slots should use the default pipeline"
+
+
+def test_removing_slots_one_by_one_never_resurrects_the_default(server):
+    """The failure this guards: each request carries the client's current slots,
+    so a fallback that fires on an empty list would silently hand the built-ins
+    back mid-way through and the count would jump UP."""
+    _status, body = _request(server, "GET", "/funpack/api/pipeline")
+    slots = body["slots"]
+    started_with = len(slots)
+    assert started_with > 1, "the default pipeline is too small to test this"
+
+    counts = [started_with]
+    # Consumers before sources: removing a source nothing can replace is refused,
+    # which is correct and not what this test is about.
+    for slot_id in [s["id"] for s in reversed(slots)]:
+        _status, body = _request(server, "POST", "/funpack/api/pipeline",
+                                 {"action": "remove", "slot": slot_id, "slots": slots})
+        if body["refused"]:
+            continue
+        slots = body["slots"]
+        counts.append(len(slots))
+
+    assert len(slots) < started_with, "nothing was actually removed"
+    assert counts == sorted(counts, reverse=True), (
+        f"the pipeline grew back part way through: {counts}")