From 30cb9e60973f68afb7cf548fc3eea3a7b227bc68 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Sun, 7 Jun 2026 14:21:59 -0700 Subject: [PATCH] =?UTF-8?q?fix(ui):=20L-shell=20step=207=20=E2=80=94=20wir?= =?UTF-8?q?e=20coordinator=20approval=20keyboard=20shortcuts=20(designer?= =?UTF-8?q?=20P2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The console twin of the interactive.js approval-key fix: the coordinator's tool-batch card shows kbd hints (Enter approve / D deny / Shift+A approve-all) but they did nothing — the keys were never wired (the standalone routed approval keys through the app.js global keydown + getFocusedPane, retired in the fork collapse). Add a pane-owned keydown on `root` that resolves the current pending batch: - _currentPendingBatch() finds the last .conv-batch with a still-pending [data-needs-approval="1"] row whose actions aren't already disabled — the in-flight double-fire guard (a second key during the resolve is a no-op). - Enter -> approve, D/Esc -> deny, Shift+A -> approve-all, routed to the existing _resolveBatchAction path. - A focus guard skips when an input/textarea/contenteditable is focused, so the keys never hijack composer typing (the coordinator has no feedback field, unlike interactive, so no feedback special-case). Verified end-to-end against the real coord pane (keydown -> _currentPendingBatch -> _resolveBatchAction -> approveWorkstream -> postJSON -> authFetch, stubbed at the HTTP boundary): Enter/D/Shift+A fire the right verb, the double-fire + focus guards hold, errs:[] + a coordinator JS guard. Live keypress confirm rides the merge gate. --- tests/test_coordinator_page.py | 29 ++++++++++++ .../console/static/coordinator/coordinator.js | 45 +++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/tests/test_coordinator_page.py b/tests/test_coordinator_page.py index d045ecbc..74511b20 100644 --- a/tests/test_coordinator_page.py +++ b/tests/test_coordinator_page.py @@ -109,6 +109,35 @@ def test_coordinator_js_exposes_inline_approval_helpers(): # a 'critical' verdict still ranks like 'crit'. shared = Path(__file__).resolve().parent.parent / ("turnstone/shared_static/conversation.js") assert 'crit: "critical"' in shared.read_text(encoding="utf-8") + + +def test_coordinator_js_approval_keyboard_shortcuts(): + """Step 7 designer P2 (the console twin of the interactive.js fix): a pending + tool-batch's kbd hints (Enter approve / D deny / Shift+A approve-all) must + actually fire. Pane-owned keydown on `root` routing to _resolveBatchAction + via _currentPendingBatch (the last un-resolved pending batch), with a focus + guard (don't hijack composer typing) and the disabled-button double-fire guard. + Asserts string presence only (no JS framework for coord.js).""" + from pathlib import Path + + body = ( + Path(__file__).resolve().parent.parent + / "turnstone/console/static/coordinator/coordinator.js" + ).read_text(encoding="utf-8") + assert 'root.addEventListener("keydown"' in body, ( + "the approval shortcuts must be a pane-owned keydown on root" + ) + assert "function _currentPendingBatch()" in body, ( + "the keydown must resolve the current pending batch (not a stale/resolved one)" + ) + # The double-fire guard: skip a batch whose actions are already disabled. + assert "btn.disabled) continue" in body + # Routes the three verbs to the existing resolve path. + assert "_resolveBatchAction(batch, true, false)" in body # Enter -> approve + assert "_resolveBatchAction(batch, false, false)" in body # D/Esc -> deny + assert "_resolveBatchAction(batch, true, true)" in body # Shift+A -> approve-all + # Focus guard so the keys never hijack composer/input typing. + assert 'ae.tagName === "TEXTAREA"' in body and "ae.isContentEditable" in body # Child approves must round-trip through the routing proxy at # /v1/api/route/workstreams/{ws_id}/approve — the bare # /v1/api/workstreams/.../approve path only works for the diff --git a/turnstone/console/static/coordinator/coordinator.js b/turnstone/console/static/coordinator/coordinator.js index e42c4ca7..832bde5e 100644 --- a/turnstone/console/static/coordinator/coordinator.js +++ b/turnstone/console/static/coordinator/coordinator.js @@ -317,6 +317,35 @@ function createCoordinatorPane(root, wsId, opts) { pm.openPane("interactive", childWs, { nodeId: childNode }); } }); + // Approval keyboard shortcuts (designer P2 — the console twin of the + // interactive.js fix): when a tool-batch is awaiting approval, route the + // card's kbd hints (Enter approve / D deny / Shift+A approve-all) to the + // resolve path. Pane-owned on `root`, no global handler. A focus guard lets + // the composer / any input keep its own keys; _currentPendingBatch skips a + // batch whose actions are already disabled (the in-flight double-fire guard). + // No feedback field here (unlike interactive), so no feedback special-case. + root.addEventListener("keydown", function (e) { + const ae = document.activeElement; + if ( + ae && + (ae.tagName === "TEXTAREA" || + ae.tagName === "INPUT" || + ae.isContentEditable) + ) + return; + const batch = _currentPendingBatch(); + if (!batch) return; + if (e.key === "Enter") { + e.preventDefault(); + _resolveBatchAction(batch, true, false); + } else if (e.key === "Escape" || e.key.toLowerCase() === "d") { + e.preventDefault(); + _resolveBatchAction(batch, false, false); + } else if (e.shiftKey && e.key.toLowerCase() === "a") { + e.preventDefault(); + _resolveBatchAction(batch, true, true); + } + }); // Off-screen aria-live="assertive" region — pending tool-batches // append into the polite messages log, which gets flipped to // aria-live="off" during streaming. Routing the action-required @@ -1249,6 +1278,22 @@ function createCoordinatorPane(root, wsId, opts) { }); } + // The tool-batch currently awaiting an approval decision (for the keyboard + // shortcuts): the last .conv-batch with a still-pending row whose actions + // aren't already disabled — i.e. not mid-resolve, so a key never double-fires. + function _currentPendingBatch() { + const batches = messagesEl.querySelectorAll(".conv-batch"); + for (let i = batches.length - 1; i >= 0; i--) { + const b = batches[i]; + if (!b.querySelector('.conv-row[data-needs-approval="1"][data-call-id]')) + continue; + const btn = b.querySelector(".conv-actions button"); + if (btn && btn.disabled) continue; // mid-resolve — don't double-fire + return b; + } + return null; + } + // Build the resolved-state status pill. Shared between the live // _morphBatchResolved path (post-approve, post-deny) and the // history-replay path inside appendToolBatch (renders resolved