fix(notifications): skip squad assignees in subscriber + inbox listeners#4323
Draft
bold84 wants to merge 1 commit into
Draft
fix(notifications): skip squad assignees in subscriber + inbox listeners#4323bold84 wants to merge 1 commit into
bold84 wants to merge 1 commit into
Conversation
When an issue is created or reassigned with assignee_type='squad', the
subscriber and notification listeners were calling AddIssueSubscriber and
notifyDirect with user_type='squad' / recipient_type='squad'. Both tables
are CHECK-constrained to ('member','agent'), so every squad assignment
logged two SQLSTATE 23514 errors:
ERR failed to add issue subscriber ... user_type=squad ... issue_subscriber_user_type_check
ERR direct notification creation failed ... inbox_item_recipient_type_check
The squad leader is already dispatched via EnqueueTaskForSquadLeader
(service/autopilot.go), so a subscriber row and an inbox item are both
impossible and redundant.
Mirrors the author_type=='system' gate added in MUL-2538
(TestSubscriberSystemCommentDoesNotSubscribe /
TestNotification_SystemCommentSkipsInboxAndMentions) — same shape of bug,
same boundary-level fix. The CLI-side resolution guard for the sibling
'project.lead_type' CHECK landed in MUL-2165; this closes the remaining
server-side gap between MUL-2165 and MUL-2538.
Changes:
- subscriber_listeners.go: gate the assignee branch in issue:created and
issue:updated on *AssigneeType != "squad".
- notification_listeners.go: same gate on the two notifyDirect call sites
for issue_assigned.
- tests: createTestSquad helper + three regression tests pinning the
invariant. The behavioral assertions hold either way (the inserts fail
on the CHECK), but the tests document the contract and will fail loudly
if anyone widens the CHECK constraints without revisiting these
listeners — same tradeoff as the MUL-2538 tests.
|
@bold84 is attempting to deploy a commit to the IndexLabs Team on Vercel. A member of the Team first needs to authorize it. |
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.
Problem
When an issue is created or reassigned with
assignee_type='squad', the subscriber and notification listeners callAddIssueSubscriberandnotifyDirectwithuser_type='squad'/recipient_type='squad'. Bothissue_subscriber.user_typeandinbox_item.recipient_typeare CHECK-constrained to('member','agent'), so every squad assignment logs twoSQLSTATE 23514errors in production:The squad leader is already dispatched via
EnqueueTaskForSquadLeader(service/autopilot.go), so a subscriber row and an inbox item are both impossible and redundant — the squad has no inbox to read.Context
This is the remaining server-side gap between two existing fixes:
squadresolution forproject.lead_typeand the subscriber handler (isWorkspaceEntity), but the server-side event-bus listeners were not touched. Theissue:created/issue:updatedlisteners kept forwarding*AssigneeTypestraight intoAddIssueSubscriber.author_type == "system"early-return insubscriber_listeners.goandnotification_listeners.gofor the child-done system comment, with an explicit comment noting "issue_subscriber.user_type is constrained to (member,agent); without the gate the listener would log a constraint violation." The squad-assignee case — the same shape of bug — was left open.This PR closes that gap using the same boundary-level gate pattern the team has applied both times.
Changes
subscriber_listeners.go: gate the assignee branch in theissue:createdandissue:updatedlisteners on*AssigneeType != "squad".notification_listeners.go: same gate on the twonotifyDirectcall sites that produceissue_assigned.createTestSquadhelper + 3 regression tests (TestSubscriberIssueCreated_SquadAssigneeNotSubscribed,TestSubscriberIssueUpdated_SquadAssigneeNotSubscribed,TestNotification_SquadAssigneeSkipsInbox) mirroring the MUL-2538 system-comment test pair.Verification
go build ./cmd/server/andgo vet ./cmd/server/clean.go test -race -run 'TestSubscriber|TestNotification' ./cmd/server/— all 30 tests pass (3 new + existing subscriber/notification suite).SQLSTATE 23514on both tables); with the fix applied, those errors are gone.A note on the regression-test shape
The new tests are invariant guards — they pass both with and without the source fix because the DB rejects the bad insert either way (the listener swallows the error and continues). This is the same tradeoff the MUL-2538 tests accepted (
TestSubscriberSystemCommentDoesNotSubscribe,TestNotification_SystemCommentSkipsInboxAndMentions). The value of these tests is:squadwithout revisiting these listeners — preventing a silent regression where squad rows start getting inserted with the old listener logic.I kept the style consistent with the existing MUL-2538 tests rather than introducing slog-output capture, which would be inconsistent with the package's testing conventions. Happy to switch to stricter output-capture tests if the team prefers.