#4412 Slack Delayed Meeting Messages by Santiordon · Pull Request #4541 · Northeastern-Electric-Racing/FinishLine

Santiordon · 2026-09-25T00:41:18Z

Changes

Slack messages now split into daily and hourly notifications.
Hourly activates event reminder notifications which sends a reminder to teams 48, 24, and 1 hour before the event.

Notes

Any other notes go here

Test Cases

Postman test to make sure events only fire once on 48, 24 and 1 hour intervals
Tests to make sure teams are collected correctly

Checklist

It can be helpful to check the Checks and Files changed tabs.
Please review the contributor guide and reach out to your Tech Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.

Closes #4412 (issue #)

Copilot

Copilot review overview

🟡 Changes recommended

Slack delivery can be reported successful before completion, and failed sends can permanently lose reminders.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 · 1

Open (3)

Prevent failed Slack sends from permanently claiming reminders · New
Await Slack sends and avoid duplicate cumulative messages · New
Test tier boundaries and exactly-once retry behavior · New

What changed in this PR

Adds separate daily/hourly Slack workflows with persistent reminders at 48, 24, and 1 hour before events.

Changes:

Adds reminder tiers, persistence, routes, controllers, and hourly scheduling.
Updates notification helpers and daily workflow.
Adds channel collection tests.

File	Description
`src/backend/tests/unit/notifications.test.ts`	Tests channel collection helpers.
`src/backend/src/utils/notifications.utils.ts`	Adds reminder and channel helpers.
`src/backend/src/transformers/notifications.transformer.ts`	Defines reminder query relations.
`src/backend/src/services/notifications.services.ts`	Implements hourly event reminders.
`src/backend/src/routes/notifications.routes.ts`	Adds daily and hourly routes.
`src/backend/src/prisma/schema.prisma`	Adds reminder models and tiers.
`src/backend/src/prisma/migrations/20260924202925_event_reminders/migration.sql`	Creates reminder database structures.
`src/backend/src/controllers/notifications.controllers.ts`	Handles notification endpoints.
`.github/workflows/notifications-hourly.yml`	Schedules hourly notifications.
`.github/workflows/notifications-daily.yml`	Updates the daily notification route.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

wavehassman

I think the root cause is GitHub scheduler. The new hourly workflow still uses GitHub's scheduler, so it'll be delayed too. Late runs won't send reminders after the meeting anymore, but they'll skip reminders: any 1-hour reminder whose run is more than about an hour late gets dropped. Can you look into other ways to do this? I'm thinking of triggering /notifications/hourly and /notifications/daily from AWS EventBridge Scheduler instead of GitHub cron, since the backend is already on AWS, but lmk if you find something different

wavehassman · 2026-09-27T16:34:21Z

-          teamSlackIds.add(team.slackId);
-        }
-      });
+    console.log(candidates);


remove this

wavehassman · 2026-09-27T16:34:57Z

+    }
  }

+  // /**


you can just delete all this

wavehassman · 2026-09-27T16:58:18Z

 import { HttpException } from '../utils/errors.utils.js';
 import { Reimbursement_Status_Type } from '@prisma/client';
-import { scheduleTimesTransformer } from '../transformers/calendar.transformer.js';
+import { HOUR_MS } from '../prisma/dates.js';


is this the seed data file?

Good catch. I went ahead and moved it to a more general file. I am editing the seed data files to use the more general one too.

Copilot

🟡 Changes recommended

Multiple critical issues currently disable delivery, risk lost reminders, expose secrets, or prevent scheduled jobs from running.

6 open findings

Debug return prevents all Slack notifications · New
Interrupted reminder claims permanently suppress retries · New
Secret name mismatch causes scheduled requests to return 401 · New
Middleware logs the shared notification secret · New
Docker build lacks the Yarn workspace-tools plugin · New
Cron expression runs hourly job every minute · New

3 resolved since last review

Await Slack sends and avoid duplicate cumulative messages
Prevent failed Slack sends from permanently claiming reminders
Test tier boundaries and exactly-once retry behavior

🧠 Review effort: Lite

Copilot

🟡 Changes recommended

Unresolved critical and moderate findings affect delivery correctness, failure visibility, and automated coverage.

2 open findings

Missing tests for reminder claim and delivery transitions · New
Standalone event notification channels are omitted · New

6 resolved since last review

Docker build lacks the Yarn workspace-tools plugin
Middleware logs the shared notification secret
Secret name mismatch causes scheduled requests to return 401
Interrupted reminder claims permanently suppress retries
Debug return prevents all Slack notifications
Cron expression runs hourly job every minute

🧠 Review effort: Lite

Copilot

🟡 Changes recommended

Critical reminder delivery, claim, and retry issues remain unresolved, along with workflow and seed determinism fixes.

3 open findings

Lease expiry allows duplicate Slack reminders · New
Slack send before status update permits duplicate reminders · New
Deleting failed claims permanently loses hourly reminders · New

2 resolved since last review

Missing tests for reminder claim and delivery transitions
Standalone event notification channels are omitted

Previously missed (1)

In code that hasn't changed since last review

Seed helper uses nondeterministic wall-clock defaults

src/backend/src/prisma/dates.ts:9

This makes the seed helper silently fall back to the wall clock whenever a caller omits either bound. The seed-time contract explicitly says generation is anchored to SEED_REFERENCE_DATE and that only realNow() may use the real clock, so a future unbounded call would make CI data nondeterministic. Keep from and to required and pass them directly to Faker.

