Add /oops-clear Slack slash command and case-insensitive machine matching#155
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new /oops-clear Slack slash command (direct and modal-driven forms) for clearing Oops/maintenance lock-out states, and updates machine name/alias resolution to be case-insensitive via MachinesConfig.get_machine(). This improves operator UX while keeping existing at-mention behavior aligned with the new command.
Changes:
- Implement
/oops-clearwith control-channel gating, a no-arg modal flow, and shared clear/validation helpers inSlackHandler. - Make
MachinesConfig.get_machine()resolve machine names and aliases case-insensitively via lowercased lookup maps. - Extend test coverage for new Slack flows and case-insensitive matching; update Slack setup/usage documentation accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_slack_handler.py | Adds unit tests for /oops-clear (direct + modal) and for case-insensitive at-mention command matching; updates init registration expectations. |
| tests/models/test_machine.py | Adds tests ensuring get_machine() matches both names and aliases regardless of case. |
| src/dm_mac/slack_handler.py | Registers and implements /oops-clear plus modal submission handling; refactors clear logic into reusable helpers; updates help text. |
| src/dm_mac/models/machine.py | Adds lowercase lookup dicts and updates get_machine() to be case-insensitive. |
| docs/source/slack.rst | Documents new commands scope, interactivity enablement, slash command setup steps, and /oops-clear usage. |
| docs/features/completed/slack-slack-command.md | Adds the completed feature design/plan doc describing the implementation and progress. |
| CLAUDE.md | Updates Slack lookup note to mention case-insensitive matching and /oops-clear. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
…mmand Write the implementation plan into the slack-slack-command feature document. The plan covers a /oops-clear slash command (direct `/oops-clear <machine>` 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) <noreply@anthropic.com>
…ensitive 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…l 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
jantman
force-pushed
the
slack-slash-command
branch
from
00e6058 to
de14e0e
Compare
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
/oops-clearSlack slash command for clearing machines (Oops / maintenance lock-out) and makes all machine name/alias matching case-insensitive.Implements the feature described in
docs/features/completed/slack-slack-command.md.1.
/oops-clearslash commandRestricted to the control channel (
SLACK_CONTROL_CHANNEL_ID); invocations elsewhere get an ephemeral rejection./oops-clear <machine name>— clears the named machine (by name or alias), equivalent to the existingclearat-mention. Silent ack on success (the existing oops/control channel posts cover the outcome); ephemeral messages for invalid names or already-clear machines./oops-clear(no argument) — opens a Block Kit modal with a single dropdown (static_select, no default) listing all currently Oopsed or locked-out machines. Selecting one and submitting clears it. If nothing is Oopsed/locked-out, an ephemeral notice is shown instead.2. Case-insensitive matching
MachinesConfig.get_machine()now resolves names and aliases case-insensitively, which applies to both the new slash command and the existing@mentioncommands (both funnel through that method).Implementation notes
_clear_machine()/_invalid_machine_msg()helpers; the existing mentionclear()was refactored onto them (no behavior change).slack-boltAsyncAppover Socket Mode.Docs
docs/source/slack.rst: setup steps to create the slash command + enable Interactivity (no Request URL under Socket Mode), thecommandsBot Token Scope, and usage for both/oops-clearforms plus a case-insensitivity note.HELP_RESPONSEand the CLAUDE.md Slack-lookup note.Testing
nox -s tests(309 passing),pre-commit,mypy,typeguard,docsall pass.slack_handler.pyat 100% coverage.nox -s safetyfails only on pre-existing upstream CVEs in transitive dependencies (aiohttp,dulwich,idna). This branch changes no dependencies (pyproject.toml/poetry.lockuntouched), so it is unrelated to this feature and best handled as a separate dependency bump.🤖 Generated with Claude Code