Fixes: @students lookup, channel display, and group-editing authorization - #11
Merged
Conversation
Resolution decided up front whether a reference was a handle or a Slack id, using a pattern for ids: `S` followed by alphanumerics. `students` uppercased is `STUDENTS` — an `S` and seven more characters — so the single most important handle in this project was read as an opaque id, matched against no group, and reported as a group that does not exist. That broke `/hawkmod group`, and worse, it broke `syncRolesFromUserGroups`: with STUDENT_USERGROUP=students the sync could not resolve the group, so nobody was rostered as a student and nobody was monitored as one. It reported this honestly as a `workspace_config` violation saying the group did not exist, which was true of the lookup and false of the workspace. It went unnoticed because the test workspace used `hawkmod-test-students`, which does not begin with `s` followed by alphanumerics alone. `mentors` was never affected either. Only the handles that happen to look like ids — students, staff, seniors — and one of those is the one that matters. Matching now tries handle *or* id and never guesses which it was given. The two cannot realistically collide: a handle would have to be spelled exactly like some other group's id. There is nothing to gain by telling them apart, and this cannot fail the way guessing did. A failed lookup now logs the handles that do exist, which is how this should have been found in the first place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`/hawkmod config` reported the alert channel as C0BPAV78LKZ, which tells a reader nothing about which channel it is — the whole point of the listing being to answer "what is it actually using". Channels stay stored by id. That part was right: an id survives the channel being renamed, where a stored `#name` would quietly stop resolving the day somebody tidied it up. So the id is kept and the name is shown, resolved at display time, falling back to the raw value if Slack cannot be asked — a settings listing that throws is worse than one that is briefly ugly. User groups render as @handle for the same reason, and the confirmation after a change now describes both the new value and the old one. 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.
Two fixes, one of which is a live monitoring outage.
@studentsnever resolved, so no student was rosteredresolveGroupdecided up front whether a reference was a handle or a Slack id, using a pattern for ids:Sfollowed by alphanumerics.studentsuppercased isSTUDENTS— anSand seven more characters. So the single most important handle in this project was read as an opaque id, matched against no group, and reported as a group that does not exist.That broke
/hawkmod config set student-group @studentsand/hawkmod group. Worse, it brokesyncRolesFromUserGroups: with the student group set tostudents, the sync could not resolve it, so nobody was rostered as a student and nobody was monitored as one. It reported this as aworkspace_configviolation saying the group did not exist — true of the lookup, false of the workspace.It went unnoticed because the test workspace used
hawkmod-test-students, which doesn't begin withSfollowed only by alphanumerics.mentorswas never affected either. Only handles that happen to look like ids —students,staff,seniors— and one of those is the one that matters.Matching now tries handle or id and never guesses which it was given. The two can't realistically collide: a handle would have to be spelled exactly like some other group's id. There's nothing to gain by telling them apart, and this can't fail the way guessing did.
A failed lookup now logs the handles that do exist, which is how this should have been found in the first place.
Channel names instead of raw ids
/hawkmod configreported the alert channel asC0BPAV78LKZ, which tells a reader nothing — and the whole point of that listing is answering "what is it actually using".Channels stay stored by id. That part was right: an id survives the channel being renamed, where a stored
#namewould quietly stop resolving the day somebody tidied it up. So the id is kept and the name is resolved at display time, falling back to the raw value if Slack can't be asked — a settings listing that throws is worse than one that's briefly ugly.User groups render as
@handlefor the same reason, and the confirmation after a change now describes both the new value and the old one.Tests
Nine cases pinning
matchesGroup, including the regression explicitly:npm run typecheck && npm test && npm run format:check && npm run build— 103 tests, all green.After merging
/hawkmod config set student-group @studentswill work. If the role sync has been failing, there should be an openworkspace_configfinding claiming the group doesn't exist — that will clear on the next sync, and/hawkmod syncforces it immediately.Worth checking
/hawkmod statusafterwards: if no student has been rostered while this was broken, the roster will fill in on the first successful sync.🤖 Generated with Claude Code