mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-27 22:34:51 -06:00
fix(ui): L-shell step 7 — wire coordinator approval keyboard shortcuts (designer P2)
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user