← Commit history

browser_close: name the {} exception in both surfaces

Kyle Bergstedt ·08c06ffa84 ·3mo ago ·parent 26d3918
3 files changed +28−1
CHANGELOG.md+22
@@ -2,6 +2,28 @@  ## UNRELEASED (rides the next version) — AD-base re-discovery + self-guarded powershell primitives +### A Key Rule taught the wrong mental model for `browser_close` (destructive)++Found while enumerating which pup verbs execute on `{}` — a safety audit prompted by KICAD+publishing kicad's dangerous list, not by a bug report.++`SKILL.md`'s Key Rules state that on session-scoped verbs `'{}'` targets the **active** session.+That is true of every such verb except one: **`browser_close '{}'` closes ALL sessions**, every+window, across every AI thread on the machine, immediately and without confirmation. So an AI that+correctly internalised the general rule and wanted to close one window had a documented reason to+expect `browser_close '{}'` to do exactly that — and would instead have destroyed every window,+including long-running dashboards belonging to other threads.++The verb's behaviour is intentional and its describe entry already said "(destructive)", so this is+not a code defect and the semantics are unchanged — a breaking change to a documented verb would be+the wrong fix. What was wrong was the *mental model* the skill taught. Both surfaces now carry the+exception explicitly: the Key Rule names it, and the describe entry (what an AI actually reads+before calling) states that `{}` here does not mean the active session and points at+`browser_close_window` for the single-window case.++Worth noting the general shape: a correct general rule with one unstated exception is more dangerous+than no rule, because it manufactures confidence in precisely the case it doesn't cover.+ ### Recorded WHY the desktop_taskbar gates stay (the obvious reading is wrong)  Raised with AD as an open question — pup gates every `desktop_taskbar` call on `win32`, and if mac
SKILL.md+5
@@ -153,6 +153,11 @@ threads; pass `wikiView:"authed"|"public"` on browser_open_window instead. Detai  - **Always pass `sessionId`** on session-scoped verbs — `'{}'` targets the *active* session, which may not   be the one you mean. There is no top-level `pup` subcommand; it's `adom-desktop browser_<cmd> <json>`.+  **ONE DANGEROUS EXCEPTION: `browser_close`.** There `'{}'` does NOT mean "the active session" — it means+  **ALL sessions, every window, across every AI thread on the machine**, immediately and without+  confirmation. If you want to close one window it is `browser_close_window {"sessionId":"…"}`, never+  `browser_close '{}'`. The rule above would otherwise lead you to expect the opposite, which is exactly+  how a tidy-up call becomes a data-loss event on someone's long-running dashboards. - **Background is guaranteed; `foreground:true` is the only way to show a window** — not sizing, not   navigating, not screenshotting. - **Own your window** (`owner:"<task>"`, task-prefixed `sessionId`); never navigate/close a window you
src/server.js+1−1
@@ -5735,7 +5735,7 @@ const server = http.createServer(async (req, res) => {             V('browser_errors', 'Console errors + failed network requests for a session/tab (clears by default).', { sessionId: 'required str', tabId: 'optional str', clear: 'optional bool (default true)' }, 10, { sessionId: 'work' }),             V('browser_wait', 'Wait N ms (or for a selector) before the next step.', { sessionId: 'required str', ms: 'optional int', selector: 'optional str' }, 60, { sessionId: 'work', ms: 500 }),             V('browser_rescan', 'Recover orphaned sessions: reconnect dropped CDP sockets + rebuild sessions from window titles. Pass adoptOrphans:true to also adopt untagged tabs.', { adoptOrphans: 'optional bool' }, 30, {}),-            V('browser_close', 'Close ALL pup sessions + windows at once (destructive).', {}, 20, {}),+            V('browser_close', 'Close ALL pup sessions + windows at once (destructive). NOTE: `{}` here does NOT mean "the active session" the way it does on other session-scoped verbs — with no args this closes EVERY window, across every AI thread on the machine, with no confirmation. To close ONE window use browser_close_window {sessionId}. Pass {sessionId} here only if you genuinely mean that one session.', {}, 20, {}),             V('desktop_record_status', 'Status of the desktop screen recorder + any active clip.', {}, 10, {}),             V('desktop_record_list', 'List completed desktop recordings on disk.', {}, 10, {}),             V('desktop_recorder_close', 'Close the desktop screen-recorder UI.', {}, 15, {}),