From 4b1536be2ccc2edb6731c48e2f7a4e433330ecfc Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Tue, 9 Jun 2026 11:44:12 -0700 Subject: [PATCH] =?UTF-8?q?fix(ui):=20ws=20lifecycle=20round=202=20?= =?UTF-8?q?=E2=80=94=20dead-session=20revive=20+=20proxied=20tab-menu=20ve?= =?UTF-8?q?rbs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reported console bugs, one shared root: a pane can outlive its session, and nothing brought the two back together. Reconnect: an interactive pane whose stream died (ws closed/evicted elsewhere, node restart, re-home) could never reconnect while its tab existed — openPane() on an existing pane was focus-only, the controller's connect() is one-shot, and its 5s recovery loop re-dialed the SAME node forever (infinite 404 polling through the console proxy). The only workaround was closing the tab before resuming. - createInteractivePane now tracks terminal failure: 3 consecutive CLOSED recovery beats -> give up (stream closed, timers + any pending history load invalidated, status bar "Disconnected", opts.onDead fired once). host.onStreamOpen (new hook) resets the counter; isDead()/markDead()/base join the controller surface; onLogin ignores a dead controller — revive owns recovery, so a deliberately closed session is never resurrected by a timer. - PaneManager.openPane fires pane.onReopen(extra) when it targets an ALREADY-OPEN pane — the explicit-intent signal (saved-list resume, rail row, child link) that activate() can't carry (hooks no-op on the active pane, and onActivate also fires on plain tab switches). getPane() added for cross-cutting lifecycle signals. - The shell paints a click-to-reconnect banner on give-up — and immediately on Tier-1 ws_closed via the new TS_SHELL.notifySessionClosed seam (the console keeps the tab, unlike the standalone's auto-close, so the conversation stays readable). Reopen/banner-click revives: tear down the dead controller, re-resolve through the origin-first POST /open lane, rebuild. The forced resolve skips BOTH beginConnect fast paths (a stale Tier-1 row must not bypass /open) while a live node leads the hint chain (an origin-first /open then reuses a genuinely-live session instead of loading a duplicate on the old meta node). The standalone lane POSTs its local /open on revive too — /events 404s on an unloaded ws. - Coordinator parity: the factory exposes reconnect() (acts only on a missing/CLOSED stream; OPEN is healthy, CONNECTING is already being worked) and the pane's onReopen drives it — the saved-list resume POSTs /open before openPane, so a fresh stream is all it needs. Tab menu: a node-proxied interactive pane's dropdown gated every verb on classic globals that only exist in ui/static/app.js, so the console got a nearly-empty menu whose one surviving verb (Export) hit the console origin and 404'd. convTabMenu gains a base-aware fallback lane: verbs POST against the pane's OWN transport base (controller's exact base -> persisted node hint -> live Tier-1 node; a verb is omitted while no base is resolvable — never aimed at the wrong origin). Close/Delete confirm first (window.confirm, the coordinator precedent) and treat 404 as intent-satisfied (nothing left to stop/delete -> drop the tab). exportWorkstreamDownload takes the base. The standalone keeps its globals lane (incl. Fork) byte-identical, and an empty verb section no longer renders a leading separator. Verified: 189 JS-pin tests; two headless-Chrome live-DOM harnesses driving the real modules — console 16/16 (connect -> ws_closed -> banner -> reopen revives on a new node with the fresh hint -> give-up stops retrying -> live-node-led resolve), standalone 10/10 (globals menu intact, revive POSTs /open exactly once, no cluster resolve). --- tests/test_interactive_pane_js.py | 27 ++ tests/test_shell_js.py | 159 +++++++++- turnstone/console/static/app.js | 9 + .../console/static/coordinator/coordinator.js | 14 + turnstone/shared_static/interactive.js | 72 ++++- turnstone/shared_static/pane.js | 27 ++ turnstone/shared_static/shell.css | 22 ++ turnstone/shared_static/shell.js | 288 +++++++++++++++--- turnstone/shared_static/utils.js | 16 +- 9 files changed, 584 insertions(+), 50 deletions(-) diff --git a/tests/test_interactive_pane_js.py b/tests/test_interactive_pane_js.py index 4d6d6f6b..537fd39f 100644 --- a/tests/test_interactive_pane_js.py +++ b/tests/test_interactive_pane_js.py @@ -165,3 +165,30 @@ def test_approval_keyboard_shortcuts_wired() -> None: "the feedback field uses the converged .conv-feedback, not the retired " ".ts-approval-feedback" ) + + +def test_controller_terminal_dead_state() -> None: + """Lifecycle round 2: the console controller must STOP reconnect-polling a + session that is gone (closed / evicted / node restarted) — three consecutive + CLOSED recovery beats → give up: stream closed, status bar terminal, + ``opts.onDead()`` fired once. A successful stream open resets the counter + (the new host.onStreamOpen seam). ``isDead()`` / ``markDead()`` / ``base`` + are the shell's revive surface; a dead controller also ignores the login + re-arm (recovery may need a DIFFERENT node — the shell's revive owns it).""" + body = _INTERACTIVE.read_text(encoding="utf-8") + # The give-up ladder. + assert "let dead = false;" in body and "let failCount = 0;" in body + assert "const giveUp = function () {" in body + assert "failCount += 1;" in body and "if (failCount >= 3) giveUp();" in body + assert 'pane._sbTokens.textContent = "Disconnected"' in body, ( + "the terminal state must be worded distinctly from the transient Reconnecting…" + ) + assert "opts.onDead" in body, "the shell must hear about the give-up" + # The reset seam: Pane.connectSSE onopen → host.onStreamOpen → failCount = 0. + assert "this._host.onStreamOpen(this)" in body + assert "onStreamOpen() {}" in body, "the default host must carry the no-op" + # The shell-facing surface. + assert "isDead()" in body and "markDead: giveUp," in body + assert "base: base," in body, "the controller must expose its transport base" + # Dead controllers don't reconnect on re-auth. + assert "if (connected && !dead) pane._loadHistoryThenConnect(wsId);" in body diff --git a/tests/test_shell_js.py b/tests/test_shell_js.py index f7da8ae3..d3cbf954 100644 --- a/tests/test_shell_js.py +++ b/tests/test_shell_js.py @@ -453,7 +453,9 @@ def test_step7_tab_menu_wired_per_persona() -> None: header's removed Export + end (5e.2e) return here as Export + Close workstream (its controller's closeSession). The three-verb close (Close pane = pm.close != Close workstream != Delete) is the spine; the standalone interactive verbs - are feature-detected globals, so the console degrades to a reduced menu.""" + prefer the feature-detected globals (which also manage its local roster), and + a deployment without them (the console) falls back to the base-aware lane + (see test_tab_menu_base_aware_verb_lane).""" shell = _SHELL_JS.read_text(encoding="utf-8") assert "function convTabMenu(" in shell, "the shared tab-menu builder must exist" assert shell.count("pane.tabMenu =") >= 3, ( @@ -471,13 +473,52 @@ def test_step7_tab_menu_wired_per_persona() -> None: assert "exportWorkstreamDownload" in shell, "Export conversation must wire the shared util" # Deployment-aware: the standalone interactive verbs are feature-detected globals. assert 'typeof window.closeWorkstream === "function"' in shell, ( - "the interactive Close workstream is a standalone-only global (feature-detected)" + "the interactive Close workstream prefers the standalone global (feature-detected)" ) assert "refreshWorkstreamTitle" in shell and "confirmDeleteWorkstream" in shell, ( - "the interactive title/delete verbs are feature-detected standalone globals" + "the interactive title/delete verbs prefer the standalone globals" ) +def test_tab_menu_base_aware_verb_lane() -> None: + """Lifecycle round 2: a proxied interactive pane's tab menu must act on the + pane's OWN transport base, not the console origin — the globals lane only + exists on the standalone. convTabMenu therefore takes a `base` getter and + falls back to POSTing the verb at {base}/v1/api/workstreams/{ws}/{verb}; a + node-verb is OMITTED while no base is resolvable (never aimed at the wrong + origin), and Export forwards the base to the shared util (a proxied export + must come from the node that owns the conversation).""" + shell = _SHELL_JS.read_text(encoding="utf-8") + assert "function postWsVerb(" in shell, "the base-aware verb POST helper must exist" + assert '"/v1/api/workstreams/" + encodeURIComponent(wsId) + "/" + verb' in shell + # The interactive pane supplies its current base: live controller's (exact), + # else the persisted node hint, else the live Tier-1 node, else null. + assert "const menuBase = ()" in shell, "the interactive pane must expose a base getter" + assert "pane._ctl && pane._ctl.base != null" in shell, ( + "a built controller's base is authoritative for the menu verbs" + ) + # Fallback verbs exist for the console: refresh-title / title / close / delete. + for verb in ('"refresh-title"', '"title"', '"close"', '"delete"'): + assert ( + f"postWsVerb(base, wsId, {verb}" in shell + or f"postWsVerb(closeBase, id, {verb}" in shell + ), f"the {verb} verb must have a base-aware fallback" + # Export rides the base too (3-arg form), and node-verbs are null-gated. + assert "exportWorkstreamDownload(wsId, null, base)" in shell + assert "base != null" in shell, "node-verbs must be omitted while the base is unresolved" + # Destructive fallbacks confirm first (window.confirm is the house precedent). + assert shell.count("window.confirm(") >= 2, ( + "the close + delete fallbacks must confirm before acting" + ) + # No leading separator when the verb section is empty. + assert "if (items.length) items.push({ separator: true })" in shell + util = (_SHARED / "utils.js").read_text(encoding="utf-8") + assert "function exportWorkstreamDownload(wsId, btn, base)" in util, ( + "the shared export util must accept the transport base" + ) + assert '(base || "") +' in util, "the export URL must be base-prefixed" + + def test_step7_tab_menu_css_promoted_shared() -> None: """Step 7: the dropdown chrome is promoted to the SHARED shell sheet (so both deployments render it), recovered from the retired .ws-tab-dropdown design but @@ -559,3 +600,115 @@ def test_step7_auth_gated_open_pane() -> None: assert "canOpen:" in shell and "onDeny:" in shell, ( "the coordinator registerType must supply the auth gate" ) + + +# --------------------------------------------------------------------------- +# Workstream-lifecycle round 2: dead-session revive + explicit-reopen seam. +# --------------------------------------------------------------------------- + + +def test_pane_manager_reopen_seam() -> None: + """openPane() on an ALREADY-OPEN pane fires `pane.onReopen(extra)` — the + explicit-intent signal (saved-list resume, rail row, child link) that + activate() cannot carry: hooks no-op on the already-active pane, and + onActivate also fires on plain tab switches. Fired AFTER activate so the + pane is visible when it reacts. getPane lets the shell reach a pane for + cross-cutting lifecycle signals.""" + pane = _PANE_JS.read_text(encoding="utf-8") + assert "onReopen(extra) {}" in pane, "ShellPane must document the onReopen hook" + assert "const existed = !!pane" in pane, "openPane must remember create-vs-focus" + assert "pane.onReopen(extra)" in pane, "openPane must fire onReopen on existing panes" + # Ordering: the reopen signal comes after activation. + assert pane.index("this.activate(paneId)") < pane.index("pane.onReopen(extra)") + assert "getPane(type, id)" in pane, "PaneManager must expose getPane for the shell" + + +def test_interactive_pane_dead_session_revive() -> None: + """The reported round-2 bug: an interactive session whose stream died + (closed / evicted / node restarted) could never reconnect while its tab + existed — openPane focused the dead pane, onActivate's connect() is one-shot, + and the controller's recovery loop re-dialed the SAME node forever. The fix: + the shell paints a click-to-reconnect banner when the controller reports + dead, and an explicit reopen (onReopen) revives — tear down the dead + controller, re-resolve the node (POST /open), rebuild.""" + shell = _SHELL_JS.read_text(encoding="utf-8") + assert "const showDeadBanner = ()" in shell, "the dead banner painter must exist" + assert "pane-dead-banner" in shell, "the banner carries its own style hook" + css = _SHELL_CSS.read_text(encoding="utf-8") + assert ".pane-dead-banner" in css and ".pane-dead-banner:focus-visible" in css, ( + "the banner is a real