diff --git a/CLAUDE.md b/CLAUDE.md index 84bce09..180bbd9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -115,7 +115,7 @@ Both tools use the same environment variables: ``NEON_ORG``, ``NEON_KEY``, and ` - `alias`: (optional) Human-readable name for the accessory (used in Slack/log lines that refer specifically to the second relay) - Users: `users.json` (schema in `models/users.py::CONFIG_SCHEMA`) - Machine names must match ESPHome configs and can only contain `[a-z0-9_-]` -- Machines can be looked up by either name or alias in Slack commands +- Machines can be looked up by either name or alias (case-insensitively) in Slack commands (both at-mentions and the `/oops-clear` slash command) **State Persistence**: - Machine state is persisted to disk on every update using pickle diff --git a/docs/features/completed/slack-slack-command.md b/docs/features/completed/slack-slack-command.md new file mode 100644 index 0000000..a0e8741 --- /dev/null +++ b/docs/features/completed/slack-slack-command.md @@ -0,0 +1,253 @@ +# Slack Slash Command and Case-Insensitive Search + +You must read, understand, and follow all instructions in `../README.md` when planning and implementing this feature. + +## Overview + +We need to improve the user experience for clearing machines (oops or maintenance). + +1. We should introduce a Slack slash command, `/oops-clear` for this. The slash command should only be usable from the configured control channel (`SLACK_CONTROL_CHANNEL_ID`). When used, it should optionally take the name of the machine to be cleared. If not provided, it should launch a Slack Block Kit Modal which has a single input, a dropdown listing all currently oopsed or locked out machines for the user to select from. There should be no default. Once the user selects the machine from the dropdown and submits the form, the machine should be cleared. Be sure to also include updates to the documentation for this new functionality and the installation process for it. +2. Both the new above slash command as well as the existing bot user mentions should be updated to use case-insensitive matching of machine names. + +## Implementation Plan + +### Overview + +This feature adds a `/oops-clear` Slack slash command (in two forms: a direct +`/oops-clear ` form and an interactive Block Kit modal form) and makes +all machine-name/alias lookups case-insensitive. The slash command is restricted +to the control channel (`SLACK_CONTROL_CHANNEL_ID`). + +Both the slash command and existing `@mention` commands resolve machines through +`MachinesConfig.get_machine()`, so making that single method case-insensitive +satisfies requirement 2 for both code paths at once. Doing this first (Milestone +1) also lets later milestones rely on case-insensitive lookups. + +The slash command and modal are handled in `slack_handler.py` using the existing +`slack-bolt` `AsyncApp` (already wired through Socket Mode in `__init__.py`), so +no new dependencies or server-startup changes are required. Slack delivers slash +commands and modal (view) submissions over the existing Socket Mode connection; +the only new Slack-app configuration the operator must do is to create the +`/oops-clear` slash command and enable Interactivity (documented in Milestone 4). + +Key files to modify: + +- `src/dm_mac/models/machine.py`: case-insensitive `get_machine()`. +- `src/dm_mac/slack_handler.py`: register and implement the slash command + modal + view-submission handlers; refresh `HELP_RESPONSE`. +- `tests/test_slack_handler.py`: unit tests for new handlers and updated `test_init`. +- `tests/models/test_machine.py` (or existing machine-config test): tests for + case-insensitive `get_machine()`. +- `docs/source/slack.rst`: setup (slash command + interactivity) and usage docs. +- `README.md` / `CLAUDE.md`: brief mentions where appropriate. + +### Design Notes / Decisions + +- **Channel gating:** Slash commands can be invoked from any channel the user can + type in, so the handler must explicitly verify `body["channel_id"] == + self.control_channel_id`. If not, respond ephemerally (`await ack("...")`) that + the command is only usable in the control channel and take no action. +- **Direct form (`/oops-clear `):** Resolve via `get_machine()`. Reuse + the same clearing logic as the existing mention `clear` command (unoops and/or + unlock if set). Respond ephemerally with the outcome (cleared / not + oopsed-or-locked / invalid machine). Clearing itself already posts to the oops + and control channels via `mach.unoops()` / `mach.unlock()`. +- **Modal form (`/oops-clear` with no argument):** Build the list of machines that + are currently `is_oopsed` or `is_locked_out`. If none, respond ephemerally + ("No machines are currently oopsed or locked out.") and do **not** open a modal + (a `static_select` with zero options is invalid Block Kit). Otherwise open a + modal via `client.views_open(trigger_id=..., view=...)` containing a single + required `input` block with a `static_select` element (no `initial_option`, so + no default). Each option uses the machine `display_name` for `text` and the + canonical machine `name` for `value`. The view carries `callback_id = + "oops_clear_modal"`. +- **Modal submission:** Registered via `self.app.view("oops_clear_modal")`. Extract + the selected machine `name` from + `body["view"]["state"]["values"][][]["selected_option"]["value"]`, + resolve it, and clear it (unoops/unlock). `await ack()` to close the modal. The + resulting Slack channel posts come from `mach.unoops()` / `mach.unlock()` as + usual. Guard against the (rare) race where the machine was already cleared + between modal open and submit. +- **Refactor for reuse:** Extract the shared "clear this machine and report what + happened" logic into a small helper (e.g. `_clear_machine(mach) -> str` returning + a human-readable result string) used by the existing mention `clear`, the slash + command direct form, and the modal submission, to avoid duplicating the + oopsed/locked-out branching. +- **Registration:** In `SlackHandler.__init__`, after the existing + `app.event("app_mention")` registration, add + `self.app.command("/oops-clear")(self.oops_clear_command)` and + `self.app.view("oops_clear_modal")(self.oops_clear_modal_submit)`. The existing + `test_init` asserts exact `mock_calls`, so it must be updated to include these. +- **No server-startup changes:** Socket Mode already routes commands and view + submissions; `__init__.py` needs no change. + +### Milestone 1: Case-Insensitive Machine Matching + +**Commit prefix:** `slack-slash-command - 1.1` through `slack-slash-command - 1.2` + +#### Task 1.1: Make `MachinesConfig.get_machine()` case-insensitive +- In `src/dm_mac/models/machine.py`, build lowercase-keyed lookup dicts + (`machines_by_name_lower`, `machines_by_alias_lower`) when machines are loaded, + and update `get_machine()` to look up `name_or_alias.lower()` against them + (name first, then alias), preserving current return semantics (`Optional[Machine]`). +- Keep the existing `machines_by_name` / `machines_by_alias` dicts intact (they are + used elsewhere, e.g. status, tests) — only the lookup in `get_machine()` changes. + +#### Task 1.2: Tests for case-insensitive lookup +- Add tests verifying `get_machine()` resolves names and aliases regardless of case + (e.g. `METAL-MILL`, `metal-mill`, `Metal Mill`, `metal mill`) and still returns + `None` for unknown values. +- This implicitly covers requirement 2 for the existing `@mention` commands, since + they call `get_machine()`. Add an explicit `app_mention`/`handle_command` test + using a mixed-case machine name to confirm end-to-end case-insensitivity for + mentions. + +**Milestone close-out:** update this document's progress section, run `nox -s tests` +(all passing), commit. + +### Milestone 2: `/oops-clear` Slash Command (Direct Form) + +**Commit prefix:** `slack-slash-command - 2.1` through `slack-slash-command - 2.3` + +#### Task 2.1: Register the slash command and shared clear helper +- Register `self.app.command("/oops-clear")(self.oops_clear_command)` in + `__init__`. Update `test_init` expectations accordingly. +- Add `_clear_machine(mach) -> str` helper encapsulating the unoops/unlock branching + and result message; refactor the existing mention `clear()` to use it. + +#### Task 2.2: Implement `oops_clear_command` (direct form + channel gating) +- Handler signature `async def oops_clear_command(self, ack, body, command)` (or + using `respond`); `await ack(...)` promptly. +- Reject (ephemerally) when `body["channel_id"] != self.control_channel_id`. +- If `command["text"]` (trimmed) is non-empty, treat it as a machine name/alias, + resolve via `get_machine()`, and clear it with `_clear_machine()`, responding + ephemerally with the result (cleared / already clear / invalid machine). +- If `command["text"]` is empty, defer to the modal flow added in Milestone 3 + (in this milestone, a placeholder ephemeral response is acceptable, replaced in M3). + +#### Task 2.3: Tests for the direct form +- Control channel + valid oopsed machine name → cleared (asserts unoops/unlock and + ephemeral confirmation). +- Control channel + machine not oopsed/locked → appropriate ephemeral message. +- Control channel + invalid machine name → invalid-machine ephemeral message. +- Non-control channel → rejected ephemerally, no state change. +- Mixed-case machine name → resolves correctly (case-insensitivity through the + command path). + +**Milestone close-out:** update progress section, run `nox -s tests`, commit. + +### Milestone 3: Block Kit Modal (Interactive Form) + +**Commit prefix:** `slack-slash-command - 3.1` through `slack-slash-command - 3.3` + +#### Task 3.1: Open the modal when no machine is given +- In `oops_clear_command`, when no text is supplied, gather machines with + `state.is_oopsed or state.is_locked_out`. +- If empty → ephemeral "No machines are currently oopsed or locked out." +- Otherwise build the modal view (single required `input` block, `static_select`, + no default, `callback_id="oops_clear_modal"`) and call + `client.views_open(trigger_id=body["trigger_id"], view=view)` after `ack()`. +- Add a small builder method (e.g. `_build_clear_modal(machines) -> dict`) so the + view structure is unit-testable in isolation. + +#### Task 3.2: Handle modal submission +- Register `self.app.view("oops_clear_modal")(self.oops_clear_modal_submit)` in + `__init__` (update `test_init`). +- Extract the selected machine `value`, resolve it, clear via `_clear_machine()`, + and `await ack()`. Handle the already-cleared/missing-machine race gracefully. + +#### Task 3.3: Tests for the modal flow +- No-argument command with one or more oopsed/locked machines → `views_open` called + with a view whose options match exactly the oopsed/locked machines (text = + display_name, value = name), no `initial_option`. +- No-argument command with nothing oopsed/locked → ephemeral message, `views_open` + not called. +- Modal submission → selected machine cleared (unoops/unlock invoked), `ack` called. +- Modal submission for a machine already cleared → graceful handling, no error. + +**Milestone close-out:** update progress section, run `nox -s tests`, commit. + +### Milestone 4: Acceptance Criteria + +**Commit prefix:** `slack-slash-command - 4.1` through `slack-slash-command - 4.4` + +#### Task 4.1: Documentation +- `docs/source/slack.rst`: + - **Setup:** add steps to create the `/oops-clear` slash command (Features → + Slash Commands → Create New Command; command `/oops-clear`, a short + description, and a usage hint such as `[machine name]`) and to enable + Interactivity (Features → Interactivity & Shortcuts → toggle on) so modal + submissions are delivered. Note that with Socket Mode no Request URL is needed. + Reinstall the app if Slack prompts that new scopes/commands require it. + - **Usage:** document `/oops-clear` (control-channel only), both the direct + `/oops-clear ` form and the no-argument modal form, and that name + matching is case-insensitive (for both slash command and mentions). +- Refresh `HELP_RESPONSE` and any README/CLAUDE.md references as needed, matching + existing style/tone/verbosity. + +#### Task 4.2: Test coverage +- Ensure all new code paths in `slack_handler.py` and `machine.py` have unit-test + coverage (`nox -s tests`). + +#### Task 4.3: All nox sessions pass +- Run and ensure passing: `nox -s tests`, `nox -s pre-commit`, `nox -s mypy`, + `nox -s typeguard`, `nox -s docs` (and `safety` if part of the standard gate). +- Fix any lint/type/format issues introduced. + +#### Task 4.4: Finalize +- Update this document marking the feature complete, then move it from + `docs/features/slack-slack-command.md` to + `docs/features/completed/slack-slack-command.md`. Commit. + +## Decisions (confirmed) + +- **No ephemeral success confirmation:** On a successful clear, the slash command + silently `ack()`s (no extra reply), relying on the existing oops/control channel + posts — matching the existing mention `clear` behavior. Ephemeral responses are + still used for *error/edge* cases (wrong channel, invalid machine, machine not + oopsed/locked, nothing to clear in the modal flow), since those produce no channel + post otherwise. (Note: a slash command must still call `ack()` within 3 s to avoid + Slack showing a failure; a silent `await ack()` satisfies this.) +- **Generic source label:** Channel messages keep the generic `"Slack"` source + already hard-coded in the `Machine` model methods. No `Machine` model changes for + source attribution. + +## Progress + +- Planning complete; approved to proceed autonomously (commit after every task). +- **Milestone 1 complete** (case-insensitive machine matching): + - 1.1: `MachinesConfig.get_machine()` is now case-insensitive via lowercased + name/alias lookup maps. + - 1.2: Added `get_machine()` case-insensitive unit tests and a mixed-case + `@mention` oops command test. `nox -s tests` passing. +- **Milestone 2 complete** (`/oops-clear` slash command, direct form): + - 2.1: Extracted shared `_invalid_machine_msg()` and `_clear_machine()` helpers + and refactored `clear()` onto them (no behavior change). + - 2.2: Registered the `/oops-clear` command and implemented + `oops_clear_command()` — control-channel gating + direct + `/oops-clear ` form (silent ack on success, ephemeral on + error/edge). No-arg case is a temporary usage hint, replaced in Milestone 3. + - 2.3: Added direct-form tests (clears oopsed, already-clear, invalid machine, + wrong channel, mixed-case) and updated `test_init`. `nox -s tests` passing. +- **Milestone 3 complete** (Block Kit modal): + - 3.1: No-arg `/oops-clear` now opens a `static_select` modal of currently + oopsed/locked machines (no default); ephemeral notice when none. + - 3.2: Registered and implemented `oops_clear_modal_submit()` to clear the + selected machine via `_clear_machine`, with graceful handling of the + already-cleared race and missing/unknown selections. + - 3.3: Added modal-flow tests (open with options, empty case, submit clears, + already-clear, no selection, unknown machine). `slack_handler.py` at 100% + coverage; `nox -s tests` passing. +- **Milestone 4 complete** (acceptance criteria): + - 4.1: Documented the slash command + interactivity setup and `/oops-clear` + usage (both forms) plus case-insensitive matching in `docs/source/slack.rst`; + refreshed `HELP_RESPONSE` and the CLAUDE.md Slack-lookup note. + - 4.2/4.3: `slack_handler.py` at 100% coverage; `nox` sessions `tests` (309 + passing), `pre-commit`, `mypy`, `typeguard`, and `docs` all pass. The + `safety` session fails only on **pre-existing upstream CVEs** in transitive + dependencies (`aiohttp`, `dulwich`, `idna`); this branch changes no + dependencies (`pyproject.toml`/`poetry.lock` untouched), so it is unrelated + to this feature and best addressed by a separate dependency-bump change. + - 4.4: Feature complete; this document moved to `docs/features/completed/`. + +**Feature complete.** diff --git a/docs/source/slack.rst b/docs/source/slack.rst index 9c87b93..d6334a6 100644 --- a/docs/source/slack.rst +++ b/docs/source/slack.rst @@ -17,7 +17,7 @@ To set up the Slack integration: 1. Create your new app "from scratch". 2. Set a meaningful name, such as ``machine-access-control`` and create the app in your Workspace. 3. In the left menu, navigate to ``OAuth & Permissions``. - 4. In the "Scopes" pane, under "Bot Token Scopes", click "Add an OAuth Scope" and add scopes for ``app_mentions:read``, ``canvases:read``, ``canvases:write``, ``channels:read``, ``chat:write``, ``groups:read``, ``groups:write``, ``incoming-webhook``, ``users.profile:read``, and ``users:read``. + 4. In the "Scopes" pane, under "Bot Token Scopes", click "Add an OAuth Scope" and add scopes for ``app_mentions:read``, ``canvases:read``, ``canvases:write``, ``channels:read``, ``chat:write``, ``commands``, ``groups:read``, ``groups:write``, ``incoming-webhook``, ``users.profile:read``, and ``users:read``. 2. In your workspace, create a new private channel for admins to interact with MAC in, and MAC to post status updates to. 3. In the left menu, navigate to ``Install App``. Click on the button to install to your workspace. When prompted for a channel for the app to post in, select the private channel that you created in the previous step. @@ -25,6 +25,9 @@ To set up the Slack integration: 5. Go back to the main settings for your app and navigate to ``Socket Mode`` under ``Settings`` on the left menu; toggle on ``Enable Socket Mode``. For ``Token Name``, enter ``socket-mode-token`` and click ``Generate``. Copy the generated token and set it as the ``SLACK_APP_TOKEN`` environment variable for the MAC server. If you need to retrieve this token later, it can be found in the ``App-Level Tokens`` pane of the ``Settings -> Basic Information`` page. 6. Go back to the main settings for your app and navigate to ``Basic Information`` under ``Settings`` on the left menu; in the ``App Credentials`` pane click ``Show`` in the ``Signing Secret`` box and then copy that value; set it as the ``SLACK_SIGNING_SECRET`` environment variable for the MAC server. 7. Go back to the main settings for your app and navigate to ``Event Subscriptions`` under ``Features`` on the left menu; click the toggle in the upper left of the panel to Enable Events; under ``Subscribe to bot events`` add a subscription for ``app_mention``. +8. Go back to the main settings for your app and navigate to ``Interactivity & Shortcuts`` under ``Features`` on the left menu; toggle ``Interactivity`` on. Because the app uses Socket Mode, no Request URL is required. This is needed so that submissions of the ``/oops-clear`` selection modal are delivered to the server. +9. Go back to the main settings for your app and navigate to ``Slash Commands`` under ``Features`` on the left menu; click ``Create New Command``. Set the ``Command`` to ``/oops-clear``, enter a short ``Short Description`` such as "Clear a machine's Oops/lockout", and an optional ``Usage Hint`` of ``[machine name]``. Save the command. (With Socket Mode enabled, no Request URL is needed.) +10. If Slack prompts you that the app needs to be reinstalled to apply the new ``commands`` scope and slash command, navigate back to ``Install App`` and reinstall it to your workspace. .. _slack.configuration: @@ -54,6 +57,16 @@ Using an example bot name of ``@machine-access-control``, the supported commands **Note:** If a machine has an ``alias`` configured in ``machines.json``, the bot's responses will use the alias instead of the machine name for better readability. +**Note:** Machine names and aliases are matched case-insensitively, for both the at-mention commands above and the ``/oops-clear`` slash command below. + +Clearing a Machine with ``/oops-clear`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +As a convenience for clearing Oops and maintenance lock-out states, MAC also provides a ``/oops-clear`` slash command. Like the control commands above, it can only be used from the ``SLACK_CONTROL_CHANNEL_ID`` channel; using it elsewhere returns a private message telling you so. + +* ``/oops-clear `` - Clear all Oops and/or maintenance lock-out states on the named machine (by name or alias). This is equivalent to the ``clear`` at-mention command. +* ``/oops-clear`` (with no machine name) - Open a modal with a single dropdown listing all machines that are currently Oopsed or locked-out. Select a machine and click ``Clear`` to clear it. If no machines are currently Oopsed or locked-out, you'll get a private message saying so instead. + In addition, changes to all machines' Oops and maintenance lock-out states will be posted as messages in the ``SLACK_OOPS_CHANNEL_ID`` channel. .. _slack.override_logins: diff --git a/src/dm_mac/models/machine.py b/src/dm_mac/models/machine.py index b5d7e3b..6131e0b 100644 --- a/src/dm_mac/models/machine.py +++ b/src/dm_mac/models/machine.py @@ -343,6 +343,10 @@ def __init__(self) -> None: logger.debug("Initializing MachinesConfig") self.machines_by_name: Dict[str, Machine] = {} self.machines_by_alias: Dict[str, Machine] = {} + #: Case-insensitive lookup maps (lowercased name/alias -> Machine), used + #: by :py:meth:`get_machine` so Slack commands can match regardless of case. + self.machines_by_name_lower: Dict[str, Machine] = {} + self.machines_by_alias_lower: Dict[str, Machine] = {} self.machines: List[Machine] = [] mdict: Dict[str, Any] mname: str @@ -352,14 +356,36 @@ def __init__(self) -> None: mach: Machine = Machine(name=mname, **mdict) self.machines.append(mach) self.machines_by_name[mach.name] = mach + self.machines_by_name_lower[mach.name.lower()] = mach if mach.alias: self.machines_by_alias[mach.alias] = mach + # Populate the case-insensitive alias map in a second pass, after every + # machine name is known, so a collision is detected regardless of config + # order. Two machines whose aliases (or an alias and another machine's + # name) differ only by case would make lookups ambiguous, so we fail + # fast rather than silently overwrite an entry. + for mach in self.machines: + if not mach.alias: + continue + alias_key: str = mach.alias.lower() + existing: Optional[Machine] = self.machines_by_name_lower.get( + alias_key + ) or self.machines_by_alias_lower.get(alias_key) + if existing is not None and existing is not mach: + raise ValueError( + f"Machine alias '{mach.alias}' (machine '{mach.name}') " + f"collides case-insensitively with machine " + f"'{existing.name}'. Machine names and aliases must be " + f"unique when compared case-insensitively." + ) + self.machines_by_alias_lower[alias_key] = mach self.load_time: float = time() def get_machine(self, name_or_alias: str) -> Optional[Machine]: - """Get a machine by name or alias.""" - return self.machines_by_name.get(name_or_alias) or self.machines_by_alias.get( - name_or_alias + """Get a machine by name or alias (case-insensitive).""" + key: str = name_or_alias.lower() + return self.machines_by_name_lower.get(key) or self.machines_by_alias_lower.get( + key ) def _load_and_validate_config(self) -> Dict[str, Dict[str, Any]]: diff --git a/src/dm_mac/slack_handler.py b/src/dm_mac/slack_handler.py index c56d520..244bcbb 100644 --- a/src/dm_mac/slack_handler.py +++ b/src/dm_mac/slack_handler.py @@ -11,7 +11,9 @@ from humanize import naturaldelta from quart import Quart from slack_bolt.async_app import AsyncApp +from slack_bolt.context.ack.async_ack import AsyncAck from slack_bolt.context.say.async_say import AsyncSay +from slack_sdk.web.async_client import AsyncWebClient from slack_sdk.web.async_slack_response import AsyncSlackResponse from dm_mac.models.machine import Machine @@ -62,6 +64,14 @@ def __eq__(self, other) -> bool: class SlackHandler: """Handle Slack integration.""" + #: Block Kit ``callback_id`` for the ``/oops-clear`` selection modal; used + #: both when opening the modal and when routing its submission. + MODAL_CALLBACK_ID: str = "oops_clear_modal" + #: ``block_id`` of the machine-selection input block in the modal. + MODAL_BLOCK_ID: str = "machine_block" + #: ``action_id`` of the machine-selection ``static_select`` element. + MODAL_ACTION_ID: str = "machine_select" + HELP_RESPONSE: str = dedent(""" Hi, I'm the Machine Access Control slack bot. Mention my username followed by one of these commands: @@ -70,6 +80,10 @@ class SlackHandler: "lock " - set maintenance lockout on this machine "clear " - clear oops and/or maintenance lockout on this machine + Machine names and aliases are matched case-insensitively. + You can also use the "/oops-clear" slash command (in the control channel) to + clear a machine, optionally with no machine name to pick one from a menu. + I am Free and Open Source software: https://github.com/jantman/machine-access-control """).strip() @@ -84,6 +98,8 @@ def __init__(self, quart_app: Quart): signing_secret=os.environ["SLACK_SIGNING_SECRET"], ) self.app.event("app_mention")(self.app_mention) + self.app.command("/oops-clear")(self.oops_clear_command) + self.app.view(self.MODAL_CALLBACK_ID)(self.oops_clear_modal_submit) logger.debug("SlackHandler initialized.") async def app_mention(self, body: Dict[str, Any], say: AsyncSay) -> None: @@ -235,17 +251,22 @@ async def lock(self, msg: Message, say: AsyncSay) -> None: return await mach.lockout(slack=self) - async def clear(self, msg: Message, say: AsyncSay) -> None: - """Clear oops and lock status on a machine.""" - mname: str = " ".join(msg.command[1:]) - mconf: MachinesConfig = self.quart.config["MACHINES"] - mach: Optional[Machine] = mconf.get_machine(mname) - if not mach: - await say( - f"Invalid machine name or alias '{mname}'. Use status command to " - f"list all machines." - ) - return + @staticmethod + def _invalid_machine_msg(name_or_alias: str) -> str: + """Return the standard 'invalid machine name or alias' message.""" + return ( + f"Invalid machine name or alias '{name_or_alias}'. Use status command " + f"to list all machines." + ) + + async def _clear_machine(self, mach: Machine) -> Optional[str]: + """Clear oops and/or maintenance lock-out status on a machine. + + Returns ``None`` if something was cleared (the resulting Slack channel + posts from :py:meth:`Machine.unoops` / :py:meth:`Machine.unlock` cover + the outcome), or a human-readable message string if the machine was + already clear (so the caller can surface it to the requester). + """ acted = False if mach.state.is_oopsed: await mach.unoops(slack=self) @@ -254,7 +275,132 @@ async def clear(self, msg: Message, say: AsyncSay) -> None: await mach.unlock(slack=self) acted = True if not acted: - await say(f"Machine {mach.display_name} is not oopsed or locked-out.") + return f"Machine {mach.display_name} is not oopsed or locked-out." + return None + + async def clear(self, msg: Message, say: AsyncSay) -> None: + """Clear oops and lock status on a machine.""" + mname: str = " ".join(msg.command[1:]) + mconf: MachinesConfig = self.quart.config["MACHINES"] + mach: Optional[Machine] = mconf.get_machine(mname) + if not mach: + await say(self._invalid_machine_msg(mname)) + return + result: Optional[str] = await self._clear_machine(mach) + if result: + await say(result) + + async def oops_clear_command( + self, ack: AsyncAck, command: Dict[str, Any], client: AsyncWebClient + ) -> None: + """Handle the ``/oops-clear`` slash command. + + Usable only from the control channel. With an argument + (``/oops-clear ``) it clears that machine directly. With + no argument it opens a Block Kit modal to pick a machine to clear. + ``ack`` is always called promptly so Slack does not report the command + as failed; error/edge cases respond with an ephemeral message, while a + successful clear acks silently (the resulting channel posts cover the + outcome). + """ + channel_id: str = command.get("channel_id", "") + if channel_id != self.control_channel_id: + logger.warning( + "Ignoring /oops-clear from non-control channel %s", channel_id + ) + await ack( + "The `/oops-clear` command can only be used in the control channel." + ) + return + text: str = command.get("text", "").strip() + mconf: MachinesConfig = self.quart.config["MACHINES"] + if text: + mach: Optional[Machine] = mconf.get_machine(text) + if not mach: + await ack(self._invalid_machine_msg(text)) + return + result: Optional[str] = await self._clear_machine(mach) + if result: + await ack(result) + else: + await ack() + return + # No machine specified: open a modal to pick from oopsed/locked machines. + machines: List[Machine] = sorted( + (m for m in mconf.machines if m.state.is_oopsed or m.state.is_locked_out), + key=lambda m: m.display_name.lower(), + ) + if not machines: + await ack("No machines are currently oopsed or locked out.") + return + await ack() + await client.views_open( + trigger_id=command["trigger_id"], + view=self._build_clear_modal(machines), + ) + + def _build_clear_modal(self, machines: List[Machine]) -> Dict[str, Any]: + """Build the ``/oops-clear`` Block Kit selection modal. + + The modal has a single required input: a ``static_select`` dropdown of + the given machines (option text = display name, value = machine name) + with no default selection. + """ + options: List[Dict[str, Any]] = [ + { + "text": {"type": "plain_text", "text": mach.display_name}, + "value": mach.name, + } + for mach in machines + ] + return { + "type": "modal", + "callback_id": self.MODAL_CALLBACK_ID, + "title": {"type": "plain_text", "text": "Clear Machine"}, + "submit": {"type": "plain_text", "text": "Clear"}, + "close": {"type": "plain_text", "text": "Cancel"}, + "blocks": [ + { + "type": "input", + "block_id": self.MODAL_BLOCK_ID, + "label": {"type": "plain_text", "text": "Machine to clear"}, + "element": { + "type": "static_select", + "action_id": self.MODAL_ACTION_ID, + "placeholder": { + "type": "plain_text", + "text": "Select a machine", + }, + "options": options, + }, + } + ], + } + + async def oops_clear_modal_submit( + self, ack: AsyncAck, view: Dict[str, Any] + ) -> None: + """Handle submission of the ``/oops-clear`` selection modal. + + Acknowledges promptly to close the modal, then clears the selected + machine. The machine may have been cleared between opening and + submitting the modal; that and any unexpected missing selection are + handled gracefully (no error raised). + """ + await ack() + try: + selected: str = view["state"]["values"][self.MODAL_BLOCK_ID][ + self.MODAL_ACTION_ID + ]["selected_option"]["value"] + except (KeyError, TypeError): + logger.warning("/oops-clear modal submitted with no machine selected") + return + mconf: MachinesConfig = self.quart.config["MACHINES"] + mach: Optional[Machine] = mconf.get_machine(selected) + if not mach: + logger.warning("/oops-clear modal selected unknown machine '%s'", selected) + return + await self._clear_machine(mach) @staticmethod def _both_relays_suffix(machine: Machine) -> str: diff --git a/tests/models/test_machine.py b/tests/models/test_machine.py index 2f799f7..751b255 100644 --- a/tests/models/test_machine.py +++ b/tests/models/test_machine.py @@ -235,3 +235,91 @@ def test_get_machine_not_found(self, fixtures_path: str, tmp_path: Path) -> None cls: MachinesConfig = MachinesConfig() machine = cls.get_machine("nonexistent") assert machine is None + + def test_get_machine_case_insensitive_name( + self, fixtures_path: str, tmp_path: Path + ) -> None: + """Test getting a machine by name regardless of case.""" + conf: Dict[str, Dict[str, Any]] = { + "metal-mill": {"authorizations_or": ["Metal Mill"], "alias": "Metal Mill"}, + "hammer": {"authorizations_or": ["Woodshop Orientation"]}, + } + cpath: str = str(os.path.join(tmp_path, "machines.json")) + with open(cpath, "w") as fh: + json.dump(conf, fh, sort_keys=True, indent=4) + with patch.dict(os.environ, {"MACHINES_CONFIG": cpath}): + with patch(f"{pbm}.MachineState", autospec=True): + cls: MachinesConfig = MachinesConfig() + for variant in ["metal-mill", "Metal-Mill", "METAL-MILL", "MeTaL-mIlL"]: + machine = cls.get_machine(variant) + assert machine is not None, variant + assert machine.name == "metal-mill" + + def test_get_machine_case_insensitive_alias( + self, fixtures_path: str, tmp_path: Path + ) -> None: + """Test getting a machine by alias regardless of case.""" + conf: Dict[str, Dict[str, Any]] = { + "metal-mill": {"authorizations_or": ["Metal Mill"], "alias": "Metal Mill"}, + "hammer": {"authorizations_or": ["Woodshop Orientation"]}, + } + cpath: str = str(os.path.join(tmp_path, "machines.json")) + with open(cpath, "w") as fh: + json.dump(conf, fh, sort_keys=True, indent=4) + with patch.dict(os.environ, {"MACHINES_CONFIG": cpath}): + with patch(f"{pbm}.MachineState", autospec=True): + cls: MachinesConfig = MachinesConfig() + for variant in ["Metal Mill", "metal mill", "METAL MILL", "mEtAl MiLl"]: + machine = cls.get_machine(variant) + assert machine is not None, variant + assert machine.name == "metal-mill" + assert machine.alias == "Metal Mill" + + def test_aliases_colliding_case_insensitively_raise( + self, fixtures_path: str, tmp_path: Path + ) -> None: + """Two aliases differing only by case must be rejected at load.""" + conf: Dict[str, Dict[str, Any]] = { + "metal-mill": {"authorizations_or": ["a"], "alias": "Big Machine"}, + "hammer": {"authorizations_or": ["b"], "alias": "big machine"}, + } + cpath: str = str(os.path.join(tmp_path, "machines.json")) + with open(cpath, "w") as fh: + json.dump(conf, fh, sort_keys=True, indent=4) + with patch.dict(os.environ, {"MACHINES_CONFIG": cpath}): + with patch(f"{pbm}.MachineState", autospec=True): + with pytest.raises(ValueError, match="collides case-insensitively"): + MachinesConfig() + + def test_alias_colliding_with_other_machine_name_raises( + self, fixtures_path: str, tmp_path: Path + ) -> None: + """An alias that matches another machine's name (case-insensitively) raises.""" + conf: Dict[str, Dict[str, Any]] = { + "metal-mill": {"authorizations_or": ["a"]}, + "hammer": {"authorizations_or": ["b"], "alias": "Metal-Mill"}, + } + cpath: str = str(os.path.join(tmp_path, "machines.json")) + with open(cpath, "w") as fh: + json.dump(conf, fh, sort_keys=True, indent=4) + with patch.dict(os.environ, {"MACHINES_CONFIG": cpath}): + with patch(f"{pbm}.MachineState", autospec=True): + with pytest.raises(ValueError, match="collides case-insensitively"): + MachinesConfig() + + def test_alias_equal_to_own_name_is_allowed( + self, fixtures_path: str, tmp_path: Path + ) -> None: + """An alias equal to the machine's own name is harmless, not a collision.""" + conf: Dict[str, Dict[str, Any]] = { + "hammer": {"authorizations_or": ["b"], "alias": "Hammer"}, + } + cpath: str = str(os.path.join(tmp_path, "machines.json")) + with open(cpath, "w") as fh: + json.dump(conf, fh, sort_keys=True, indent=4) + with patch.dict(os.environ, {"MACHINES_CONFIG": cpath}): + with patch(f"{pbm}.MachineState", autospec=True): + cls: MachinesConfig = MachinesConfig() + machine = cls.get_machine("HAMMER") + assert machine is not None + assert machine.name == "hammer" diff --git a/tests/test_slack_handler.py b/tests/test_slack_handler.py index 0f44df9..22e075a 100644 --- a/tests/test_slack_handler.py +++ b/tests/test_slack_handler.py @@ -138,10 +138,18 @@ def test_init(self) -> None: call(token="btoken", signing_secret="secret"), call().event("app_mention"), call().event("app_mention")(self.cls.app_mention), + call().command("/oops-clear"), + call().command("/oops-clear")(self.cls.oops_clear_command), + call().view("oops_clear_modal"), + call().view("oops_clear_modal")(self.cls.oops_clear_modal_submit), ] assert self.slack_app.mock_calls == [ call.event("app_mention"), call.event("app_mention")(self.cls.app_mention), + call.command("/oops-clear"), + call.command("/oops-clear")(self.cls.oops_clear_command), + call.view("oops_clear_modal"), + call.view("oops_clear_modal")(self.cls.oops_clear_modal_submit), ] assert self.cls.app == self.slack_app @@ -734,6 +742,314 @@ async def test_handle_command_oops_by_alias(self, tmp_path) -> None: assert self.slack_app.mock_calls == [] assert mconf.machines_by_name["metal-mill"].state.is_oopsed is True + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_handle_command_oops_case_insensitive(self, tmp_path) -> None: + """Oops command matches machine name/alias case-insensitively.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + mconf: MachinesConfig = self.quart_app.config["MACHINES"] + mconf.machines_by_name["metal-mill"].state.is_oopsed = False + mconf.machines_by_name["metal-mill"].state.is_locked_out = False + msg = Message( + text="<@U12345678> oops MeTaL MiLl", + user_id="U5678", + user_name="User Name", + user_handle="displayName", + channel_id="Cadmin", + channel_name="AdminChannel", + ) + say = AsyncMock() + await self.cls.handle_command(msg, say) + assert say.mock_calls == [] + assert self.slack_client.mock_calls == [ + call.chat_postMessage( + channel="Cadmin", + text="Machine Metal Mill oopsed via Slack by unknown user.", + ), + call.chat_postMessage( + channel="Coops", text="Machine Metal Mill has been Oops'ed!" + ), + ] + assert self.slack_app.mock_calls == [] + assert mconf.machines_by_name["metal-mill"].state.is_oopsed is True + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_oops_clear_command_clears_oopsed(self, tmp_path) -> None: + """Direct /oops-clear form clears an oopsed machine and acks silently.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + mconf: MachinesConfig = self.quart_app.config["MACHINES"] + mconf.machines_by_name["metal-mill"].state.is_oopsed = True + mconf.machines_by_name["metal-mill"].state.is_locked_out = False + ack = AsyncMock() + client = AsyncMock() + command = {"channel_id": "Cadmin", "text": "metal-mill"} + await self.cls.oops_clear_command(ack, command, client) + assert ack.mock_calls == [call()] + assert self.slack_client.mock_calls == [ + call.chat_postMessage( + channel="Cadmin", text="Machine Metal Mill un-oopsed via Slack." + ), + call.chat_postMessage( + channel="Coops", text="Machine Metal Mill oops has been cleared." + ), + ] + assert mconf.machines_by_name["metal-mill"].state.is_oopsed is False + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_oops_clear_command_not_oopsed_or_locked(self, tmp_path) -> None: + """Direct /oops-clear form on a clear machine acks with a message.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + mconf: MachinesConfig = self.quart_app.config["MACHINES"] + mconf.machines_by_name["metal-mill"].state.is_oopsed = False + mconf.machines_by_name["metal-mill"].state.is_locked_out = False + ack = AsyncMock() + client = AsyncMock() + command = {"channel_id": "Cadmin", "text": "metal-mill"} + await self.cls.oops_clear_command(ack, command, client) + assert ack.mock_calls == [ + call("Machine Metal Mill is not oopsed or locked-out.") + ] + assert self.slack_client.mock_calls == [] + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_oops_clear_command_invalid_machine(self, tmp_path) -> None: + """Direct /oops-clear form with an unknown machine acks with an error.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + ack = AsyncMock() + client = AsyncMock() + command = {"channel_id": "Cadmin", "text": "invalid-name"} + await self.cls.oops_clear_command(ack, command, client) + assert ack.mock_calls == [ + call( + "Invalid machine name or alias 'invalid-name'. " + "Use status command to list all machines." + ) + ] + assert self.slack_client.mock_calls == [] + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_oops_clear_command_wrong_channel(self, tmp_path) -> None: + """/oops-clear from a non-control channel is rejected, no state change.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + mconf: MachinesConfig = self.quart_app.config["MACHINES"] + mconf.machines_by_name["metal-mill"].state.is_oopsed = True + mconf.machines_by_name["metal-mill"].state.is_locked_out = False + ack = AsyncMock() + client = AsyncMock() + command = {"channel_id": "Coops", "text": "metal-mill"} + await self.cls.oops_clear_command(ack, command, client) + assert ack.mock_calls == [ + call("The `/oops-clear` command can only be used in the control channel.") + ] + assert self.slack_client.mock_calls == [] + assert mconf.machines_by_name["metal-mill"].state.is_oopsed is True + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_oops_clear_command_case_insensitive(self, tmp_path) -> None: + """Direct /oops-clear form matches machine alias case-insensitively.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + mconf: MachinesConfig = self.quart_app.config["MACHINES"] + mconf.machines_by_name["metal-mill"].state.is_oopsed = True + mconf.machines_by_name["metal-mill"].state.is_locked_out = False + ack = AsyncMock() + client = AsyncMock() + command = {"channel_id": "Cadmin", "text": "MeTaL MiLl"} + await self.cls.oops_clear_command(ack, command, client) + assert ack.mock_calls == [call()] + assert self.slack_client.mock_calls == [ + call.chat_postMessage( + channel="Cadmin", text="Machine Metal Mill un-oopsed via Slack." + ), + call.chat_postMessage( + channel="Coops", text="Machine Metal Mill oops has been cleared." + ), + ] + assert mconf.machines_by_name["metal-mill"].state.is_oopsed is False + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_oops_clear_command_opens_modal(self, tmp_path) -> None: + """No-arg /oops-clear opens a modal of oopsed/locked machines.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + mconf: MachinesConfig = self.quart_app.config["MACHINES"] + # clear everything first, then mark two + for mach in mconf.machines: + mach.state.is_oopsed = False + mach.state.is_locked_out = False + mconf.machines_by_name["metal-mill"].state.is_oopsed = True + mconf.machines_by_name["hammer"].state.is_locked_out = True + ack = AsyncMock() + client = AsyncMock() + command = {"channel_id": "Cadmin", "text": "", "trigger_id": "T123"} + await self.cls.oops_clear_command(ack, command, client) + assert ack.mock_calls == [call()] + # sorted by display name (case-insensitive): hammer, then Metal Mill + expected_view = { + "type": "modal", + "callback_id": "oops_clear_modal", + "title": {"type": "plain_text", "text": "Clear Machine"}, + "submit": {"type": "plain_text", "text": "Clear"}, + "close": {"type": "plain_text", "text": "Cancel"}, + "blocks": [ + { + "type": "input", + "block_id": "machine_block", + "label": {"type": "plain_text", "text": "Machine to clear"}, + "element": { + "type": "static_select", + "action_id": "machine_select", + "placeholder": { + "type": "plain_text", + "text": "Select a machine", + }, + "options": [ + { + "text": {"type": "plain_text", "text": "hammer"}, + "value": "hammer", + }, + { + "text": {"type": "plain_text", "text": "Metal Mill"}, + "value": "metal-mill", + }, + ], + }, + } + ], + } + assert client.mock_calls == [ + call.views_open(trigger_id="T123", view=expected_view) + ] + # no default selection + element = expected_view["blocks"][0]["element"] + assert "initial_option" not in element + assert self.slack_client.mock_calls == [] + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_oops_clear_command_no_machines_to_clear(self, tmp_path) -> None: + """No-arg /oops-clear with nothing oopsed/locked acks, opens no modal.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + mconf: MachinesConfig = self.quart_app.config["MACHINES"] + for mach in mconf.machines: + mach.state.is_oopsed = False + mach.state.is_locked_out = False + ack = AsyncMock() + client = AsyncMock() + command = {"channel_id": "Cadmin", "text": "", "trigger_id": "T123"} + await self.cls.oops_clear_command(ack, command, client) + assert ack.mock_calls == [ + call("No machines are currently oopsed or locked out.") + ] + assert client.mock_calls == [] + assert self.slack_client.mock_calls == [] + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_oops_clear_modal_submit_clears(self, tmp_path) -> None: + """Modal submission clears the selected machine.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + mconf: MachinesConfig = self.quart_app.config["MACHINES"] + mconf.machines_by_name["metal-mill"].state.is_oopsed = True + mconf.machines_by_name["metal-mill"].state.is_locked_out = False + ack = AsyncMock() + view = { + "state": { + "values": { + "machine_block": { + "machine_select": {"selected_option": {"value": "metal-mill"}} + } + } + } + } + await self.cls.oops_clear_modal_submit(ack, view) + assert ack.mock_calls == [call()] + assert self.slack_client.mock_calls == [ + call.chat_postMessage( + channel="Cadmin", text="Machine Metal Mill un-oopsed via Slack." + ), + call.chat_postMessage( + channel="Coops", text="Machine Metal Mill oops has been cleared." + ), + ] + assert mconf.machines_by_name["metal-mill"].state.is_oopsed is False + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_oops_clear_modal_submit_already_clear(self, tmp_path) -> None: + """Modal submission for an already-clear machine is graceful.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + mconf: MachinesConfig = self.quart_app.config["MACHINES"] + mconf.machines_by_name["metal-mill"].state.is_oopsed = False + mconf.machines_by_name["metal-mill"].state.is_locked_out = False + ack = AsyncMock() + view = { + "state": { + "values": { + "machine_block": { + "machine_select": {"selected_option": {"value": "metal-mill"}} + } + } + } + } + await self.cls.oops_clear_modal_submit(ack, view) + assert ack.mock_calls == [call()] + assert self.slack_client.mock_calls == [] + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_oops_clear_modal_submit_no_selection(self, tmp_path) -> None: + """Modal submission with no selection is handled without error.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + ack = AsyncMock() + view = { + "state": { + "values": { + "machine_block": {"machine_select": {"selected_option": None}} + } + } + } + await self.cls.oops_clear_modal_submit(ack, view) + assert ack.mock_calls == [call()] + assert self.slack_client.mock_calls == [] + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_oops_clear_modal_submit_unknown_machine(self, tmp_path) -> None: + """Modal submission selecting an unknown machine is handled gracefully.""" + self.slack_app.reset_mock() + self.slack_client.reset_mock() + setup_machines(tmp_path, self) + ack = AsyncMock() + view = { + "state": { + "values": { + "machine_block": { + "machine_select": { + "selected_option": {"value": "ghost-machine"} + } + } + } + } + } + await self.cls.oops_clear_modal_submit(ack, view) + assert ack.mock_calls == [call()] + assert self.slack_client.mock_calls == [] + def setup_machines(fixture_dir: Path, test_class: TestSlackHandler) -> None: fpath: str = os.path.abspath(