Configure hawk-mod from Slack, not from a file on the host - #10
Merged
Conversation
The roster-row guard on `/hawkmod group` was a deadlock. Rows are created by the user group sync, which only creates them for people already in a role group — so the command whose job is putting somebody in their first group refused everybody who needed it. The only ways onto the roster were editing the group by hand in Slack, or the CSV import, which needs a shell on the host. An app whose first-run instruction is "SSH into the server" is broken, and this had quietly reintroduced that. The comment justifying the guard — that an edit should never be the first thing hawk-mod learns about someone — had it backwards. Joining a role group is *how* someone becomes known; that is what the sync's `create` decision is for. So the subject of an edit is now a Slack account, not a roster row. `GroupEditRequest.subject` already allowed this; only the command insisted otherwise. The write fires `subteam_members_changed`, the sync creates the row moments later, and the reply says so rather than leaving a caller to wonder why `whois` finds nothing yet. Unchanged: an unrostered person added to the adults group needs no written reason, because they have no role to lose. That is what the sync has always done with someone added to that group by hand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The adults group is @mentors in production. Nothing was broken by the placeholder — both handles come from STUDENT_USERGROUP/ADULT_USERGROUP at runtime — but every example, prompt and fallback said "adults", which is the wrong thing to type and the wrong thing to look for in Slack. Changed where the text names a handle somebody types: .env.example, the setup script's prompt, the README's example, the `?? "adults"` fallbacks, and the test fixtures, so the regression test around moving a student into the mentors group reads the way production does. Left alone where "adults" is the role rather than the group. The two-adult rule, `isAdult`, screened adults, and the guidance text are about people, not about what a Slack group is called. `domain/rules/rosterSync.ts` stays handle-agnostic too — it is handed two sets and never learns what anyone named them, which is what keeps it a pure rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Which user group declares students is a decision a Slack admin makes, and until now only an SSH session could change it. `authz.ts` already rejected that shape of problem once — "an app whose first-run instruction is 'SSH into the server' is broken" — and then it was left true of the settings that decide who is monitored. Four settings move: the student group, the mentor group, the editable-group allowlist, and the alert channel. `src/settings.ts` resolves each one database → environment → unset, so the env var becomes a seed rather than the source of truth. Nothing breaks for a running host and there is no flag day; a value set from Slack simply wins. `/hawkmod config` shows every value *and where it came from*. That second half is the point: "which group is it actually using, and who set it" is the question asked when the roster looks wrong, and it cost two rounds of guessing to answer before this existed. What cannot move is an allowlist, not an oversight. Slack credentials cannot be configured from Slack, and TOKEN_ENCRYPTION_KEY must never be reachable — changing it makes every stored token undecryptable and every enrolled adult invisible, with coverage still reporting 100%. A test asserts those keys stay unreachable. Handles are validated against Slack before they are stored, because a stored typo reads exactly like an empty group: nobody rostered, nobody monitored, no complaint. Changing a role group re-syncs immediately rather than at 3am, so the setting cannot look applied while it is not. Making the alert channel optional meant every caller had to face what an unset one means: a finding recorded and announced to nobody. It is an error at boot and an error at every use, naming the finding going unreported and the command that fixes it. `settings.ts` reads process.env directly rather than through `config()`, which would throw at import time in the CLI. Verified: the CLI still runs on an empty environment. 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.
Stacked on #9 — base is
manage-user-groups, so review that one first.Which user group declares students is a decision a Slack admin makes, and until now only an SSH session could change it.
src/slack/authz.tsalready rejected that shape of problem once:That principle got applied to admin authority and then left unapplied to the settings that decide who is monitored.
What moves
student-group,mentor-group,managed-groups,alert-channel.src/settings.tsresolves each database → environment → unset, so the env var becomes a seed rather than the source of truth. Nothing breaks for a running host and there is no flag day — a value set from Slack simply wins.Showing where each value came from is half the feature. "Which group is it actually using, and who set it" is the question asked when the roster looks wrong, and answering it previously took two rounds of guessing and an SSH session.
What cannot move, and why that's an allowlist rather than an oversight
Slack credentials can't be configured from Slack — you can't set the thing that lets hawk-mod reach Slack. And
TOKEN_ENCRYPTION_KEYmust never be reachable: changing it makes every stored token undecryptable and every enrolled adult invisible, with coverage still reporting 100%. A test asserts those keys stay unreachable, so widening the allowlist carelessly fails CI.Two safety properties worth reviewing closely
Handles are validated against Slack before storing. A stored typo reads exactly like an empty group: nobody rostered, nobody monitored, no complaint. The sweep would raise it eventually; refusing it turns tomorrow's finding into an error the person who caused it is still reading.
Changing a role group re-syncs immediately. Leaving it until 3am would mean the setting looked applied and was not.
The alert channel forced an honest reckoning
Making
ALERT_CHANNEL_IDoptional made the typechecker surface all five places that assumed a channel exists. An unset alert channel means a finding recorded and announced to nobody — the exact failure this project defines itself against. So it's an error at boot and an error at every use, naming the finding going unreported and the command that fixes it. It is never silent.Verified
env -i) — exits 0, migrations apply.settings.tsreadsprocess.envdirectly rather than throughconfig(), which would throw at import time and breakimport-roster/findings. This is the constraint CLAUDE.md is most emphatic about.env; aftersetSettingit readsslack; the editable-group allowlist widens;setting_changesrecords(unset) -> mentors by Ty.npm run typecheck && npm test && npm run format:check && npm run build— 94 tests, all green.Not in scope
Cron schedules and
TZstay in the environment. They're read once at boot to register jobs, so making them live means re-registering jobs on change — real work for something set once. Say the word if you want them too.🤖 Generated with Claude Code