Manage user group membership from /hawkmod - #9
Merged
Conversation
Adds `/hawkmod group add|remove @user @group`, and `/hawkmod deactivate` without which the first command would be lying. Group membership and monitoring are not the same fact, and this is the change that forces the distinction into the open. Removing someone from @Students leaves them a student on the roster — `reconcileRoles` is add-only, deliberately — so the command says so in its reply rather than letting the caller assume the roster followed. Ending monitoring is `/hawkmod deactivate`, which demands a person and a reason and is the only operation here that makes hawk-mod see less. CONTEXT.md names the two ideas so they stop sharing a word. The mirror of that asymmetry is new: a deactivated person who reappears in a role group is monitored again immediately, as a `reactivate` decision. Gaining protection never needs approval; losing it always does. Without this a student who rejoined the team next season would sit in @Students, known to hawk-mod, and unmonitored — with coverage reporting 100%, because inactive people leave both sides of that ratio. Writes go out on an administrator's own token, not the bot's. Slack accepts a bot token for usergroups.users.update only when the workspace lets *everyone* edit user groups, which §6 forbids; the restriction is the point, so the token is what changes. That token gets its own installation row: Slack issues one token per authorization carrying only that authorization's scopes, so sharing a row would mean an administrator who is also an enrolled mentor losing their DM token the first time they authorized group editing. `saveInstallation` now refuses a `user` write whose scopes cannot read DMs, because nothing else in the system would ever have noticed. Edits go through a pure plan. usergroups.users.update replaces a group's entire member list, so a bad input does not corrupt a group, it empties one — and the eventual spreadsheet-driven sync is exactly the shape of input that fails that way quietly. The plan carries its own refusal so a caller cannot apply a bad one by forgetting a flag, and the single-user command is the degenerate case of that sync rather than a separate path. group_changes records who asked. role_changes cannot: it is written by the sync, which runs from a Slack event long after the human is gone. Requires `usergroups:write` as a *user* scope in the Slack app manifest. Bot scopes are unchanged, so no workspace reinstall. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Adds
/hawkmod group add|remove @user @group, and/hawkmod deactivatewithout which the first command would be lying.Built as the reusable half of the user-triggered Google Sheet → user groups sync, so the single-user command is the degenerate case of that sync rather than a separate path.
Group membership is a declaration; the roster is monitoring
These are not the same fact, and this change forces the distinction into the open.
Removing someone from
@studentsleaves them a student on the roster —reconcileRolesis add-only, deliberately — so the command says so in its reply rather than letting the caller assume the roster followed. Ending monitoring is/hawkmod deactivate, which demands a person and a reason, and is the only operation here that makes hawk-mod see less.CONTEXT.md(new) names the two ideas so they stop sharing a word.Coming back is automatic; leaving is not
A deactivated person who reappears in a role group is monitored again immediately, as a new
reactivatedecision. Gaining protection never needs approval; losing it always does.Without this, a student who rejoined the team next season would sit in
@students, known to hawk-mod, and unmonitored — with/hawkmod statusreporting 100% coverage, because inactive people leave both sides of that ratio. An absent alert, not a wrong one.This edits
rosterSync.ts, which CLAUDE.md guards most explicitly. It upholds the invariant rather than breaking it — the ban is on the sync subtracting monitoring, and this only ever adds — but the comment now says what the rule actually protects.Why an admin's token and not the bot's
Slack accepts a bot token for
usergroups.users.updateonly when the workspace lets everyone edit user groups. §6 forbids that. The restriction is the point, so the token is what changes — which also means Slack attributes each change to a real person.That token gets its own installation row. Slack issues one token per authorization carrying only that authorization's scopes, so sharing a row would mean an administrator who is also an enrolled mentor losing their DM token the first time they authorized group editing — silently, with coverage still reading 100%.
saveInstallationnow refuses auserwrite whose scopes cannot read DMs, because nothing else in the system would ever have noticed.Every edit goes through a plan
usergroups.users.updatereplaces a group's entire member list — there is no add-one endpoint. So a bad input does not corrupt a group, it empties one, and a spreadsheet is exactly the shape of input that fails that way quietly.domain/rules/groupMembership.tsis pure, diffs desired against actual, and carries its own refusal so a caller cannot apply a bad plan by forgetting to check a flag elsewhere.Editable groups are an allowlist (
MANAGED_USERGROUPS) — blast radius, not authorization: every caller is already a Slack admin who could edit any group by hand.group_changesrecords who asked.role_changescannot: it is written by the sync, which runs from a Slack event long after the human is gone, and most group edits will change no role at all once subteams are managed here.A bug found and fixed mid-build
The required-reason gate on moving a student into
@adultswas dead code as first written. It compared the raw slash-command argument to the configured handle, but the manifest setsshould_escape: true, so Slack sends<!subteam^S0614TY5A|adults>. An opaque id never equals"adults", so the gate would have silently never fired and a student could have been moved by typo with nothing recorded.Moved behind group resolution, where the handle is known, and pinned with a regression test asserting a group id does not match.
Verified, not assumed
0006, confirmed every row and the UNIQUE index survived theinstallationstable rebuild.generateInstallUrl— probed Bolt at runtime; produces a valid URL with emptyscopeanduser_scope=usergroups:write.npm run typecheck && npm test && npm run format:check && npm run build— 83 tests, all green.Before merging changes anything in the workspace
docs/slack-app-manifest.yamlnow listsusergroups:writeunder user scopes. Until that lands at api.slack.com the authorize link is rejected before anyone sees a consent screen. Bot scopes are unchanged, so no workspace reinstall.Deliberately not in scope
Bulk plans are meant to preview before applying, but there is no bulk caller yet, so there is nothing to preview. The threshold refusal is implemented and tested; the preview-and-confirm UI belongs with the Sheet sync that needs it. That sync should be a thin caller: read the sheet, build a
desiredset, hand it toplanGroupMembership.🤖 Generated with Claude Code