🧠 Review effort: Lite

Copilot

🔵 Needs a closer look

Three moderate review findings remain unresolved.

0 open findings

3 resolved since last review

Deleting failed claims permanently loses hourly reminders
Slack send before status update permits duplicate reminders
Lease expiry allows duplicate Slack reminders

Previously missed (2)

In code that hasn't changed since last review

Ensure notification curl fails on HTTP errors

.github/workflows/notifications-daily.yml:14

Plain curl exits successfully for HTTP 401/500 responses, so a failed daily notification request will be recorded as a successful workflow run and will not be surfaced for retry. Use the same fail-on-HTTP-error behavior as the hourly workflow.

Keep seed date bounds required for deterministic behavior

src/backend/src/prisma/dates.ts:10

This makes the seed date helper silently fall back to wall-clock time whenever a caller omits a bound, reintroducing the nondeterminism that seed-time.ts explicitly forbids. The previous signature required both dates, and all current callers already pass them; keep both parameters required instead of using Date.now().

🧠 Review effort: Lite

wavehassman

Since this schedule is back after the revert, once the bay container is live, won't every message will go out twice. Can we switch it to workflow_dispatch?

Also, should figure out if the bay compose file has restart: unless-stopped, and whether it come back up after a reboot, since it'll be the only thing sending notifications now

wavehassman · 2026-10-14T02:12:35Z


 export default class NotificationsService {
  static async sendDailySlackNotifications() {
    await NotificationsService.sendTaskDeadlineSlackNotifications();


Can we move sendEventSlackNotifications() above this? Events are more time-sensitive, so they shouldn't wait on task deadline sends if Slack is slow

wavehassman · 2026-10-14T02:13:11Z

+export default defineJob({
+  name: 'daily-notifications',
+  schedule: '0 9 * * *',
+  endpoint: '/notifications/task-deadlines'


The endpoint does all its Slack sends before it responds, and the default timeout is 60s. On a slow Slack day this logs FAILED even though the messages went out. Make it timeoutMs: 10 * 60_000?

#4412 Restructuring to daily and hourly notifications

82593d3

Santiordon requested a lite review from Copilot September 25, 2026 00:41

Santiordon self-assigned this Sep 25, 2026

Copilot started reviewing on behalf of Santiordon September 25, 2026 00:41 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread src/backend/src/services/notifications.services.ts Outdated

Comment thread src/backend/src/services/notifications.services.ts Outdated

Comment thread src/backend/src/utils/notifications.utils.ts Outdated

Santiordon added 3 commits September 25, 2026 16:58

#4412 Reformat to delete claims where message failed to send

16b8e1f

#4412 Add tests and fix linting + prettier

c3205ed

#4412 Fix tests and due tier

ef9797f

wavehassman requested changes Sep 27, 2026

View reviewed changes

Santiordon added 3 commits September 28, 2026 17:11

#4412 Generalize time MS into a util file and address changes

3bb2596

#4412 Cron image and workspace

4c376e8

#4412 Testing crons

61ebdd2

Santiordon requested a lite review from Copilot October 8, 2026 22:15

Copilot started reviewing on behalf of Santiordon October 8, 2026 22:15 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

Santiordon added 3 commits October 8, 2026 18:22

#4412 Merge develop

8f962fa

#4412 Fix copilot suggestions

958b9f1

#4412 Switch back to NOTIFICATION_ENDPOINT_SECRET

63914db

Santiordon requested a lite review from Copilot October 8, 2026 23:12

Copilot started reviewing on behalf of Santiordon October 8, 2026 23:13 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

Comment thread src/backend/src/services/notifications.services.ts Outdated

Comment thread src/backend/src/utils/notifications.utils.ts Outdated

#4412 Fix omitting notificationChannelIds and add tests

7ade2be

Santiordon requested a lite review from Copilot October 8, 2026 23:31

Copilot started reviewing on behalf of Santiordon October 8, 2026 23:31 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

Comment thread src/backend/src/services/notifications.services.ts Outdated

Comment thread src/backend/src/services/notifications.services.ts Outdated

Comment thread src/backend/src/services/notifications.services.ts Outdated

Santiordon added 3 commits October 8, 2026 19:43

#4412 Add leases

140e6f7

#4412 More tests

ce6b754

#4412 Prettier

a954052

Santiordon requested a lite review from Copilot October 8, 2026 23:58

Copilot started reviewing on behalf of Santiordon October 8, 2026 23:59 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Santiordon added 5 commits October 9, 2026 16:08

#4412 Final Copilot suggestions

f02530e

#4412 Remove schedule from scheduled workflows

f683e84

#4412 Revert hourly additions

245b5bd

#4412 Final changes

e148523

#4412 Switch notification to go out 9am

b433cca

wavehassman self-requested a review October 14, 2026 01:55

wavehassman reviewed Oct 14, 2026

View reviewed changes

+                  }
                 }
+                // /**

Conversation

Santiordon commented Sep 25, 2026

Changes

Notes

Test Cases

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wavehassman left a comment

Choose a reason for hiding this comment

Uh oh!

wavehassman Sep 27, 2026

Choose a reason for hiding this comment

Uh oh!

wavehassman Sep 27, 2026

Choose a reason for hiding this comment

Uh oh!

wavehassman Sep 27, 2026

Choose a reason for hiding this comment

Uh oh!

Santiordon Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

wavehassman left a comment

Choose a reason for hiding this comment

Uh oh!

wavehassman Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

wavehassman Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants