From 6f5cb33923ff69096be8f2c06cd062f82a0885ba Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Tue, 28 Apr 2026 00:25:32 -0700 Subject: [PATCH] =?UTF-8?q?feat(coord):=20composer=20parity=20with=20inter?= =?UTF-8?q?active=20=E2=80=94=20stop/queue/attach=20(#438)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(coord): composer parity with interactive — stop/queue/attach Bring the coordinator one-pane UI to feature parity with the interactive composer: in-composer Stop button replaces Send during a turn, queue-while-busy with !!! priority + dismiss, paperclip attach + drag/drop/paste. The coord backend already supported all three (lifted send/cancel/attachment handlers, emit_message_queued=True, supports_attachments=True); this wires the UI through. Backend: - Wire make_dequeue_handler(coord_endpoint_config) so DELETE /v1/api/workstreams/{ws_id}/send works for coord-kind workstreams. - Add the matching OpenAPI EndpointSpec. - Five new test_dequeue_* tests (success, not_found, missing msg_id, unknown ws, scope gate) pin the URL/method/scope contract. Frontend extraction: - New shared modules composer_attachments.js (createAttachmentController) and composer_queue.js (createQueueController) replace ~300 LOC of pre-existing duplication between the interactive Pane and the coord IIFE. Both panes now share one source of truth for the chip pipeline, optimistic queue bubble, and busy-edge promote sweep. Coordinator pane: - Composer constructor adds attachments/stopBtn/queueWhileBusy/ busyPlaceholder/dragDrop options. - setBusy now drives off SSE state_change (running/thinking/attention → busy; idle/error → idle), with composer.setBusy unconditional and the edge-only work (timer cleanup + queue.onIdleEdge) gated on the actual transition. - Cancel uses the in-composer Stop with a 2s "Force Stop" affordance + 10s safety auto-recover; the legacy header-mounted #coord-cancel-btn is removed. - coordCloseSession suspends SSE before close and re-establishes it on any failure path so the UI never goes dark on a still-alive session. - Race handling: bind() releases the queued slot server-side when the bubble was already dismissed or promoted; rehydrate re-checks getWsId in its .then so a stale-tab response can't clobber the new tab's chips. Interactive pane: - Pane class adopts the same controllers via this.attachments / this.queue. Pane.prototype.uploadAttachment, _renderAttachmentChip, _swapPlaceholderChip, _removeAttachmentChip, removeAttachment, rehydrateAttachments wrapper, addQueuedMessage, _dequeueMessage, and _promoteQueuedMessages are all gone — the controllers own the state. - setBusy collapses to the same shape as coord: composer.setBusy + edge calc + queue.onIdleEdge on idle. CSS: - Move .msg-queued / .queued-badge / .queued-dismiss styles from ui/static/style.css into shared_static/chat.css so both panes share one rendering. - Add .coord-drop-target overlay rule so the coord pane shows the drag-and-drop affordance. Tests pass: 160 in the impacted suites (coord endpoints + attachments + session routes), including 5 new dequeue tests for coord. * fix(coord): Copilot review + lint follow-ups Lint: - ruff: cast(MagicMock, ...) → cast("MagicMock", ...) under ``from __future__ import annotations`` (UP037). Copilot review (PR #438): - composer_queue _sendDelete now invokes onAfterDequeue on success so a bind() race-DELETE (queued bubble dismissed pre-bind or promote sweep raced ahead) still rehydrates the caller's chip pile; released attachment reservations no longer linger invisibly until the next page load. - Coord's createQueueController gains onAfterDequeue: attachments. rehydrate(). The previous omission was a v2 review carry-over from before coord supported attachments — now it does, so the same contract as interactive applies. - Both panes' send-response handler now accepts status:queued without a queuedEl (SSE-not-yet-connected race on initial load): flips busy so subsequent sends queue correctly. The current message keeps its optimistic user bubble — accepted UX gap (no in-UI dismiss for THIS message) since flipping a rendered user bubble into a queued one mid-stream would be jarring. - Doc updates: chat.css comment + composer_queue.js module docstring refer to the renamed onIdleEdge() instead of the removed promote()/promoteQueuedMessages. --- tests/test_coordinator_endpoints.py | 105 +++++ turnstone/api/console_spec.py | 18 + turnstone/console/server.py | 2 + .../static/coordinator/coordinator.css | 32 ++ .../console/static/coordinator/coordinator.js | 346 ++++++++++++-- .../console/static/coordinator/index.html | 3 +- turnstone/shared_static/chat.css | 43 ++ .../shared_static/composer_attachments.js | 307 ++++++++++++ turnstone/shared_static/composer_queue.js | 229 +++++++++ turnstone/ui/static/app.js | 442 ++++-------------- turnstone/ui/static/index.html | 2 + turnstone/ui/static/style.css | 32 -- 12 files changed, 1116 insertions(+), 445 deletions(-) create mode 100644 turnstone/shared_static/composer_attachments.js create mode 100644 turnstone/shared_static/composer_queue.js diff --git a/tests/test_coordinator_endpoints.py b/tests/test_coordinator_endpoints.py index 1e2c499d..b2e2271b 100644 --- a/tests/test_coordinator_endpoints.py +++ b/tests/test_coordinator_endpoints.py @@ -11,6 +11,7 @@ the lifted ``approve`` and ``close`` handlers from from __future__ import annotations +from typing import cast from unittest.mock import MagicMock import httpx @@ -62,6 +63,7 @@ from turnstone.core.session_routes import ( make_cancel_handler, make_close_handler, make_create_handler, + make_dequeue_handler, make_detail_handler, make_history_handler, make_list_handler, @@ -165,6 +167,11 @@ def _make_client( make_send_handler(_coord_endpoint_config), methods=["POST"], ), + Route( + "/v1/api/workstreams/{ws_id}/send", + make_dequeue_handler(_coord_endpoint_config), + methods=["DELETE"], + ), Route( "/v1/api/workstreams/{ws_id}/approve", make_approve_handler(_coord_endpoint_config), @@ -742,6 +749,104 @@ def test_send_requires_message(storage): assert resp.status_code == 400 +# --------------------------------------------------------------------------- +# Dequeue (DELETE /send) — lifted handler wired onto the coord endpoint +# config. Pins the URL/scope contract so a regression in the route table +# (wrong endpoint_config, wrong method, missing gate) trips a test. +# --------------------------------------------------------------------------- + + +def test_dequeue_removes_queued_coord_message(storage): + """POST /send while busy → queued; DELETE /send with msg_id → removed.""" + mgr = _build_mgr(storage) + ws = mgr.create(user_id="user-1") + # _build_mgr's session_factory returns a MagicMock posing as a + # ChatSession so the test can stub queue_message / dequeue_message. + session = cast("MagicMock", ws.session) + + # Force the queue path by marking the worker as already running. + # The lifted send handler hands off to ``session_worker.send`` which + # picks ``enqueue`` over ``run`` when ``ws._worker_running`` is True. + ws._worker_running = True + session.queue_message.return_value = ("hi", "important", "msg-abc") + session.dequeue_message.return_value = True + + client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry()) + send_resp = client.post( + f"/v1/api/workstreams/{ws.id}/send", + json={"message": "hi"}, + headers=_COORD_HEADERS, + ) + assert send_resp.status_code == 200, send_resp.text + body = send_resp.json() + assert body["status"] == "queued" + assert body["msg_id"] == "msg-abc" + + dequeue_resp = client.request( + "DELETE", + f"/v1/api/workstreams/{ws.id}/send", + json={"msg_id": "msg-abc"}, + headers=_COORD_HEADERS, + ) + assert dequeue_resp.status_code == 200, dequeue_resp.text + assert dequeue_resp.json() == {"status": "removed"} + session.dequeue_message.assert_called_with("msg-abc") + + +def test_dequeue_unknown_msg_id_returns_not_found(storage): + mgr = _build_mgr(storage) + ws = mgr.create(user_id="user-1") + cast("MagicMock", ws.session).dequeue_message.return_value = False + client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry()) + resp = client.request( + "DELETE", + f"/v1/api/workstreams/{ws.id}/send", + json={"msg_id": "missing"}, + headers=_COORD_HEADERS, + ) + assert resp.status_code == 200 + assert resp.json() == {"status": "not_found"} + + +def test_dequeue_requires_msg_id(storage): + mgr = _build_mgr(storage) + ws = mgr.create(user_id="user-1") + client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry()) + resp = client.request( + "DELETE", + f"/v1/api/workstreams/{ws.id}/send", + json={}, + headers=_COORD_HEADERS, + ) + assert resp.status_code == 400 + + +def test_dequeue_unknown_ws_returns_404(storage): + mgr = _build_mgr(storage) + client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry()) + resp = client.request( + "DELETE", + "/v1/api/workstreams/0000000000000000000000000000000000000000000000000000000000000000/send", + json={"msg_id": "anything"}, + headers=_COORD_HEADERS, + ) + assert resp.status_code == 404 + assert "coordinator not found" in resp.json()["error"] + + +def test_dequeue_requires_admin_coordinator_scope(storage): + mgr = _build_mgr(storage) + ws = mgr.create(user_id="user-1") + client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry()) + resp = client.request( + "DELETE", + f"/v1/api/workstreams/{ws.id}/send", + json={"msg_id": "msg-abc"}, + headers={"X-Test-User": "user-1", "X-Test-Perms": "read"}, + ) + assert resp.status_code == 403 + + def test_close_records_audit_and_removes_from_mgr(storage): mgr = _build_mgr(storage) ws = mgr.create(user_id="user-1") diff --git a/turnstone/api/console_spec.py b/turnstone/api/console_spec.py index 9acef4dd..4138894a 100644 --- a/turnstone/api/console_spec.py +++ b/turnstone/api/console_spec.py @@ -128,6 +128,7 @@ from turnstone.api.schemas import ( UserInfo, ) from turnstone.api.server_schemas import ( + DequeueRequest, ListAttachmentsResponse, ListSkillSummaryResponse, ListWorkstreamsResponse, @@ -1190,6 +1191,23 @@ CONSOLE_ENDPOINTS: list[EndpointSpec] = [ error_codes=[400, 403, 404, 500, 503], tags=["Coordinator"], ), + EndpointSpec( + "/v1/api/workstreams/{ws_id}/send", + "DELETE", + "Cancel a queued coordinator message", + description=( + "Removes a previously-queued message identified by ``msg_id`` " + "from the coordinator session's pending queue. Returns " + "``status: removed`` when the queue had the entry, " + "``status: not_found`` otherwise. Reservations attached to " + "the dequeued message are released so the attachments can be " + "reused — parity with the interactive surface." + ), + request_model=DequeueRequest, + response_model=StatusResponse, + error_codes=[400, 403, 404, 503], + tags=["Coordinator"], + ), EndpointSpec( "/v1/api/workstreams/{ws_id}/send", "POST", diff --git a/turnstone/console/server.py b/turnstone/console/server.py index 8ea29033..375a07b3 100644 --- a/turnstone/console/server.py +++ b/turnstone/console/server.py @@ -64,6 +64,7 @@ from turnstone.core.session_routes import ( make_cancel_handler, make_close_handler, make_create_handler, + make_dequeue_handler, make_detail_handler, make_events_handler, make_history_handler, @@ -10190,6 +10191,7 @@ def create_app( supports_close_reason=False, ), send=make_send_handler(coord_endpoint_config), # lifted: shared body (P1.5) + dequeue=make_dequeue_handler(coord_endpoint_config), # lifted: shared body approve=make_approve_handler(coord_endpoint_config), # lifted: shared body cancel=make_cancel_handler( # lifted: shared body coord_endpoint_config, diff --git a/turnstone/console/static/coordinator/coordinator.css b/turnstone/console/static/coordinator/coordinator.css index 088122cd..c1f8dbd1 100644 --- a/turnstone/console/static/coordinator/coordinator.css +++ b/turnstone/console/static/coordinator/coordinator.css @@ -269,6 +269,38 @@ border-color: var(--err); } +/* ========================================================================== + Drag-and-drop overlay — applied to #coord-main while the user is + dragging files from the OS over the chat pane. Composer wires this on + construction (dragDrop.targetEl=coord-main, dropClass=coord-drop-target). + The dashed outline + "Drop file to attach" overlay mirror the interactive + pane's pattern so the affordance reads the same in both surfaces. + ========================================================================== */ +#coord-main { + position: relative; /* anchors the ::after overlay */ +} +#coord-main.coord-drop-target { + outline: 2px dashed var(--accent); + outline-offset: -6px; +} +#coord-main.coord-drop-target::after { + content: "Drop file to attach"; + position: absolute; + inset: 0; + display: flex; + align-items: center; + justify-content: center; + background: color-mix(in srgb, var(--accent) 10%, transparent); + color: var(--ink); + font-family: var(--font-mono); + font-size: 14px; + font-weight: 600; + letter-spacing: 0.05em; + text-transform: uppercase; + pointer-events: none; + z-index: 10; +} + /* Match .btn .kbd (in shared_static/ui-base.css) — --ink-3 clears AA at 10px, --ink-4 is borderline on light panels. */ .approval-dock button.act .kbd { diff --git a/turnstone/console/static/coordinator/coordinator.js b/turnstone/console/static/coordinator/coordinator.js index 59254c30..19fddb44 100644 --- a/turnstone/console/static/coordinator/coordinator.js +++ b/turnstone/console/static/coordinator/coordinator.js @@ -34,29 +34,76 @@ } const messagesEl = document.getElementById("coord-messages"); + const coordMain = document.getElementById("coord-main"); const composerMount = document.getElementById("coord-composer-mount"); const composer = new Composer(composerMount, { placeholder: "Message the coordinator\u2026", ariaLabel: "Coordinator input", - onSend: function (text) { - coordSend(text); + attachments: { + onAttach: function (file) { + attachments.upload(file); + }, }, - // Preserve the coordinator's pre-refactor Enter-on-touch behaviour - // — coordinator sessions are short and tap-to-send via the - // on-screen Return key is a quicker workflow than tapping a Send - // button. + stopBtn: true, + queueWhileBusy: true, + busyPlaceholder: "Queue a message\u2026 (!!! for urgent)", + onSend: function () { + coordSend(); + }, + onStop: function () { + cancelGeneration(); + }, + // Coord sessions are short — tap-to-send via the on-screen Return + // key is faster than tapping a Send button on touch. touchEnterSends: true, - // No attachments yet — coordinator-side attach mid-conversation - // requires a backend ingest path that doesn't exist; defer. - // No stopBtn — coordinator already has a header-mounted cancel - // button (#coord-cancel-btn) that fires coordCancel(). + dragDrop: { targetEl: coordMain, dropClass: "coord-drop-target" }, }); + const stopBtn = composer.stopBtn; + const attachments = createAttachmentController({ + chipsEl: composer.chipsEl, + getWsId: function () { + return wsId; + }, + }); + const queue = createQueueController({ + messagesEl: messagesEl, + getWsId: function () { + return wsId; + }, + // Coord chat bubbles wrap content in a .msg-body div (appendMsg + // below); the queue bubble matches so its border + padding align. + wrapInBody: true, + // Re-fetch attachments after a dequeue so the user can see (and + // reuse) any reservations the server-side dequeue released. Trades + // a small in-flight-placeholder clobbering window for the strictly + // worse alternative of attachments lingering invisibly until the + // next page load. + onAfterDequeue: function () { + attachments.rehydrate(); + }, + // Idle-edge cleanup of the cancel/force-stop timers — without + // this they fire on the *next* busy turn, relabel Stop to "Force + // Stop", and surface a misleading "Cancel didn't complete in + // time" toast unrelated to the new turn. + onIdle: function () { + if (cancelTimeoutId) { + clearTimeout(cancelTimeoutId); + cancelTimeoutId = null; + } + if (forceTimeoutId) { + clearTimeout(forceTimeoutId); + forceTimeoutId = null; + } + }, + }); + let busy = false; + let cancelTimeoutId = null; + let forceTimeoutId = null; const statusEl = document.getElementById("coord-status"); const sseEl = document.getElementById("coord-sse-status"); const nameEl = document.getElementById("coord-name"); const approvalBar = document.getElementById("coord-approval-bar"); const approvalTools = document.getElementById("coord-approval-tools"); - const cancelBtn = document.getElementById("coord-cancel-btn"); const childrenTreeEl = document.getElementById("coord-children-tree"); const childrenCountEl = document.getElementById("coord-children-count"); const childrenRefreshBtn = document.getElementById("coord-children-refresh"); @@ -578,38 +625,138 @@ // Send / cancel / close // ------------------------------------------------------------------ - window.coordSend = function (text) { - const msg = (text || "").trim(); - if (!msg) return false; + // Busy reflects whether the worker is mid-turn. SSE state_change + // events drive it (running/thinking/attention → busy; idle/error → + // idle) so a server-side transition the user didn't initiate + // (another tab, judge reset) still keeps the composer in sync. + // + // composer.setBusy runs unconditionally so the Stop button label / + // dataset.forceCancel / placeholder stay canonical even on a + // redundant call — that idempotent reset is the contract any future + // caller relies on. queue.onIdleEdge runs only on the actual edge + // (it's the heavier work — querySelectorAll-driven promote sweep + // plus the cancel-timer cleanup wired via the onIdle hook above). + function setBusy(b) { + const next = !!b; + composer.setBusy(next); + const edge = next !== busy; + busy = next; + if (edge && !next) queue.onIdleEdge(); + } + + window.coordSend = function () { + const text = composer.value; + const trimmed = (text || "").trim(); + if (!trimmed) return false; + + const snap = attachments.snapshot(); + + let queuedEl = null; + if (busy) { + // Server re-parses the !!! prefix to set queue priority — the + // optimistic bubble strips it for display. + let displayText = trimmed; + let priority = "notice"; + if (trimmed.startsWith("!!!")) { + displayText = trimmed.slice(3).trimStart(); + priority = "important"; + } + queuedEl = queue.addQueuedMessage(displayText, priority); + } else { + setBusy(true); + appendText("user", trimmed, { label: "you" }); + } composer.clear(); - composer.setBusy(true); - postJSON("/v1/api/workstreams/" + encodeURIComponent(wsId) + "/send", { - message: msg, + + authFetch("/v1/api/workstreams/" + encodeURIComponent(wsId) + "/send", { + method: "POST", + credentials: "include", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + message: trimmed, + attachment_ids: snap.attachment_ids, + }), }) - .then((resp) => { - if (!resp.ok) { - return resp.text().then((txt) => { - throw new Error("send failed: " + resp.status + " " + txt); + .then((r) => r.json()) + .then((data) => { + if (data && data.status === "queued" && data.msg_id) { + // Race: server returned queued but the client thought it was + // idle (SSE state_change hadn't arrived yet on initial load / + // reconnect). The optimistic user bubble is already in the + // log; we can't bind msg_id to a queued bubble retroactively + // without flipping the visual state mid-stream. Flip the busy + // flag so any subsequent send takes the queue path correctly, + // and accept the small UX gap (no in-UI dismiss for THIS + // message). The server still delivers it on worker drain. + if (queuedEl) queue.bind(queuedEl, data.msg_id); + else setBusy(true); + attachments.consume(data.attached_ids, data.dropped_attachment_ids); + } else if (data && data.status === "busy") { + if (queuedEl) queue.remove(queuedEl); + appendText("error", "Server is busy. Please wait.", { + label: "error", }); + if (!queuedEl) setBusy(false); + } else if (data && data.status === "queue_full") { + if (queuedEl) queue.remove(queuedEl); + appendText("error", "Message queue full. Please wait.", { + label: "error", + }); + } else { + attachments.consume( + data && data.attached_ids, + data && data.dropped_attachment_ids, + ); } - appendText("user", msg, { label: "you" }); }) .catch((e) => { - if (typeof toast !== "undefined" && toast.error) toast.error(String(e)); - else console.error(e); - }) - .finally(() => { - composer.setBusy(false); + if (queuedEl) queue.remove(queuedEl); + appendText( + "error", + "Connection error: " + (e && e.message ? e.message : e), + { label: "error" }, + ); + if (!queuedEl) setBusy(false); }); return false; }; - window.coordCancel = function () { - postJSON( - "/v1/api/workstreams/" + encodeURIComponent(wsId) + "/cancel", - {}, - ).catch((e) => console.error(e)); - }; + function cancelGeneration() { + if (!busy || stopBtn.disabled) return; + const force = stopBtn.dataset.forceCancel === "true"; + stopBtn.disabled = true; + authFetch("/v1/api/workstreams/" + encodeURIComponent(wsId) + "/cancel", { + method: "POST", + credentials: "include", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ force: force }), + }) + .then(() => { + if (force) { + // Force cancel abandons the worker thread server-side; the + // SSE state_change → idle may not arrive (the thread may be + // stuck past the cancel checkpoint), so transition the UI + // directly. setBusy(false) clears cancel/force timers and + // composer.setBusy(false) resets the Stop button label, + // aria-label, and dataset.forceCancel — so the next turn + // starts in graceful-cancel mode without a stale Force Stop + // primed for the first click. + appendText("info", "Force stopped. Previous generation abandoned.", { + label: "info", + }); + setBusy(false); + } + }) + .catch((e) => { + appendText( + "error", + "Cancel error: " + (e && e.message ? e.message : e), + { label: "error" }, + ); + // Re-enable so the user can retry. + if (busy) stopBtn.disabled = false; + }); + } window.coordCloseSession = async function () { if ( @@ -618,17 +765,63 @@ ) ) return; + // Suspend SSE reconnect first — the moment the server pops the ws + // from coord_mgr the next reconnect would 404 and surface a stream + // error toast right before the redirect, which reads as "the end + // button broke" even though the close succeeded. On any failure + // path we MUST resume SSE before returning so the user isn't left + // staring at a stale page disconnected from a still-alive session. + const resumeSse = () => { + try { + connectSSE(); + } catch (_) { + /* connectSSE schedules its own reconnect on failure */ + } + }; try { - const resp = await postJSON( + if (evtSource) evtSource.close(); + if (reconnectTimer) { + clearTimeout(reconnectTimer); + reconnectTimer = null; + } + } catch (_) { + /* best-effort suspension */ + } + let resp; + try { + resp = await postJSON( "/v1/api/workstreams/" + encodeURIComponent(wsId) + "/close", {}, ); - if (!resp.ok) throw new Error("close failed: HTTP " + resp.status); - window.location.href = "/"; } catch (e) { - if (typeof toast !== "undefined" && toast.error) toast.error(String(e)); - else console.error(e); + // authFetch throws Error("auth") and shows the login modal on + // 401; other network failures land here too. Surface the cause + // visibly — silent toast.error wasn't enough for operators + // troubleshooting a stuck end-button. + const msg = + e && e.message === "auth" + ? "Sign-in required to end this session." + : "Close request failed: " + (e && e.message ? e.message : e); + if (typeof toast !== "undefined" && toast.error) toast.error(msg); + else window.alert(msg); + resumeSse(); + return; } + if (!resp.ok) { + let detail = "HTTP " + resp.status; + try { + const body = await resp.json(); + if (body && body.error) detail += " — " + body.error; + } catch (_) { + /* non-JSON body — fall back to status code */ + } + const msg = "Could not end session: " + detail; + if (typeof toast !== "undefined" && toast.error) toast.error(msg); + else window.alert(msg); + resumeSse(); + return; + } + window.location.href = "/"; }; // ------------------------------------------------------------------ @@ -891,22 +1084,74 @@ break; case "state_change": statusEl.textContent = ev.state || ""; - cancelBtn.style.display = - ev.state === "running" || ev.state === "thinking" ? "" : "none"; + // Drive the composer's busy state from the canonical + // server-side workstream state so the Stop button + queue + // mode follow whatever the worker is doing — including + // transitions we didn't initiate (cross-tab cancel, judge + // reset, idle-after-error). Mirrors the interactive pane. + if (ev.state === "idle" || ev.state === "error") { + setBusy(false); + } else if ( + ev.state === "running" || + ev.state === "thinking" || + ev.state === "attention" + ) { + setBusy(true); + } break; case "rename": nameEl.textContent = ev.name || ""; break; case "message_queued": - // Live-worker reuse path: send appended to the running - // worker's pending queue rather than spawning a new one. - // Surface as an info row so operators see queueing happen - // instead of dropping the event silently. - appendText( - "info", - "Queued (priority: " + (ev.priority || "normal") + ")", - { label: "queued" }, - ); + // Server confirms the queued slot — the optimistic bubble + // already showed it; nothing to render here. (Earlier this + // surfaced an extra info row, which doubled up with the + // queued bubble once the composer started rendering one.) + break; + case "busy_error": + // Worker is still alive after a cancel attempt; re-arm the + // Stop button so the user can try again (or escalate to + // force-stop after the 2s window). + appendText("error", ev.message || "Server is busy.", { + label: "error", + }); + if (busy) { + stopBtn.disabled = false; + stopBtn.textContent = "■ Stop"; + stopBtn.setAttribute("aria-label", "Stop generation"); + delete stopBtn.dataset.forceCancel; + } + break; + case "cancelled": + // Cancel was accepted; the worker may still be finishing + // (tool call in flight). Show "Cancelling…" and offer a + // Force Stop after 2s. state_change → idle is what actually + // clears busy; the 10s safety timer covers the connection-drop + // case. + if (!busy) break; + clearTimeout(cancelTimeoutId); + clearTimeout(forceTimeoutId); + stopBtn.disabled = true; + stopBtn.textContent = "Cancelling…"; + stopBtn.setAttribute("aria-label", "Cancelling generation"); + cancelTimeoutId = setTimeout(() => { + if (busy) { + stopBtn.disabled = false; + stopBtn.textContent = "⚠ Force Stop"; + stopBtn.setAttribute("aria-label", "Force stop generation"); + stopBtn.dataset.forceCancel = "true"; + } + }, 2000); + forceTimeoutId = setTimeout(() => { + if (busy) { + appendText( + "info", + "Cancel didn't complete in time. You may need to resend your last message.", + { label: "info" }, + ); + setBusy(false); + } + }, 10000); break; case "tool_info": // Renamed from ``tools_auto_approved`` when ``approve_tools`` @@ -2352,6 +2597,9 @@ // Load children + tasks in parallel — neither blocks SSE connection. loadChildren(); loadTasks(); + // Pull any in-flight attachment reservations (page reload / cross-tab + // switch) so the chips reappear instead of silently orphaning rows. + attachments.rehydrate(); connectSSE(); } diff --git a/turnstone/console/static/coordinator/index.html b/turnstone/console/static/coordinator/index.html index 609970a7..d63b9f77 100644 --- a/turnstone/console/static/coordinator/index.html +++ b/turnstone/console/static/coordinator/index.html @@ -585,7 +585,6 @@ connecting… - @@ -686,6 +685,8 @@ + + diff --git a/turnstone/shared_static/chat.css b/turnstone/shared_static/chat.css index 4420d203..15a41d39 100644 --- a/turnstone/shared_static/chat.css +++ b/turnstone/shared_static/chat.css @@ -451,6 +451,49 @@ color: #fff; } +/* ============================================================ + Queued-message bubble — optimistic UI when the user sends + while the worker is still mid-turn (queueWhileBusy=true). + The bubble dims, gains a dashed border, and carries a + "queued" badge + dismiss button until the shared queue + controller observes the busy → idle edge via + ``queue.onIdleEdge()`` and promotes it to a normal user + message. The "important" variant is the !!! priority + prefix path. + ============================================================ */ +.msg-queued { + opacity: 0.65; + border-style: dashed; +} +.msg-queued-important { + opacity: 0.8; + border-color: var(--yellow, var(--warn)); +} +.queued-badge { + font-size: 10px; + text-transform: uppercase; + letter-spacing: 0.05em; + color: var(--fg-dim, var(--ink-3)); + margin-right: 4px; +} +.msg-queued-important .queued-badge { + color: var(--yellow, var(--warn)); +} +.queued-dismiss { + background: none; + border: none; + color: var(--fg-dim, var(--ink-3)); + cursor: pointer; + font-size: 14px; + padding: 0 4px; + margin-left: 8px; + float: right; + line-height: 1; +} +.queued-dismiss:hover { + color: var(--red, var(--err)); +} + /* ============================================================ Stacked layout — textarea above, actions row below. Used by creation forms (coordinator-create, future dashboard diff --git a/turnstone/shared_static/composer_attachments.js b/turnstone/shared_static/composer_attachments.js new file mode 100644 index 00000000..e42eb104 --- /dev/null +++ b/turnstone/shared_static/composer_attachments.js @@ -0,0 +1,307 @@ +/* composer_attachments.js — shared paperclip / chip / upload pipeline. + * + * Used by both: + * - turnstone/ui/static/app.js (interactive Pane) + * - turnstone/console/static/coordinator/coordinator.js (coord IIFE) + * + * Owns: the in-flight `pendingAttachments` Map (insertion-ordered, so + * the send-time iteration order reflects user selection rather than + * upload-completion order) and the chips DOM container. Caller wires + * the controller's `upload()` into the Composer's `attachments.onAttach` + * callback. + * + * Server contract: POST /v1/api/workstreams/{ws_id}/attachments returns + * `{attachment_id, filename, size_bytes, mime_type, kind}`. DELETE + * /v1/api/workstreams/{ws_id}/attachments/{attachment_id} releases the + * pending row. GET returns `{attachments: [...]}` for rehydrate. + * + * Returned controller surface: + * upload(file) — POST a File; renders a placeholder chip + * that swaps to the real id on success. + * remove(attachmentId) — DELETE the chip + server-side row. + * clearChips() — drop all chips + map entries (no DELETE). + * rehydrate() — pull the server-side pending list (page + * reload / tab switch). + * snapshot() — {attachments, attachment_ids} of stable + * chips only (skips in-flight placeholders), + * ready to feed into a /send body. + * consume(attached_ids, + * dropped_ids?) — strip chips for ids the server reserved; + * surface a toast if any were dropped. + * isEmpty() — true when no chips are pending. + */ +(function (root) { + "use strict"; + + function formatSize(n) { + if (n < 1024) return n + " B"; + if (n < 1024 * 1024) return (n / 1024).toFixed(1) + " KB"; + return (n / (1024 * 1024)).toFixed(1) + " MB"; + } + + function _toastError(msg) { + if (typeof root.toast !== "undefined" && root.toast.error) { + root.toast.error(msg); + } + } + + /** + * @param {Object} opts + * chipsEl: HTMLElement — chips render target (composer.chipsEl). + * getWsId: () => string — current workstream id (function so the + * interactive pane can swap tabs without re-instantiating). + * authFetch: optional override (default window.authFetch). + * onError: optional (msg, err) => void — replaces the default toast + * for upload failures. + */ + function createAttachmentController(opts) { + if (!opts || !opts.chipsEl) + throw new Error("createAttachmentController: chipsEl required"); + if (typeof opts.getWsId !== "function") + throw new Error("createAttachmentController: getWsId must be a function"); + var chipsEl = opts.chipsEl; + var getWsId = opts.getWsId; + var onError = opts.onError || _toastError; + // Lazy authFetch lookup — shared/auth.js is loaded before this + // module in production, but being lazy avoids surprising + // construction-order failures and keeps the test stub (which + // defines window.authFetch later) working. + function _authFetch(url, init) { + var fn = opts.authFetch || root.authFetch; + return fn(url, init); + } + var pending = new Map(); + + function renderChip(info) { + var chip = document.createElement("span"); + chip.className = "composer-chip composer-chip-" + (info.kind || "other"); + chip.setAttribute("role", "listitem"); + chip.dataset.attachmentId = info.attachment_id; + + var icon = document.createElement("span"); + icon.className = "composer-chip-icon"; + icon.setAttribute("aria-hidden", "true"); + icon.textContent = info.kind === "image" ? "🖼" : "📄"; + chip.appendChild(icon); + + var name = document.createElement("span"); + name.className = "composer-chip-name"; + name.textContent = info.filename || "(unnamed)"; + name.title = info.filename || ""; + chip.appendChild(name); + + var size = document.createElement("span"); + size.className = "composer-chip-size"; + size.textContent = formatSize(info.size_bytes || 0); + chip.appendChild(size); + + var btn = document.createElement("button"); + btn.type = "button"; + btn.className = "composer-chip-remove"; + btn.setAttribute( + "aria-label", + "Remove attachment " + (info.filename || ""), + ); + btn.title = "Remove"; + btn.textContent = "×"; + btn.addEventListener("click", function () { + remove(info.attachment_id); + }); + chip.appendChild(btn); + + chipsEl.appendChild(chip); + return chip; + } + + function _findChip(id) { + return chipsEl.querySelector('[data-attachment-id="' + id + '"]'); + } + + function _removeChipDom(id) { + var chip = _findChip(id); + if (chip) chip.remove(); + } + + // Replace one Map key with another in place, preserving insertion + // order. JS Map iteration is insertion-ordered, so naïve + // `delete + set` would push the entry to the end of the order — + // breaking the contract that send() iterates chips in user- + // selection order. Localised here so callers (and the swap path + // below) don't restate the rationale. + function _replaceMapKey(map, oldKey, newKey, newVal) { + var rebuilt = new Map(); + map.forEach(function (val, key) { + if (key === oldKey) rebuilt.set(newKey, newVal); + else rebuilt.set(key, val); + }); + map.clear(); + rebuilt.forEach(function (val, key) { + map.set(key, val); + }); + } + + function _swapPlaceholder(placeholderId, info) { + // If the user removed the placeholder mid-upload (chip + map + // entry both gone), drop the response — resurrecting a chip the + // user dismissed would attach an untracked element (not in the + // map, so coordSend wouldn't include it) and confuse them. + if (!pending.has(placeholderId)) return; + _replaceMapKey(pending, placeholderId, info.attachment_id, info); + + var chip = _findChip(placeholderId); + if (chip) { + chip.dataset.attachmentId = info.attachment_id; + var name = chip.querySelector(".composer-chip-name"); + if (name) { + name.textContent = info.filename || "(unnamed)"; + name.title = info.filename || ""; + } + var size = chip.querySelector(".composer-chip-size"); + if (size) size.textContent = formatSize(info.size_bytes || 0); + } else { + renderChip(info); + } + } + + function upload(file) { + var wsId = getWsId(); + if (!wsId || !file) return; + var fd = new FormData(); + fd.append("file", file, file.name); + + var placeholderId = "__uploading_" + Date.now() + "_" + Math.random(); + var placeholder = { + attachment_id: placeholderId, + filename: file.name, + size_bytes: file.size, + mime_type: file.type || "", + kind: (file.type || "").indexOf("image/") === 0 ? "image" : "text", + uploading: true, + }; + pending.set(placeholderId, placeholder); + renderChip(placeholder); + + _authFetch( + "/v1/api/workstreams/" + encodeURIComponent(wsId) + "/attachments", + { method: "POST", credentials: "include", body: fd }, + ) + .then(function (r) { + return r.json().then(function (body) { + return { ok: r.ok, status: r.status, body: body }; + }); + }) + .then(function (res) { + if (!res.ok) { + _removeChipDom(placeholderId); + pending.delete(placeholderId); + onError((res.body && res.body.error) || "Upload failed"); + return; + } + _swapPlaceholder(placeholderId, res.body); + }) + .catch(function (e) { + _removeChipDom(placeholderId); + pending.delete(placeholderId); + if (e && e.message !== "auth") onError("Upload failed", e); + }); + } + + function remove(attachmentId) { + var info = pending.get(attachmentId); + if (!info) return; + _removeChipDom(attachmentId); + pending.delete(attachmentId); + if (info.uploading) return; // no server-side row yet + var wsId = getWsId(); + if (!wsId) return; + _authFetch( + "/v1/api/workstreams/" + + encodeURIComponent(wsId) + + "/attachments/" + + encodeURIComponent(attachmentId), + { method: "DELETE", credentials: "include" }, + ).catch(function () { + // Non-fatal — chip is gone client-side; the row will be + // garbage-collected by the attachment GC sweep. + }); + } + + function clearChips() { + pending.clear(); + chipsEl.textContent = ""; + } + + function rehydrate() { + var wsId = getWsId(); + if (!wsId) return Promise.resolve(); + return _authFetch( + "/v1/api/workstreams/" + encodeURIComponent(wsId) + "/attachments", + { method: "GET", credentials: "include" }, + ) + .then(function (r) { + return r.ok ? r.json() : null; + }) + .then(function (body) { + if (!body) return; + // Tab swap mid-fetch: a stale response would clobber the new + // tab's chips with the old tab's data. Re-check the live + // wsId before mutating any DOM. + if (getWsId() !== wsId) return; + clearChips(); + (body.attachments || []).forEach(function (a) { + pending.set(a.attachment_id, a); + renderChip(a); + }); + }) + .catch(function () { + /* non-fatal */ + }); + } + + function snapshot() { + var attachments = []; + var ids = []; + pending.forEach(function (info, id) { + if (info && !info.uploading) { + attachments.push(info); + ids.push(id); + } + }); + return { attachments: attachments, attachment_ids: ids }; + } + + function consume(attachedIds, droppedIds) { + if (Array.isArray(attachedIds)) { + attachedIds.forEach(function (id) { + _removeChipDom(id); + pending.delete(id); + }); + if (Array.isArray(droppedIds) && droppedIds.length) { + onError( + "Some attachments couldn’t be included (" + + droppedIds.length + + ") — they’re still in your composer.", + ); + } + } else { + clearChips(); + } + } + + function isEmpty() { + return pending.size === 0; + } + + return { + upload: upload, + remove: remove, + clearChips: clearChips, + rehydrate: rehydrate, + snapshot: snapshot, + consume: consume, + isEmpty: isEmpty, + }; + } + + root.createAttachmentController = createAttachmentController; +})(typeof window !== "undefined" ? window : globalThis); diff --git a/turnstone/shared_static/composer_queue.js b/turnstone/shared_static/composer_queue.js new file mode 100644 index 00000000..40544cf3 --- /dev/null +++ b/turnstone/shared_static/composer_queue.js @@ -0,0 +1,229 @@ +/* composer_queue.js — shared optimistic-queue UI for the chat composer. + * + * Used by both: + * - turnstone/ui/static/app.js (interactive Pane) + * - turnstone/console/static/coordinator/coordinator.js (coord IIFE) + * + * What this owns: + * - The queued-message bubble's DOM shape (msg-queued / queued-badge + * / queued-dismiss) and its dismiss-while-in-flight state machine. + * - The on-idle sweep that strips queued styling once the worker + * drains (caller invokes onIdleEdge() on the busy → idle edge). + * + * What this does NOT own: + * - Sending. Caller renders the bubble before the POST, then later + * calls bind(el, msgId) when the server's response carries the id, + * or remove(el) on a queue_full / busy reject path. + * - Busy state. The shape is "addQueuedMessage on send, promote on + * idle"; caller orchestrates both around its own busy flag. + * + * Caller options: + * messagesEl: HTMLElement — chat log container. + * getWsId: () => string — current ws id (function so the + * interactive pane can swap tabs without re-instantiating). + * wrapInBody: bool — when true (coord), wrap the queued content in a + * .msg-body div to match the surrounding .msg shape; when + * false (interactive), append children directly to the + * .msg element. Default false to match the historical + * interactive shape. + * authFetch: optional override (default window.authFetch). + * onAfterDequeue: optional () => void — interactive hooks attachment + * re-fetch here. Coord deliberately omits. + * onIdle: optional () => void — fires inside onIdleEdge() after + * the bubble sweep, so the consumer can run its own + * edge-only cleanup (e.g. clearing cancel/force-stop + * timers) without re-implementing edge detection. + * + * Returned controller surface: + * addQueuedMessage(text, priority) -> el + * priority: "important" | anything-else (treated as "notice") + * bind(el, msgId) + * Server returned a queued msg_id. Stamps msgId onto the bubble, + * or releases the slot server-side when the bubble can no longer + * be dequeued (user dismissed pre-bind, or the promote sweep + * raced ahead). Caller need only invoke. + * remove(el) + * Drop the bubble (busy / queue_full / connection-error path). + * onIdleEdge() + * Caller invokes once per busy → idle transition. Strips queued + * styling from every bubble and then fires the onIdle hook. + */ +(function (root) { + "use strict"; + + function createQueueController(opts) { + if (!opts || !opts.messagesEl) + throw new Error("createQueueController: messagesEl required"); + if (typeof opts.getWsId !== "function") + throw new Error("createQueueController: getWsId must be a function"); + var messagesEl = opts.messagesEl; + var getWsId = opts.getWsId; + var wrapInBody = !!opts.wrapInBody; + var onAfterDequeue = + typeof opts.onAfterDequeue === "function" ? opts.onAfterDequeue : null; + var onIdle = typeof opts.onIdle === "function" ? opts.onIdle : null; + // Lazy authFetch lookup — see composer_attachments.js for the + // rationale; same load-order robustness applies here. + function _authFetch(url, init) { + var fn = opts.authFetch || root.authFetch; + return fn(url, init); + } + + function _scrollIntoView() { + messagesEl.scrollTop = messagesEl.scrollHeight; + } + + function _deleteRequest(msgId) { + var wsId = getWsId(); + if (!wsId || !msgId) return null; + return _authFetch( + "/v1/api/workstreams/" + encodeURIComponent(wsId) + "/send", + { + method: "DELETE", + credentials: "include", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ msg_id: msgId }), + }, + ); + } + + // Fire-and-forget DELETE — used by bind() on a raced-away bubble + // where the caller has no DOM follow-up. Still invokes + // onAfterDequeue on success: the server-side reservation release + // freed any attachments the queued message held, and the caller + // typically rehydrates its chip pile so the user can reuse them. + function _sendDelete(msgId) { + var p = _deleteRequest(msgId); + if (!p) return; + p.then(function () { + if (onAfterDequeue) onAfterDequeue(); + }).catch(function () { + /* network error — promote loop strips queued styling on idle */ + }); + } + + function addQueuedMessage(text, priority) { + var el = document.createElement("div"); + el.className = "msg user msg-queued"; + el.setAttribute("role", "status"); + var important = priority === "important"; + if (important) { + el.classList.add("msg-queued-important"); + el.setAttribute("aria-label", "Important message queued: " + text); + } else { + el.setAttribute("aria-label", "Message queued: " + text); + } + + var badge = document.createElement("span"); + badge.className = "queued-badge"; + badge.setAttribute("aria-hidden", "true"); + badge.textContent = important ? "queued (!!!) " : "queued "; + + var textNode = document.createTextNode(text); + + var dismiss = document.createElement("button"); + dismiss.type = "button"; + dismiss.className = "queued-dismiss"; + dismiss.title = "Remove from queue"; + dismiss.setAttribute("aria-label", "Remove queued message"); + dismiss.textContent = "×"; + dismiss.addEventListener("click", function (e) { + e.stopPropagation(); + dequeue(el); + }); + + var host; + if (wrapInBody) { + host = document.createElement("div"); + host.className = "msg-body"; + el.appendChild(host); + } else { + host = el; + } + host.appendChild(badge); + host.appendChild(textNode); + host.appendChild(dismiss); + + messagesEl.appendChild(el); + _scrollIntoView(); + return el; + } + + // Dismiss flow: + // - msg_id known → DELETE /send, optimistically remove on success. + // - msg_id not yet bound → mark pendingDismiss; bind() picks it up + // when the send response arrives. + function dequeue(el) { + var msgId = el.dataset.msgId; + if (!msgId) { + el.dataset.pendingDismiss = "true"; + el.remove(); + return; + } + var p = _deleteRequest(msgId); + if (!p) return; + p.then(function (r) { + return r.json(); + }) + .then(function (data) { + if (data && data.status === "removed") el.remove(); + if (onAfterDequeue) onAfterDequeue(); + }) + .catch(function () { + /* network error — promote loop strips queued styling on idle */ + }); + } + + // Server returned status:queued + msg_id. Stamps msgId onto the + // bubble, OR releases the slot server-side when the bubble can no + // longer be dequeued from the UI (user dismissed pre-bind, or the + // promote sweep raced ahead and stripped .msg-queued / its dismiss + // button). Caller need only invoke; the controller handles all + // three races without further callbacks. + function bind(el, msgId) { + if (!el || !msgId) return; + var racedAway = + el.dataset.pendingDismiss || !el.classList.contains("msg-queued"); + if (racedAway) { + _sendDelete(msgId); + return; + } + el.dataset.msgId = msgId; + } + + function remove(el) { + if (el && el.parentNode) el.remove(); + } + + // Caller invokes onIdleEdge() exactly once per busy → idle + // transition. The controller strips queued styling from every + // bubble (so optimistic queues render as normal user messages + // once the worker has drained them) and then fires the optional + // onIdle hook so the consumer can run its own edge-only cleanup + // (e.g. clearing the cancel/force-stop timers) without each + // consumer re-implementing the same edge-detection logic. + function onIdleEdge() { + var queued = messagesEl.querySelectorAll(".msg-queued"); + queued.forEach(function (el) { + el.classList.remove("msg-queued", "msg-queued-important"); + delete el.dataset.msgId; + el.removeAttribute("role"); + el.removeAttribute("aria-label"); + var badge = el.querySelector(".queued-badge"); + if (badge) badge.remove(); + var dismiss = el.querySelector(".queued-dismiss"); + if (dismiss) dismiss.remove(); + }); + if (onIdle) onIdle(); + } + + return { + addQueuedMessage: addQueuedMessage, + bind: bind, + remove: remove, + onIdleEdge: onIdleEdge, + }; + } + + root.createQueueController = createQueueController; +})(typeof window !== "undefined" ? window : globalThis); diff --git a/turnstone/ui/static/app.js b/turnstone/ui/static/app.js index 4f1f1f82..68d17ce5 100644 --- a/turnstone/ui/static/app.js +++ b/turnstone/ui/static/app.js @@ -33,9 +33,6 @@ function Pane(wsId) { this._cancelTimeout = null; this._forceTimeout = null; this._pendingEditSend = null; - // Map - this.pendingAttachments = new Map(); - this.attachChipsEl = null; this._createDOM(); } @@ -175,7 +172,7 @@ Pane.prototype._createDOM = function () { this.composer = new Composer(this.el, { attachments: { onAttach: function (file) { - self.uploadAttachment(file); + self.attachments.upload(file); }, }, stopBtn: true, @@ -189,10 +186,43 @@ Pane.prototype._createDOM = function () { }, dragDrop: { targetEl: this.el, dropClass: "pane-drop-target" }, }); - this.attachChipsEl = this.composer.chipsEl; this.inputEl = this.composer.inputEl; this.sendBtn = this.composer.sendBtn; this.stopBtn = this.composer.stopBtn; + // Lazy wsId read \u2014 a tab swap (Pane re-bound to a new workstream) + // changes the closure target without re-instantiating the controllers. + this.attachments = createAttachmentController({ + chipsEl: this.composer.chipsEl, + getWsId: function () { + return self.wsId; + }, + onError: function (msg) { + showToast(msg); + }, + }); + this.queue = createQueueController({ + messagesEl: this.messagesEl, + getWsId: function () { + return self.wsId; + }, + onAfterDequeue: function () { + self.attachments.rehydrate(); + }, + // Idle-edge cleanup of the cancel/force-stop timers — without + // this they fire on the *next* busy turn, relabel Stop to "Force + // Stop", and surface a misleading "Cancel didn't complete in + // time" toast about a turn the user already moved past. + onIdle: function () { + if (self._cancelTimeout) { + clearTimeout(self._cancelTimeout); + self._cancelTimeout = null; + } + if (self._forceTimeout) { + clearTimeout(self._forceTimeout); + self._forceTimeout = null; + } + }, + }); }; Pane.prototype.reset = function () { @@ -204,206 +234,7 @@ Pane.prototype.reset = function () { this.approvalBlockEl = null; this._pendingEditSend = null; this.inputEl.disabled = false; - this.clearAttachmentChips(); -}; - -// --------------------------------------------------------------------------- -// Attachment handling -// --------------------------------------------------------------------------- - -Pane.prototype.clearAttachmentChips = function () { - if (this.pendingAttachments) this.pendingAttachments.clear(); - if (this.attachChipsEl) this.attachChipsEl.textContent = ""; -}; - -function _formatAttachSize(n) { - if (n < 1024) return n + " B"; - if (n < 1024 * 1024) return (n / 1024).toFixed(1) + " KB"; - return (n / (1024 * 1024)).toFixed(1) + " MB"; -} - -Pane.prototype._renderAttachmentChip = function (info) { - var self = this; - var chip = document.createElement("span"); - chip.className = "composer-chip composer-chip-" + (info.kind || "other"); - chip.setAttribute("role", "listitem"); - chip.dataset.attachmentId = info.attachment_id; - - var icon = document.createElement("span"); - icon.className = "composer-chip-icon"; - icon.setAttribute("aria-hidden", "true"); - icon.textContent = info.kind === "image" ? "\ud83d\uddbc" : "\ud83d\udcc4"; - chip.appendChild(icon); - - var label = document.createElement("span"); - label.className = "composer-chip-name"; - label.textContent = info.filename || "(unnamed)"; - label.title = info.filename || ""; - chip.appendChild(label); - - var size = document.createElement("span"); - size.className = "composer-chip-size"; - size.textContent = _formatAttachSize(info.size_bytes || 0); - chip.appendChild(size); - - var remove = document.createElement("button"); - remove.type = "button"; - remove.className = "composer-chip-remove"; - remove.setAttribute( - "aria-label", - "Remove attachment " + (info.filename || ""), - ); - remove.title = "Remove"; - remove.textContent = "\u00d7"; - remove.onclick = function () { - self.removeAttachment(info.attachment_id); - }; - chip.appendChild(remove); - - this.attachChipsEl.appendChild(chip); -}; - -Pane.prototype.uploadAttachment = function (file) { - if (!this.wsId || !file) return; - var self = this; - var wsId = this.wsId; - var fd = new FormData(); - fd.append("file", file, file.name); - - // Placeholder chip with upload-in-flight state - var placeholderId = "__uploading_" + Date.now() + "_" + Math.random(); - this.pendingAttachments.set(placeholderId, { - attachment_id: placeholderId, - filename: file.name, - size_bytes: file.size, - mime_type: file.type || "", - kind: (file.type || "").indexOf("image/") === 0 ? "image" : "text", - uploading: true, - }); - this._renderAttachmentChip(this.pendingAttachments.get(placeholderId)); - - authFetch( - "/v1/api/workstreams/" + encodeURIComponent(wsId) + "/attachments", - { method: "POST", body: fd }, - ) - .then(function (r) { - return r.json().then(function (body) { - return { ok: r.ok, status: r.status, body: body }; - }); - }) - .then(function (res) { - if (!res.ok) { - // Drop the placeholder; nothing to swap in. - self._removeAttachmentChip(placeholderId); - showToast((res.body && res.body.error) || "Upload failed"); - return; - } - // Swap placeholder → real id in place so chip and pending-Map - // ordering reflect user selection, not upload-completion order. - self._swapPlaceholderChip(placeholderId, res.body); - }) - .catch(function (e) { - // Always clean up the placeholder (including auth failures) so - // an "uploading…" chip can't get stuck across re-auth. - self._removeAttachmentChip(placeholderId); - if ((e && e.message) !== "auth") { - showToast("Upload failed"); - } - }); -}; - -Pane.prototype._removeAttachmentChip = function (id) { - var chip = this.attachChipsEl.querySelector( - '[data-attachment-id="' + id + '"]', - ); - if (chip) chip.remove(); - this.pendingAttachments.delete(id); -}; - -Pane.prototype._swapPlaceholderChip = function (placeholderId, info) { - // Rebuild pendingAttachments preserving insertion order, swapping - // the placeholder key for the real attachment_id. JS Map iteration - // is insertion-ordered, so naïve delete+set would move the entry to - // the end and reorder send(). - var rebuilt = new Map(); - this.pendingAttachments.forEach(function (val, key) { - if (key === placeholderId) { - rebuilt.set(info.attachment_id, info); - } else { - rebuilt.set(key, val); - } - }); - this.pendingAttachments = rebuilt; - - // Update the existing chip DOM in place so visual order matches. - var chip = this.attachChipsEl.querySelector( - '[data-attachment-id="' + placeholderId + '"]', - ); - if (chip) { - chip.dataset.attachmentId = info.attachment_id; - var name = chip.querySelector(".composer-chip-name"); - if (name) { - name.textContent = info.filename || "(unnamed)"; - name.title = info.filename || ""; - } - var size = chip.querySelector(".composer-chip-size"); - if (size) size.textContent = _formatAttachSize(info.size_bytes || 0); - } else { - // Chip missing (user removed it mid-upload?); render fresh. - this._renderAttachmentChip(info); - } -}; - -Pane.prototype.removeAttachment = function (attachmentId) { - var wsId = this.wsId; - var info = this.pendingAttachments.get(attachmentId); - if (!info) return; - var chip = this.attachChipsEl.querySelector( - '[data-attachment-id="' + attachmentId + '"]', - ); - - // Optimistic remove - if (chip) chip.remove(); - this.pendingAttachments.delete(attachmentId); - - // In-flight placeholders have no server-side row yet - if (info.uploading) return; - - authFetch( - "/v1/api/workstreams/" + - encodeURIComponent(wsId) + - "/attachments/" + - encodeURIComponent(attachmentId), - { method: "DELETE" }, - ).catch(function (e) { - if ((e && e.message) !== "auth") { - showToast("Failed to remove attachment"); - } - }); -}; - -Pane.prototype.rehydrateAttachments = function () { - if (!this.wsId) return; - var self = this; - var wsId = this.wsId; - authFetch( - "/v1/api/workstreams/" + encodeURIComponent(wsId) + "/attachments", - { method: "GET" }, - ) - .then(function (r) { - if (!r.ok) return null; - return r.json(); - }) - .then(function (body) { - // Tab may have switched between fire and response - if (!body || self.wsId !== wsId) return; - self.clearAttachmentChips(); - (body.attachments || []).forEach(function (a) { - self.pendingAttachments.set(a.attachment_id, a); - self._renderAttachmentChip(a); - }); - }) - .catch(function () {}); + this.attachments.clearChips(); }; Pane.prototype.updateWsName = function () { @@ -431,30 +262,19 @@ Pane.prototype.disconnectSSE = function () { } }; +// composer.setBusy runs unconditionally so the Stop button label / +// dataset.forceCancel / placeholder stay canonical even on a redundant +// call (Pane.reset() and any future caller relies on that idempotent +// reset). queue.onIdleEdge runs only on the actual edge — it carries +// the heavier work (querySelectorAll-driven promote sweep + cancel- +// timer cleanup wired via the queue's onIdle hook). Pane.prototype.setBusy = function (b) { - this.busy = b; - this.messagesEl.dataset.busy = b ? "true" : "false"; - // Composer owns send/stop button display, label rotation, and the - // stop button's "■ Stop" / aria-label / dataset reset on every - // transition (so cancelGeneration's transient "Cancelling…" label - // doesn't persist into the next busy cycle). - this.composer.setBusy(b); - if (!b) this._promoteQueuedMessages(); -}; - -Pane.prototype._promoteQueuedMessages = function () { - var queuedMsgs = this.messagesEl.querySelectorAll(".msg-queued"); - for (var i = 0; i < queuedMsgs.length; i++) { - var el = queuedMsgs[i]; - el.classList.remove("msg-queued", "msg-queued-important"); - delete el.dataset.msgId; - el.removeAttribute("role"); - el.removeAttribute("aria-label"); - var badge = el.querySelector(".queued-badge"); - if (badge) badge.remove(); - var dismiss = el.querySelector(".queued-dismiss"); - if (dismiss) dismiss.remove(); - } + var next = !!b; + this.composer.setBusy(next); + this.messagesEl.dataset.busy = next ? "true" : "false"; + var edge = next !== this.busy; + this.busy = next; + if (edge && !next) this.queue.onIdleEdge(); }; Pane.prototype.showEmptyState = function () { @@ -477,8 +297,8 @@ Pane.prototype.connectSSE = function (wsId) { var wsChanged = this.wsId !== wsId; this.wsId = wsId; if (wsChanged) { - this.clearAttachmentChips(); - this.rehydrateAttachments(); + this.attachments.clearChips(); + this.attachments.rehydrate(); } this.evtSource = new EventSource( @@ -883,74 +703,6 @@ Pane.prototype.addUserMessage = function (text, attachments) { this.scrollToBottom(true); }; -Pane.prototype.addQueuedMessage = function (text, priority) { - this.removeEmptyState(); - var self = this; - var el = document.createElement("div"); - el.className = "msg user msg-queued"; - el.setAttribute("role", "status"); - if (priority === "important") { - el.classList.add("msg-queued-important"); - el.setAttribute("aria-label", "Important message queued: " + text); - } else { - el.setAttribute("aria-label", "Message queued: " + text); - } - var badge = document.createElement("span"); - badge.className = "queued-badge"; - badge.setAttribute("aria-hidden", "true"); - badge.textContent = priority === "important" ? "queued (!!!) " : "queued "; - el.appendChild(badge); - el.appendChild(document.createTextNode(text)); - // Dismiss button — remove from queue before injection - var dismiss = document.createElement("button"); - dismiss.className = "queued-dismiss"; - dismiss.title = "Remove from queue"; - dismiss.setAttribute("aria-label", "Remove queued message"); - dismiss.textContent = "\u00d7"; - dismiss.addEventListener("click", function (e) { - e.stopPropagation(); - self._dequeueMessage(el); - }); - el.appendChild(dismiss); - this.messagesEl.appendChild(el); - this.scrollToBottom(true); - return el; -}; - -Pane.prototype._dequeueMessage = function (el) { - var self = this; - var msgId = el.dataset.msgId; - if (!msgId) { - // ID not yet set — mark for deferred DELETE when send response arrives - el.dataset.pendingDismiss = "true"; - el.remove(); - return; - } - authFetch("/v1/api/workstreams/" + encodeURIComponent(this.wsId) + "/send", { - method: "DELETE", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ msg_id: msgId }), - }) - .then(function (r) { - return r.json(); - }) - .then(function (data) { - if (data.status === "removed") { - el.remove(); - } - // Either path warrants a chip-strip refresh: a successful remove - // unreserves the attachments server-side, and a "not_found" means - // dispatch raced us so the actual pending state may differ from - // what the UI last saw. Re-fetch to stay in sync. (Leave the - // message bubble visible on not_found — the promote loop strips - // the queued styling on idle.) - self.rehydrateAttachments(); - }) - .catch(function () { - // Network error — don't remove, message may have been injected - }); -}; - Pane.prototype._addUserMsgActions = function (el, text) { var self = this; var bar = document.createElement("div"); @@ -1803,31 +1555,22 @@ Pane.prototype.sendMessage = function () { var self = this; var isBusy = this.busy; var queuedEl = null; - - // Snapshot attachments for this turn (stable-ids only — skip in-flight - // placeholders, which may not have server-assigned ids yet). - var attachmentList = []; - var attachmentIds = []; - this.pendingAttachments.forEach(function (info, id) { - if (info && !info.uploading) { - attachmentList.push(info); - attachmentIds.push(id); - } - }); + var snap = this.attachments.snapshot(); if (isBusy) { - // Queue message for injection at the next tool-result seam. - // Strip !!! prefix for display, show priority badge instead. + // Server re-parses the !!! prefix to set queue priority — the + // optimistic bubble strips it for display. var displayText = text; var priority = "notice"; if (text.startsWith("!!!")) { displayText = text.slice(3).trimStart(); priority = "important"; } - queuedEl = this.addQueuedMessage(displayText, priority); + this.removeEmptyState(); + queuedEl = this.queue.addQueuedMessage(displayText, priority); } else { this.setBusy(true); - this.addUserMessage(text, attachmentList); + this.addUserMessage(text, snap.attachments); } this.composer.clear(); @@ -1836,71 +1579,44 @@ Pane.prototype.sendMessage = function () { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ message: text, - attachment_ids: attachmentIds, + attachment_ids: snap.attachment_ids, }), }) .then(function (r) { return r.json(); }) .then(function (data) { - // Clear only the chips that were actually reserved/attached on - // the server; leftover (dropped) ones stay in the composer so - // the user can see what's still pending. - var _consumeChips = function () { - var attached = Array.isArray(data.attached_ids) - ? data.attached_ids - : null; - if (attached) { - attached.forEach(function (id) { - var chip = self.attachChipsEl.querySelector( - '[data-attachment-id="' + id + '"]', - ); - if (chip) chip.remove(); - self.pendingAttachments.delete(id); - }); - if ( - Array.isArray(data.dropped_attachment_ids) && - data.dropped_attachment_ids.length - ) { - showToast( - "Some attachments couldn't be included (" + - data.dropped_attachment_ids.length + - ") — they're still in your composer.", - ); - } - } else { - self.clearAttachmentChips(); - } - }; - - if (data.status === "queued" && data.msg_id && queuedEl) { - if (queuedEl.dataset.pendingDismiss) { - // User dismissed before ID arrived — send deferred DELETE - authFetch( - "/v1/api/workstreams/" + encodeURIComponent(self.wsId) + "/send", - { - method: "DELETE", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ msg_id: data.msg_id }), - }, - ); - } else { - queuedEl.dataset.msgId = data.msg_id; - } - _consumeChips(); + if (data.status === "queued" && data.msg_id) { + // queuedEl-present path: bind() handles the three known races + // (pre-bind dismiss, promote sweep raced ahead, normal accept). + // queuedEl-absent path: client thought it was idle but the + // server saw a live worker (SSE state_change hadn't arrived + // yet). Flip busy so subsequent sends queue correctly; the + // optimistic user bubble is already in the log and the server + // still delivers the message on worker drain — accept the + // small UX gap (no in-UI dismiss for THIS message). + if (queuedEl) self.queue.bind(queuedEl, data.msg_id); + else self.setBusy(true); + self.attachments.consume( + data.attached_ids, + data.dropped_attachment_ids, + ); } else if (data.status === "busy") { - if (queuedEl) queuedEl.remove(); + if (queuedEl) self.queue.remove(queuedEl); self.addErrorMessage("Server is busy. Please wait."); if (!isBusy) self.setBusy(false); } else if (data.status === "queue_full") { - if (queuedEl) queuedEl.remove(); + if (queuedEl) self.queue.remove(queuedEl); self.addErrorMessage("Message queue full. Please wait."); } else { - _consumeChips(); + self.attachments.consume( + data.attached_ids, + data.dropped_attachment_ids, + ); } }) .catch(function (err) { - if (queuedEl) queuedEl.remove(); + if (queuedEl) self.queue.remove(queuedEl); self.addErrorMessage("Connection error: " + err.message); if (!isBusy) self.setBusy(false); }); diff --git a/turnstone/ui/static/index.html b/turnstone/ui/static/index.html index 92a51c77..381762fe 100644 --- a/turnstone/ui/static/index.html +++ b/turnstone/ui/static/index.html @@ -542,6 +542,8 @@ + + diff --git a/turnstone/ui/static/style.css b/turnstone/ui/static/style.css index d57d196a..ad4ad0e3 100644 --- a/turnstone/ui/static/style.css +++ b/turnstone/ui/static/style.css @@ -642,38 +642,6 @@ .msg.user { color: var(--fg-bright); } -.msg-queued { - opacity: 0.65; - border-style: dashed; -} -.msg-queued-important { - opacity: 0.8; - border-color: var(--yellow); -} -.queued-badge { - font-size: 10px; - text-transform: uppercase; - letter-spacing: 0.05em; - color: var(--fg-dim); - margin-right: 4px; -} -.msg-queued-important .queued-badge { - color: var(--yellow); -} -.queued-dismiss { - background: none; - border: none; - color: var(--fg-dim); - cursor: pointer; - font-size: 14px; - padding: 0 4px; - margin-left: 8px; - float: right; - line-height: 1; -} -.queued-dismiss:hover { - color: var(--red); -} /* .msg.assistant / .msg.info / .msg.error alignment + baseline visuals come from shared_static/chat.css. Interactive UI adds a pre-wrap override for info messages and a tightened tool-message shape with