diff --git a/tests/test_shell_js.py b/tests/test_shell_js.py index 373b0084..f7da8ae3 100644 --- a/tests/test_shell_js.py +++ b/tests/test_shell_js.py @@ -241,6 +241,16 @@ def test_console_launcher_node_strategy() -> None: assert "Composer.prototype.setOptionFieldVisible" in composer, ( "composer must support conditionally revealing an option field" ) + # Review bug-1 regression: picking a node fires the composer `change` event, + # which re-runs _populateLauncherNodes -> setOptionChoices (a rebuild that + # resets the to its placeholder. Without this + // the selection is wiped the instant it's made, so a Specific-node session can + // never be launched. Restored only if the node still exists (else it falls + // back to the placeholder, which is correct — the picked node disappeared). + const previous = _homeCoordComposer.getOptionValue("node_id"); const choices = []; if (clusterState && clusterState.nodes) { Object.keys(clusterState.nodes) @@ -1079,6 +1086,7 @@ function _populateLauncherNodes() { }); } _homeCoordComposer.setOptionChoices("node_id", choices); + if (previous) _homeCoordComposer.setOptionValue("node_id", previous); } function _wireLauncherToggle() { @@ -1980,37 +1988,34 @@ window.TS_APP.resolveInteractiveNode = function (wsId, hintNodeId) { return r.ok ? { nodeId: nodeId } : { status: r.status }; }); }; + // Single error surface: the failure is returned as {error} and the interactive + // pane writes it into its .pane-status line (the pane is always the active tab + // when it resolves). No toast — one source of truth, no wording drift. const failResult = function (status) { - if (status === 429) { - showToast("Node at capacity; close a session and retry"); + if (status === 429) return { error: "Node at capacity — close a session and retry." }; - } - if (status === 403) { - showToast("Not permitted to open this session"); + if (status === 403) return { error: "You don’t have permission to open this session." }; - } return { error: "Could not open this session (" + status + ")." }; }; const routeFallback = function () { return authFetch("/v1/api/route?ws_id=" + encodeURIComponent(wsId)) .then(function (r) { if (!r.ok) { - const msg = - r.status === 503 - ? "No nodes available to open this session." - : "Could not locate the session node (" + r.status + ")."; - showToast(msg); - return { error: msg }; + return { + error: + r.status === 503 + ? "No nodes available to open this session." + : "Could not locate the session node (" + r.status + ").", + }; } return r.json(); }) .then(function (route) { - if (!route) return { error: "Could not locate the session node." }; - if (route.error) return route; // route fetch already failed + toasted - if (!route.node_id) { - showToast("Could not locate the session node"); + if (!route || route.error) + return route || { error: "Could not locate the session node." }; + if (!route.node_id) return { error: "Could not locate the session node." }; - } return openOn(route.node_id).then(function (res) { return res.nodeId ? res : failResult(res.status); }); diff --git a/turnstone/shared_static/composer.js b/turnstone/shared_static/composer.js index 159ee746..a0b9fb6f 100644 --- a/turnstone/shared_static/composer.js +++ b/turnstone/shared_static/composer.js @@ -754,8 +754,9 @@ // Show/hide a single options field (its label + control row). Lets a caller // reveal a field conditionally — e.g. the launcher shows the node picker only // for the interactive persona, and the node list only under "Specific node". - // Toggles inline display (robust against the field row's own flex/grid rule, - // which [hidden] alone would lose a specificity battle with). + // Sets inline display because the row is `display: contents` (chat.css:537), + // which outweighs the [hidden] attribute's UA `display:none` on specificity — + // so toggling [hidden] alone would not actually hide the row. Composer.prototype.setOptionFieldVisible = function (id, visible) { var ctrl = this._optionFields && this._optionFields[id]; if (!ctrl) return; diff --git a/turnstone/shared_static/shell.css b/turnstone/shared_static/shell.css index 774f6231..872a41cd 100644 --- a/turnstone/shared_static/shell.css +++ b/turnstone/shared_static/shell.css @@ -721,6 +721,12 @@ color: var(--fg-dim); font-size: 13px; } +/* An errored resolve is click-to-retry (see shell.js showResolveError). */ +.pane-status--retry { + cursor: pointer; + text-decoration: underline; + text-underline-offset: 2px; +} /* ===== Dashboard session launcher — persona toggle (coordinator | interactive). A console-dashboard control; lives here because the L-shell loads shell.css. */ diff --git a/turnstone/shared_static/shell.js b/turnstone/shared_static/shell.js index c816a5b8..5c0f2f54 100644 --- a/turnstone/shared_static/shell.js +++ b/turnstone/shared_static/shell.js @@ -391,14 +391,77 @@ async function mountShell() { // resolved node after ensureInteractiveNode settles, below. if (extra && extra.nodeId) pane.meta = { nodeId: extra.nodeId }; pane.onMount = function () { - // The controller is built LAZILY on first activate — only after the owning - // node is resolved AND the session is (re)opened there (the node /events - // stream 404s on a ws not loaded on its node, so a rehydrated pane can't - // just connect blind). onMount only reserves a status line so a - // not-yet-resolved pane isn't a blank box. + // The controller is built LAZILY on first activate (see beginConnect) — + // a node-proxied session may need its node resolved + (re)opened first. + // onMount only reserves a status line so a not-yet-connected pane isn't a + // blank box. this._statusEl = make("div", "pane-status", "Connecting…"); this.bodyEl.append(this._statusEl); }; + // Build the controller on a resolved node, persist that node for the next + // reload, and open the stream. nodeId null = standalone-local (base=""). + const buildController = (nodeId) => { + if (pane._closed) return; // resolved after the tab was closed + // Persist the node so a reload restores onto the SAME node — but skip the + // write when it already matches (no redundant sessionStorage round-trip). + if (nodeId && (!pane.meta || pane.meta.nodeId !== nodeId)) { + pane.meta = { nodeId }; + pm.setPaneMeta(pane.id, pane.meta); + } + if (pane._statusEl && pane._statusEl.parentNode) pane._statusEl.remove(); + pane._statusEl = null; + pane._ctl = createInteractivePane(pane.bodyEl, id, { + nodeId, + onClose: () => pm.close(pane.id), + }); + pane._ctl.connect(); + if (window.TS_LOGIN && pane._ctl.onLogin) { + pane._loginArmed = true; + window.TS_LOGIN.subscribe(pane._ctl.onLogin); + } + }; + // Errored resolve (capacity / no node free): show it in the status line and + // offer a one-click retry — re-clicking the tab won't re-fire onActivate + // (PaneManager fires it only on a pane CHANGE), so without this a transient + // failure would strand the pane until the user closed + reopened it. + const showResolveError = (msg) => { + const el = pane._statusEl; + if (!el) return; + el.className = "pane-status pane-status--retry msg error"; + el.textContent = msg || "Could not connect to this session."; + el.title = "Click to retry"; + el.onclick = () => { + if (pane._ctl || pane._resolving) return; + el.className = "pane-status"; + el.title = ""; + el.onclick = null; + el.textContent = "Connecting…"; + beginConnect(); + }; + }; + // First-activate connect. A LIVE session (Tier-1 already names its node, so + // it is loaded there) connects DIRECTLY — no /open round-trip; this is the + // hot rail / active-row / just-created path. Standalone runs locally. Only + // the dormant / reload case (the snapshot has no node for this ws) resolves + // the node + (re)opens the session, whose /events would otherwise 404. + const beginConnect = () => { + const liveNode = caps.cluster ? nodeForWs(id) : null; + if (liveNode || !caps.cluster) { + buildController(liveNode || null); + return; + } + pane._resolving = true; + const hint = (pane.meta && pane.meta.nodeId) || (extra && extra.nodeId); + ensureInteractiveNode(caps, id, hint).then((res) => { + pane._resolving = false; + if (pane._closed) return; // closed mid-resolve — don't build into a detached body + if (!res || res.error) { + showResolveError(res && res.error); + return; + } + buildController(res.nodeId); + }); + }; pane.onActivate = function () { pm.setTabGlyph(pane.id, glyph(stateForWs(id))); // live Tier-1 state glyph if (this._ctl) { @@ -406,35 +469,7 @@ async function mountShell() { return; } if (this._resolving) return; // first-activate resolve already in flight - this._resolving = true; - const hint = (this.meta && this.meta.nodeId) || (extra && extra.nodeId); - ensureInteractiveNode(caps, id, hint).then((res) => { - this._resolving = false; - if (this._closed) return; // pane closed mid-resolve — don't build into a detached body - if (!res || res.error) { - if (this._statusEl) { - this._statusEl.className = "pane-status msg error"; - this._statusEl.textContent = - (res && res.error) || "Could not connect to this session."; - } - return; - } - // Pin + persist the resolved node so the next reload reuses it. - this.meta = { nodeId: res.nodeId }; - pm.setPaneMeta(pane.id, this.meta); - if (this._statusEl && this._statusEl.parentNode) - this._statusEl.remove(); - this._statusEl = null; - this._ctl = createInteractivePane(this.bodyEl, id, { - nodeId: res.nodeId, - onClose: () => pm.close(pane.id), - }); - this._ctl.connect(); - if (window.TS_LOGIN && this._ctl.onLogin) { - this._loginArmed = true; - window.TS_LOGIN.subscribe(this._ctl.onLogin); - } - }); + beginConnect(); }; pane.onDeactivate = function () { if (this._ctl && this._ctl.deactivate) this._ctl.deactivate();