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