From 21051894200280bb109293ecaab45421b85b4abb Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Thu, 11 Jun 2026 17:41:33 -0400 Subject: [PATCH 01/11] slack slash command --- docs/features/slack-slack-command.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 docs/features/slack-slack-command.md diff --git a/docs/features/slack-slack-command.md b/docs/features/slack-slack-command.md new file mode 100644 index 0000000..d1c72c0 --- /dev/null +++ b/docs/features/slack-slack-command.md @@ -0,0 +1,10 @@ +# 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. From 6dd48672c60050829f93c4949b9da52d2181b480 Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Thu, 11 Jun 2026 19:01:16 -0400 Subject: [PATCH 02/11] docs(feature): add implementation plan for Slack /oops-clear slash command Write the implementation plan into the slack-slack-command feature document. The plan covers a /oops-clear slash command (direct `/oops-clear ` form and a Block Kit modal form) restricted to the control channel, plus case-insensitive machine name/alias matching for both the slash command and existing @mention commands. Organized into four milestones: case-insensitive get_machine(), the direct slash-command form, the interactive modal, and acceptance criteria. Records two confirmed decisions: silent ack() on successful clear (no ephemeral confirmation) and keeping the generic "Slack" source label without Machine model changes. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/slack-slack-command.md | 209 +++++++++++++++++++++++++++ 1 file changed, 209 insertions(+) diff --git a/docs/features/slack-slack-command.md b/docs/features/slack-slack-command.md index d1c72c0..1150a65 100644 --- a/docs/features/slack-slack-command.md +++ b/docs/features/slack-slack-command.md @@ -8,3 +8,212 @@ We need to improve the user experience for clearing machines (oops or maintenanc 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; awaiting human approval before implementation. + + From 071fd55273645c8b222bf00e67fa5be6ac96c6c5 Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Thu, 11 Jun 2026 19:03:37 -0400 Subject: [PATCH 03/11] slack-slash-command - 1.1: make MachinesConfig.get_machine() case-insensitive Add lowercased name/alias lookup maps (machines_by_name_lower, machines_by_alias_lower) populated alongside the existing exact-match maps, and change get_machine() to resolve name_or_alias.lower() against them. This makes machine-name matching case-insensitive for every caller of get_machine(), including the existing Slack @mention commands and the forthcoming /oops-clear slash command. The original exact-match maps are left intact for existing callers. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/dm_mac/models/machine.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/dm_mac/models/machine.py b/src/dm_mac/models/machine.py index b5d7e3b..262a849 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,17 @@ 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 + self.machines_by_alias_lower[mach.alias.lower()] = 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]]: From 859b3c746cfdcceb1d40cbdb2779b33f4ce65ccf Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Thu, 11 Jun 2026 19:05:15 -0400 Subject: [PATCH 04/11] slack-slash-command - 1.2: tests for case-insensitive machine matching Add unit tests covering MachinesConfig.get_machine() resolving machine names and aliases regardless of case, plus a SlackHandler test asserting the @mention oops command matches a mixed-case machine alias ("MeTaL MiLl"). Update the feature document Milestone 1 progress. All nox tests passing. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/slack-slack-command.md | 7 ++++- tests/models/test_machine.py | 39 ++++++++++++++++++++++++++++ tests/test_slack_handler.py | 32 +++++++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/docs/features/slack-slack-command.md b/docs/features/slack-slack-command.md index 1150a65..7e7c0b4 100644 --- a/docs/features/slack-slack-command.md +++ b/docs/features/slack-slack-command.md @@ -214,6 +214,11 @@ Key files to modify: ## Progress -- Planning complete; awaiting human approval before implementation. +- 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. diff --git a/tests/models/test_machine.py b/tests/models/test_machine.py index 2f799f7..add605e 100644 --- a/tests/models/test_machine.py +++ b/tests/models/test_machine.py @@ -235,3 +235,42 @@ 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" diff --git a/tests/test_slack_handler.py b/tests/test_slack_handler.py index 0f44df9..59f65ad 100644 --- a/tests/test_slack_handler.py +++ b/tests/test_slack_handler.py @@ -734,6 +734,38 @@ 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 + def setup_machines(fixture_dir: Path, test_class: TestSlackHandler) -> None: fpath: str = os.path.abspath( From 936cb35a766d5a4c22ed3851a988936e60864a01 Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Thu, 11 Jun 2026 19:06:58 -0400 Subject: [PATCH 05/11] slack-slash-command - 2.1: extract shared clear/invalid-machine helpers Refactor SlackHandler.clear() to delegate to two new reusable helpers that the forthcoming /oops-clear slash command and modal will share: - _invalid_machine_msg(name): the standard "invalid machine name or alias" message. - _clear_machine(mach): clears oops and/or lock-out, returning None when something was cleared (channel posts cover it) or a message string when the machine was already clear. Pure refactor with no behavior change; existing SlackHandler tests pass unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/dm_mac/slack_handler.py | 42 ++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/src/dm_mac/slack_handler.py b/src/dm_mac/slack_handler.py index c56d520..3a3d4dd 100644 --- a/src/dm_mac/slack_handler.py +++ b/src/dm_mac/slack_handler.py @@ -235,17 +235,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 +259,20 @@ 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) @staticmethod def _both_relays_suffix(machine: Machine) -> str: From 694d56b633ff6802acb066eb03cabf30183a7d91 Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Thu, 11 Jun 2026 19:09:09 -0400 Subject: [PATCH 06/11] slack-slash-command - 2.2: implement /oops-clear direct form + channel gating Register the /oops-clear slash command and implement its direct form in SlackHandler.oops_clear_command(). The command is restricted to the control channel (SLACK_CONTROL_CHANNEL_ID); invocations elsewhere get an ephemeral rejection. With a machine name/alias argument it resolves the machine (case-insensitively) and clears it via the shared _clear_machine helper: invalid names and already-clear machines get an ephemeral message, while a successful clear acks silently (the existing oops/ control channel posts cover the outcome). When no argument is given it currently acks with a usage hint; the Block Kit selection modal replaces this in Milestone 3. Update test_init to assert the new command registration. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/dm_mac/slack_handler.py | 41 +++++++++++++++++++++++++++++++++++++ tests/test_slack_handler.py | 4 ++++ 2 files changed, 45 insertions(+) diff --git a/src/dm_mac/slack_handler.py b/src/dm_mac/slack_handler.py index 3a3d4dd..6b86028 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 @@ -84,6 +86,7 @@ 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) logger.debug("SlackHandler initialized.") async def app_mention(self, body: Dict[str, Any], say: AsyncSay) -> None: @@ -274,6 +277,44 @@ async def clear(self, msg: Message, say: AsyncSay) -> None: 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 (see Milestone 3). ``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 the selection modal (Milestone 3). + await ack("Please specify a machine name: `/oops-clear `") + @staticmethod def _both_relays_suffix(machine: Machine) -> str: """Return ' (both relays)' if machine has second_relay, else empty.""" diff --git a/tests/test_slack_handler.py b/tests/test_slack_handler.py index 59f65ad..759eef0 100644 --- a/tests/test_slack_handler.py +++ b/tests/test_slack_handler.py @@ -138,10 +138,14 @@ 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), ] 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), ] assert self.cls.app == self.slack_app From 4bcaa623a570992bfec884051d58f709d8047f3c Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Thu, 11 Jun 2026 19:11:14 -0400 Subject: [PATCH 07/11] slack-slash-command - 2.3: tests for /oops-clear direct form Add SlackHandler.oops_clear_command() direct-form tests covering: clears an oopsed machine with a silent ack, acks with a message when the machine is already clear, rejects an invalid machine name, rejects invocation from a non-control channel without changing state, and matches the machine alias case-insensitively. Update the feature document Milestone 2 progress. All 28 slack handler tests passing. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/slack-slack-command.md | 9 +++ tests/test_slack_handler.py | 103 +++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/docs/features/slack-slack-command.md b/docs/features/slack-slack-command.md index 7e7c0b4..74f217f 100644 --- a/docs/features/slack-slack-command.md +++ b/docs/features/slack-slack-command.md @@ -220,5 +220,14 @@ Key files to modify: 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. diff --git a/tests/test_slack_handler.py b/tests/test_slack_handler.py index 759eef0..7242882 100644 --- a/tests/test_slack_handler.py +++ b/tests/test_slack_handler.py @@ -770,6 +770,109 @@ async def test_handle_command_oops_case_insensitive(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_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 + def setup_machines(fixture_dir: Path, test_class: TestSlackHandler) -> None: fpath: str = os.path.abspath( From 39afbed83db2147fea6a7c53cf14aa5782a0f63b Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Thu, 11 Jun 2026 19:13:55 -0400 Subject: [PATCH 08/11] slack-slash-command - 3.1/3.2: /oops-clear Block Kit selection modal Implement the no-argument /oops-clear flow. When invoked with no machine name, oops_clear_command() gathers machines that are currently oopsed or locked out; if none, it acks with an ephemeral notice, otherwise it opens a Block Kit modal (built by _build_clear_modal) whose single required static_select lists those machines (option text = display name, value = machine name) with no default selection. Register and implement the view-submission handler oops_clear_modal_submit(), keyed on the MODAL_CALLBACK_ID ("oops_clear_modal"): it acks to close the modal, extracts the selected machine, and clears it via the shared _clear_machine helper. A machine cleared between modal open and submit, or a missing selection, is handled gracefully. Modal block/action/callback ids are class constants. test_init updated to assert the view registration. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/dm_mac/slack_handler.py | 87 ++++++++++++++++++++++++++++++++++++- tests/test_slack_handler.py | 4 ++ 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/dm_mac/slack_handler.py b/src/dm_mac/slack_handler.py index 6b86028..3bdc60a 100644 --- a/src/dm_mac/slack_handler.py +++ b/src/dm_mac/slack_handler.py @@ -64,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: @@ -87,6 +95,7 @@ def __init__(self, quart_app: Quart): ) 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: @@ -312,8 +321,82 @@ async def oops_clear_command( else: await ack() return - # No machine specified: open the selection modal (Milestone 3). - await ack("Please specify a machine name: `/oops-clear `") + # 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/test_slack_handler.py b/tests/test_slack_handler.py index 7242882..19c32b6 100644 --- a/tests/test_slack_handler.py +++ b/tests/test_slack_handler.py @@ -140,12 +140,16 @@ def test_init(self) -> None: 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 From 7dbe6a11a53952b1fc232d7885dd5dd159a5e4df Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Thu, 11 Jun 2026 19:16:27 -0400 Subject: [PATCH 09/11] slack-slash-command - 3.3: tests for /oops-clear modal flow Add tests covering the Block Kit modal flow: no-arg command opens a modal whose options exactly match the oopsed/locked machines (sorted by display name, text = display name, value = name, no initial_option); no-arg command with nothing oopsed/locked acks ephemerally and opens no modal; modal submission clears the selected machine; and submission for an already-clear machine, with no selection, or for an unknown machine is handled gracefully. slack_handler.py is now at 100% test coverage. Update the feature document Milestone 3 progress. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/slack-slack-command.md | 9 ++ tests/test_slack_handler.py | 173 +++++++++++++++++++++++++++ 2 files changed, 182 insertions(+) diff --git a/docs/features/slack-slack-command.md b/docs/features/slack-slack-command.md index 74f217f..e7e3e8a 100644 --- a/docs/features/slack-slack-command.md +++ b/docs/features/slack-slack-command.md @@ -229,5 +229,14 @@ Key files to modify: 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. diff --git a/tests/test_slack_handler.py b/tests/test_slack_handler.py index 19c32b6..22e075a 100644 --- a/tests/test_slack_handler.py +++ b/tests/test_slack_handler.py @@ -877,6 +877,179 @@ async def test_oops_clear_command_case_insensitive(self, tmp_path) -> None: ] 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( From 4de7d86870735868dd0ec2b04811e82baa0f1517 Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Thu, 11 Jun 2026 19:22:39 -0400 Subject: [PATCH 10/11] slack-slash-command - 4.x: docs, HELP_RESPONSE, finalize feature Acceptance-criteria milestone for the /oops-clear slash command and case-insensitive machine matching feature: - docs/source/slack.rst: document creating the /oops-clear slash command and enabling Interactivity (no Request URL needed under Socket Mode), add the "commands" Bot Token Scope, and add a usage section for both /oops-clear forms plus a note that name/alias matching is case-insensitive. - slack_handler.py: extend HELP_RESPONSE to mention case-insensitive matching and the /oops-clear slash command. - CLAUDE.md: note that Slack machine lookups are case-insensitive and cover both at-mentions and the slash command. - Move the feature document to docs/features/completed/. nox tests (309), pre-commit, mypy, typeguard, and docs all pass. The safety session fails only on pre-existing upstream CVEs (aiohttp, dulwich, idna) in transitive deps; this branch changes no dependencies, so it is unrelated to this feature. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 2 +- .../{ => completed}/slack-slack-command.md | 13 +++++++++++++ docs/source/slack.rst | 15 ++++++++++++++- src/dm_mac/slack_handler.py | 4 ++++ 4 files changed, 32 insertions(+), 2 deletions(-) rename docs/features/{ => completed}/slack-slack-command.md (94%) 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/slack-slack-command.md b/docs/features/completed/slack-slack-command.md similarity index 94% rename from docs/features/slack-slack-command.md rename to docs/features/completed/slack-slack-command.md index e7e3e8a..6e5705e 100644 --- a/docs/features/slack-slack-command.md +++ b/docs/features/completed/slack-slack-command.md @@ -238,5 +238,18 @@ Key files to modify: - 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..39533d1 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 dialog 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/slack_handler.py b/src/dm_mac/slack_handler.py index 3bdc60a..ed28065 100644 --- a/src/dm_mac/slack_handler.py +++ b/src/dm_mac/slack_handler.py @@ -80,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() From de14e0ed3116ea56aaacf4648d1352ee51f8cf1a Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Fri, 12 Jun 2026 17:04:50 -0400 Subject: [PATCH 11/11] slack-slash-command: address PR review comments Address Copilot review feedback on PR #155: - machine.py: detect case-insensitive collisions when building the lowercased alias lookup map and raise a clear ValueError at load instead of silently overwriting an entry. The alias map is now populated in a second pass after all machine names are known, so a collision (alias vs. alias, or alias vs. another machine's name) is detected regardless of config order. An alias equal to the machine's own name remains allowed. Add tests for both collision cases and the allowed self-alias case. - slack_handler.py: drop the internal "see Milestone 3" planning reference from oops_clear_command's docstring and tidy the wrapping. - docs/features/completed/slack-slack-command.md: fix the relative link to the feature README (../README.md now that the doc lives in completed/) and remove stray tool-output artifact lines at the end. - docs/source/slack.rst: refer to the Block Kit "modal" rather than "dialog" for consistency. All nox sessions (tests, pre-commit, mypy, docs) pass; safety now also passes after the main-branch dependency refresh. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../features/completed/slack-slack-command.md | 4 +- docs/source/slack.rst | 2 +- src/dm_mac/models/machine.py | 21 +++++++- src/dm_mac/slack_handler.py | 12 ++--- tests/models/test_machine.py | 49 +++++++++++++++++++ 5 files changed, 77 insertions(+), 11 deletions(-) diff --git a/docs/features/completed/slack-slack-command.md b/docs/features/completed/slack-slack-command.md index 6e5705e..a0e8741 100644 --- a/docs/features/completed/slack-slack-command.md +++ b/docs/features/completed/slack-slack-command.md @@ -1,6 +1,6 @@ # Slack Slash Command and Case-Insensitive Search -You must read, understand, and follow all instructions in `./README.md` when planning and implementing this feature. +You must read, understand, and follow all instructions in `../README.md` when planning and implementing this feature. ## Overview @@ -251,5 +251,3 @@ Key files to modify: - 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 39533d1..d6334a6 100644 --- a/docs/source/slack.rst +++ b/docs/source/slack.rst @@ -65,7 +65,7 @@ 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 dialog 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. +* ``/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. diff --git a/src/dm_mac/models/machine.py b/src/dm_mac/models/machine.py index 262a849..6131e0b 100644 --- a/src/dm_mac/models/machine.py +++ b/src/dm_mac/models/machine.py @@ -359,7 +359,26 @@ def __init__(self) -> None: self.machines_by_name_lower[mach.name.lower()] = mach if mach.alias: self.machines_by_alias[mach.alias] = mach - self.machines_by_alias_lower[mach.alias.lower()] = 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]: diff --git a/src/dm_mac/slack_handler.py b/src/dm_mac/slack_handler.py index ed28065..244bcbb 100644 --- a/src/dm_mac/slack_handler.py +++ b/src/dm_mac/slack_handler.py @@ -296,12 +296,12 @@ async def oops_clear_command( """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 (see Milestone 3). ``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). + (``/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: diff --git a/tests/models/test_machine.py b/tests/models/test_machine.py index add605e..751b255 100644 --- a/tests/models/test_machine.py +++ b/tests/models/test_machine.py @@ -274,3 +274,52 @@ def test_get_machine_case_insensitive_alias( 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"