diff --git a/orchestrator/templates/page-inbox.js b/orchestrator/templates/page-inbox.js index a4615a5..40a2f6e 100644 --- a/orchestrator/templates/page-inbox.js +++ b/orchestrator/templates/page-inbox.js @@ -65,24 +65,30 @@ const rejectOne = (item) => submit([entryOf(item, "reject", "")], `Denial: ${short(item.title)}`); const chooseOne = (item, label) => submit([entryOf(item, "approve", label)], `Choice: ${label}`); - function bulk(items, label) { - const usable = items.filter(selectable); + const denialOf = (item) => (item.deniable ? {...entryOf(item, "reject", ""), channel: "post", reason: "", to: item.to} : item.kind === "review" ? entryOf(item, "reject", "") : null); + const pickable = (item) => !item.held && (selectable(item) || denialOf(item) !== null); + + function bulk(items, label, deny) { + const usable = items.filter(deny ? (item) => denialOf(item) !== null : selectable); if (!usable.length) { - toast(`${label}: nothing to grant`, [], false); + toast(`${label}: nothing to ${deny ? "deny" : "grant"}`, [], false); return; } const rows = usable.map((item) => h("li", {}, - h("div", {class: "row"}, pill(ITEM_LABELS[item.kind], KIND_TONES[item.kind]), item.kind === "decision" ? h("span", {class: "muted"}, `→ ${item.recommended}`) : null), + h("div", {class: "row"}, pill(ITEM_LABELS[item.kind], KIND_TONES[item.kind]), item.kind === "decision" && !deny ? h("span", {class: "muted"}, `→ ${item.recommended}`) : null), h("strong", {}, item.title), item.kind === "operator" ? h("div", {class: "command"}, h("code", {}, item.command)) : null, item.kind === "operator" ? h("span", {class: "muted"}, `in ${item.directory}`) : null, item.kind === "ask" ? h("div", {class: "command"}, h("code", {}, item.command)) : null)); - const confirm = h("button", {type: "button", class: "btn primary"}, `Grant ${usable.length}`); + const confirm = h("button", {type: "button", class: deny ? "btn danger" : "btn primary"}, `${deny ? "Deny" : "Grant"} ${usable.length}`); confirm.addEventListener("click", () => { closeSheet(); - submit(usable.map((item) => entryOf(item, "approve", item.kind === "decision" ? item.recommended : "")), `${label}: ${usable.length} entries`); + submit(usable.map((item) => (deny ? denialOf(item) : entryOf(item, "approve", item.kind === "decision" ? item.recommended : ""))), `${label}: ${usable.length} entries`); }); - openSheet(label, [h("p", {class: "muted"}, "Every entry and every full command that gets granted. After the click the passkey signs the whole selection once; then 3 s remain to undo."), h("ul", {class: "mini"}, rows), h("div", {class: "actions"}, h("span", {class: "spacer"}), btn("Cancel", "btn ghost", closeSheet), confirm)], false); + const note = deny + ? "Every entry and every full command that gets denied; each requester gets its denial by post. After the click the passkey signs the whole selection once; then 3 s remain to undo." + : "Every entry and every full command that gets granted. After the click the passkey signs the whole selection once; then 3 s remain to undo."; + openSheet(label, [h("p", {class: "muted"}, note), h("ul", {class: "mini"}, rows), h("div", {class: "actions"}, h("span", {class: "spacer"}), btn("Cancel", "btn ghost", closeSheet), confirm)], false); confirm.focus(); } @@ -101,8 +107,8 @@ return date.getTime() / 1000; } - function toggle(item) { - if (!item || !selectable(item)) { + function toggle(item, refocus) { + if (!item || !pickable(item)) { return; } const key = keyOf(item); @@ -111,7 +117,12 @@ } else { ui.selected.add(key); } + ui.cursor = Math.max(visibleInbox().findIndex((entry) => keyOf(entry) === key), 0); drawInbox(); + const box = inboxDom.querySelector(`.item[data-index="${ui.cursor}"] > input[type="checkbox"]`); + if (refocus && box) { + box.focus({preventScroll: true}); + } } function actionsFor(item) { @@ -142,7 +153,7 @@ function inboxItem(item, index) { const red = item.kind === "review" && item.gate !== "pass"; const classes = ["item", isGreen(item) ? "green" : "", red ? "red" : "", item.held ? "held" : "", index === ui.cursor ? "cursor" : ""].filter(Boolean).join(" "); - const box = h("input", {type: "checkbox", "aria-label": `Select ${item.title}`, checked: ui.selected.has(keyOf(item)), disabled: !selectable(item), on: {change: () => toggle(item)}}); + const box = h("input", {type: "checkbox", "aria-label": `Select ${item.title}`, checked: ui.selected.has(keyOf(item)), disabled: !pickable(item), on: {change: () => toggle(item, true)}}); return h("li", {class: classes, "data-index": index}, box, h("div", {}, @@ -204,12 +215,15 @@ ui.snoozed = event.target.checked; drawInbox(); }}}); - const choose = h("button", {type: "button", class: "btn small soft", ...named(`Grant selection (${chosen.length})`), disabled: !chosen.length || ui.busy, on: {click: () => bulk(chosen, "Grant selection")}}, icon("bi-check2-square"), String(chosen.length)); - const all = h("button", {type: "button", class: "btn small primary", ...named(`Grant all green (${green.length})`), disabled: !green.length || ui.busy, on: {click: () => bulk(green, "Grant all green")}}, icon("bi-check2-all"), String(green.length)); + const deniable = chosen.filter((item) => denialOf(item) !== null); + const grantable = chosen.filter(selectable); + const choose = h("button", {type: "button", class: "btn small soft", ...named(`Grant selection (${grantable.length})`), disabled: !grantable.length || ui.busy, on: {click: () => bulk(chosen, "Grant selection", false)}}, icon("bi-check2-square"), String(grantable.length)); + const deny = h("button", {type: "button", class: "btn small danger", ...named(`Deny selection (${deniable.length})`, `Deny selection (${deniable.length}): each requester gets its denial by post, one passkey signature, 3 s to undo`), disabled: !deniable.length || ui.busy, on: {click: () => bulk(chosen, "Deny selection", true)}}, icon("bi-x-square"), String(deniable.length)); + const all = h("button", {type: "button", class: "btn small primary", ...named(`Grant all green (${green.length})`), disabled: !green.length || ui.busy, on: {click: () => bulk(green, "Grant all green", false)}}, icon("bi-check2-all"), String(green.length)); inboxDom.replaceChildren( h("div", {class: "inbox-head"}, h("h2", {class: "group-title"}, "Inbox", counter(waitingCount())), h("span", {class: "spacer"}), hidden ? h("label", {class: "check"}, snoozed, `${hidden} snoozed`) : null), - h("div", {class: "inbox-actions"}, choose, all), + h("div", {class: "inbox-actions"}, choose, deny, all), onLocalhost() ? "" : h("p", {class: "muted"}, "The passkey signs grants only under ", localLink()), items.length ? h("ol", {class: "inbox-list"}, items.map(inboxItem)) : emptyState("All done", "Nothing is waiting for you right now.")); settleConsoles(items); diff --git a/orchestrator/templates/page-keyboard.js b/orchestrator/templates/page-keyboard.js index 5eb6d69..3269825 100644 --- a/orchestrator/templates/page-keyboard.js +++ b/orchestrator/templates/page-keyboard.js @@ -62,7 +62,7 @@ moveCursor(event.key === "j" ? 1 : -1); } else if (inInbox && event.key === "x") { event.preventDefault(); - toggle(current); + toggle(current, false); } else if (inInbox && event.key === "a") { event.preventDefault(); if (current && !current.held) { diff --git a/tests/e2e/test_orchestrator_e2e.py b/tests/e2e/test_orchestrator_e2e.py index 6889183..d8fb827 100644 --- a/tests/e2e/test_orchestrator_e2e.py +++ b/tests/e2e/test_orchestrator_e2e.py @@ -26,7 +26,7 @@ sys.path.insert(0, str(HERE.parent)) from hub_case import HubCase from hub_engines import wav_bytes from hub_support import ASK, HOST_STATE, OPTIONS, RUNNER, iso, verifier -from kvb_orchestrator import skill_scripts +from kvb_orchestrator import batches, skill_scripts from test_hub_cleanup import DONE_TIP, FAKE_PKGMGR, host from test_hub_offices import COUNCIL_SCRIPT from test_hub_offices import host as office_host @@ -909,6 +909,48 @@ RUN_DONE = """(async () => {@HELPERS@@LABELS@ const lines = out().textContent.trim().split("\\n"); return {done: line(), last: lines[lines.length - 1], lines: lines.length, followed: atEnd(), actions: all(".item-actions button", out().closest("li")).map(name)}; })()""".replace("@HELPERS@", HELPERS).replace("@LABELS@", LABELS) +INBOX_PICK = """(async () => {@HELPERS@ + navigator.credentials.get = @SIGNS@ ? navigator.credentials.get.bind(navigator.credentials) : () => Promise.reject(new DOMException("The operator canceled.", "NotAllowedError")); + const posted = []; + const send = window.fetch.bind(window); + window.fetch = (path, init) => { + if (["/api/sign", "/api/inbox/approve"].includes(String(path))) { + posted.push([String(path), JSON.parse(init.body).items.map((item) => [item.id, item.verb, item.channel || null])]); + } + return send(path, init); + }; + const list = await until(() => document.querySelector("#main .inbox-list"), 8000); + if (!list) { + return {error: text(document.getElementById("main"))}; + } + let scroller = list; + while (scroller && !(scroller.scrollHeight > scroller.clientHeight + 4 && getComputedStyle(scroller).overflowY !== "visible")) { + scroller = scroller.parentElement; + } + scroller = scroller || document.scrollingElement; + const last = all("#main .inbox-list > li").filter((li) => li.textContent.includes("echo row")).pop(); + last.scrollIntoView({block: "center"}); + await sleep(300); + const title = text(last.querySelector(".linkish")); + const before = scroller.scrollTop; + last.querySelector("input").click(); + await sleep(300); + const after = scroller.scrollTop; + const holder = document.activeElement && document.activeElement.closest("li"); + const focused = Boolean(holder && text(holder.querySelector(".linkish")) === title && document.activeElement.checked); + all("#main .inbox-list > li").find((li) => li.textContent.includes("pick a name")).querySelector("input").click(); + await sleep(300); + const buttons = all("#main .inbox-actions button").map(name); + all("#main .inbox-actions button").find((button) => name(button).startsWith("Deny selection")).click(); + await until(() => document.querySelector(".sheet-box .actions"), 4000); + const listed = all(".sheet-box ul.mini > li strong").map(text); + const confirm = all(".sheet-box .actions button").find((button) => text(button).startsWith("Deny ")); + const label = text(confirm); + const shown = new Set(all("#toasts .toast")); + confirm.click(); + const toast = await until(() => all("#toasts .toast").filter((node) => !shown.has(node)).map(text).find((entry) => entry.startsWith("Deny selection")), 15000); + return {title, before, moved: Math.abs(after - before), focused, buttons, listed, label, toast, posted, wide: document.documentElement.scrollWidth > window.innerWidth + 1}; +})()""".replace("@HELPERS@", HELPERS) LAYOUT = """(async () => {@HELPERS@@LABELS@ await until(() => document.querySelector("#main .console") && rowOf("Rule request"), 8000); const rule = rowOf("Rule request"); @@ -2099,6 +2141,46 @@ class TestOrchestratorE2E(HubCase): self.assertEqual(probe["console"], "doneExit code 0 after 9 s") self.assertTrue(shot.is_file() and shot.stat().st_size > 1000, shot) + def check_inbox_picks(self, browser: str, driver: str, binary: str, signs: bool) -> None: + """Args: + signs: as in check_night_menu. + """ + fill = {"description": "fill", "workdir": "/repo", "status": "queued", "roles": {"Operator": "R", "Skills session": "A"}} + rows = {f"operator/row-{n:02}": {**fill, "title": f"echo row {n:02}", "check": "manual", "seq": n} for n in range(1, 15)} + rows["operator/name"] = {**fill, "title": "pick a name", "check": "decision", "seq": 15} + self.write_council({"rasci.json": rows}) + probe = INBOX_PICK.replace("@SIGNS@", "true" if signs else "false") + steps = [{"width": width, "height": height, "url": f"http://localhost:{self.port}/?{width}#stream/inbox", "wait": 1200, "probe": probe, "shot": str(OUT / f"{browser}-{width}-inbox-deny-selection.png")} for width, height in WIDTHS] + stops = [] + with mock.patch.object(batches, "UNDO", 0.3): + data = self.drive(steps, self.authenticator() if signs else None, driver, binary) + if signs: + self.settle(lambda: len([call for call in self.scripts.calls if call[0] == skill_scripts.RASCI]) >= 2 * len(WIDTHS)) + stops = [args for script, args in self.scripts.calls if script == skill_scripts.RASCI] + (OUT / f"{browser}-inbox-deny-selection.json").write_text(json.dumps(data, indent=1), encoding="utf-8") + self.assertEqual([error for error in data["errors"] if "(style-src-elem)" not in error and "/favicon.ico" not in error], []) + denied = [] + for found, (width, _) in zip(data["results"], WIDTHS, strict=True): + with self.subTest(browser=browser, width=width): + self.assertNotIn("error", found) + self.assertGreater(found["before"], 0, found) + self.assertEqual((found["moved"], found["focused"], found["wide"]), (0, True, False), found) + self.assertTrue({"Grant selection (1)", "Deny selection (2)"} <= set(found["buttons"]), found["buttons"]) + self.assertEqual((sorted(found["listed"]), found["label"]), (sorted([found["title"], "pick a name"]), "Deny 2")) + task = f"operator/row-{found['title'][-2:]}" + items = sorted([[task, "reject", "post"], ["operator/name", "reject", "post"]]) + self.assertEqual([[path, sorted(entries)] for path, entries in found["posted"]], [["/api/sign", items], *([["/api/inbox/approve", items]] if signs else [])]) + self.assertIn("takes effect" if signs else "not signed", found["toast"]) + denied += [task, "operator/name"] + self.assertEqual(sorted(stops), sorted([["set", task, "--status", "stopped"] for task in denied]) if signs else []) + + def test_chromium_keeps_the_inbox_scroll_on_a_tick_and_denies_a_selection_with_one_signature_at_three_widths(self): + self.check_inbox_picks("chromium", "cdp.mjs", os.environ.get("CHROMIUM", "chromium"), signs=True) + + @unittest.skipUnless(shutil.which(os.environ.get("FIREFOX", "firefox")), "no Firefox") + def test_firefox_keeps_the_inbox_scroll_on_a_tick_and_denies_nothing_unsigned_at_three_widths(self): + self.check_inbox_picks("firefox", "bidi.mjs", os.environ.get("FIREFOX", "firefox"), signs=False) + def choice_steps(self, browser: str, signs: bool, questions: dict[int, str]) -> list[dict]: flag = "true" if signs else "false" local = f"http://localhost:{self.port}/" diff --git a/tests/test_hub_deny.py b/tests/test_hub_deny.py index 76797b8..1c238e6 100644 --- a/tests/test_hub_deny.py +++ b/tests/test_hub_deny.py @@ -85,6 +85,28 @@ class TestHubDeny(HubCase): self.assertIn("call to Skills session", logged[0]["text"]) self.assertNotIn(ask_id, [item["id"] for item in self.snapshot()["inbox"]]) + def test_one_signature_denies_a_mixed_selection_and_tells_each_requester(self): + self.claim("s1", "Skills session") + row = {"title": PUSH, "workdir": "/repo", "description": "ship", "check": "manual", "status": "queued", "roles": {"Operator": "R", "Skills session": "A"}} + self.write_council({"rasci.json": {"operator/push": row}}) + ask_id = self.create_ask(ASK) + review_id = self.request("feat: deny me", "") + digest = hashlib.sha256(PUSH.encode()).hexdigest() + entries = [ + self.denial("ask", ask_id, "", "post", ""), + self.denial("operator", "operator/push", digest, "post", ""), + {**self.entry("review", review_id, "", ""), "verb": "reject"}, + ] + with mock.patch.object(batches, "UNDO", 0.05): + self.assertEqual(self.approve(entries, self.signed(entries, self.pin()))[0], 202) + self.settle(lambda: self.ask_status(ask_id, "0") == "denied") + self.settle(lambda: (skill_scripts.RASCI, ["set", "operator/push", "--status", "stopped"]) in self.scripts.calls) + self.settle(lambda: self.tool("review_wait", {"id": review_id, "timeout_seconds": 0})["structuredContent"]["status"] == "rejected") + self.settle(lambda: len(self.radio()) >= 2) + self.assertEqual(sorted(sent[:3] for sent in self.radio()), [["post", "--to", "Skills session"]] * 2) + self.assertTrue(all("Denied by the operator: " in sent[-1] for sent in self.radio()), self.radio()) + self.assertFalse({ask_id, review_id} & {item["id"] for item in self.snapshot()["inbox"]}) + def test_every_class_but_mayday_reaches_the_agent_or_all(self): session = "s9" signer = self.pin()