Fix/bgw 07 by gnahk66 · Pull Request #112 · thoth-tech/doubtfire-api

gnahk66 · 2026-08-30T15:22:57Z

Description

Fixes a cleanup issue in AcceptOverseerJob where the Overseer work directory under tmp/overseer/<task-id>-<assessment-id> was only removed when the job completed successfully.

If an exception occurred after the work directory was created, the existing rescue block re-raised the exception but the directory remained on disk.

This change moves the cleanup into an ensure block:

ensure
FileUtils.rm_rf(work_dir) if work_dir
end

Fixes # (AcceptOverseerJob leaks its overseer work directory on every failure and nothing else ever cleans it)
This ensures the job-specific work directory is removed on both success and failure while preserving the existing exception handling so Sidekiq can still record failures and retry them.

A regression test was added to force an exception after the work directory is created and verify that:

the exception is still re-raised;
the work directory is removed.

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The issue was reproduced manually by forcing an exception inside AcceptOverseerJob#perform after the Overseer work directory had been created.

Before the fix, the exception was raised and the job directory remained under tmp/overseer.

After moving the cleanup into ensure, the same failure was raised but the job-specific work directory was removed.

Regression test:
bin/rails test test/sidekiq/accept_overseer_job_test.rb
Result:
1 runs, 4 assertions, 0 failures, 0 errors, 0 skips

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if appropriate
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
I have created or extended unit tests to address my new additions
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

docs(pwa): document offline behaviour

…payloads test(notifications): assert every event produces a valid push payload

…ls-core security(api): apply Rails 8.0.x security patches

Every other event in this feature hangs off something a person did. A deadline getting closer is nobody doing anything, so there is no model to hook and it has to be swept for. SendDueSoonRemindersJob runs daily at 8am and reminds a student about a task due within three days that they still owe work on. Three days is long enough to act on over a weekend and short enough that it is about this task rather than the rest of the trimester. Daily and at a fixed time because a deadline only moves once a day, and because that also fixes when the emails land: on a shorter interval a student gets theirs at whatever hour their task crossed into the window. The hour follows TZ in the process environment, which development/api.env sets to Australia/Melbourne. Not sending twice is the hard part. The job runs again tomorrow and the task is still due soon tomorrow, so a Notification.exists? guard on user, type, event and link means one reminder per student per task. The index on (user_id, event) is what makes that cheap. Retrying the whole sweep is safe for the same reason. Recipients come from projects, not from Task rows: OnTrack creates a row the first time anyone touches the task, so the students who have not started have no row and they are the ones a reminder is for. The job never calls task_for_task_definition, which creates the row it cannot find, nor task_definitions_and_status, which calls it and runs two queries per definition per project. Definitions are read once per unit and tasks once per project. Per student deadlines come from Webcal.end_date_for_task_definition, the app's existing answer to this and what the calendar feed shows, so extensions and a flexible unit's grade level overrides are handled and cannot drift from what the student sees. discuss and demonstrate are excluded. Both mean the student has submitted and is waiting on a tutor. A failure on one project is logged and collected, and the collected ids are raised at the end so Sidekiq retries. Swallowing them would leave perform successful, and a task due today is filtered out as overdue tomorrow. Not verified end to end: there is no Sidekiq worker in the dev stack (EN-F03), so the schedule entry has not been seen to fire. Tests: 18 runs, 64 assertions, 0 failures, 0 errors.

payload_for now sets a tag, and the operating system does the collapsing. Two notifications sharing a tag means the second replaces the first on screen rather than stacking beneath it. The tag is the event plus the link. The link is the only handle the api has on the subject, and the event is what makes it a conversation rather than a topic: a run of comments on one task is one thing being said repeatedly, and a deadline change to the same task is something else that must not quietly replace it. notification_type is deliberately not used, because it is only the preference category, so task_due_date_changed, task_status_changed, new_task_available and task_due_soon are all `task` and would have shared a tag. The notification id is not in it either, which would collapse nothing at all. link is nullable, and with no link there is nothing to be about, so the tag falls back to the id and that notification collapses with nothing. One shared empty tag would hide every linkless notification behind the newest. renotify is false, so a replacement updates the banner without making a sound again. The case this exists for is a tutor posting five comments on one task in two minutes; the tag already collapses those, and renotify true would put the buzz back on every one of them. tag and renotify are both in NOTIFICATION_OPTION_NAMES in Angular's ngsw-worker.js, so both reach showNotification with no change on the web side. Title and body are untouched, and two of the new tests assert that, since the wording belongs to MN-D01 and MN-D05. Also adds require 'minitest/mock' to the test file. test_helper.rb does not pull it in, so Object#stub does not exist, and test_both_timeouts_are_passed_to_the_gem has been erroring with "undefined method 'stub' for module WebPush" since it was written. One line fixes that test too. Tests: 21 runs, 38 assertions, 0 failures, 0 errors.

…k-session fix(deps): update Rack and Rack Session

…h-dependencies fix(deps): update OAuth2 JWT and Ruby SAML

…p-client fix(deps): update Faraday and Addressable

…a-compat fix(deps): upgrade Puma to 7.2.1 for PROXY protocol fixes

feat(notifications): add due soon reminder job

…tifications feat(notifications): collapse repeat push notifications with a tag

…amplification-risk docs(notifications): add recipient amplification risk review

Adds `rake db:ppi_sample_data`, creating a deterministic minimum test data set for testing the Peer Progress Indicator dashboard: 2 units, 2 classes of 16 students per unit (4 students per target grade — Pass/Credit/Distinction/High Distinction), and 7 tasks per unit with completion rates scaled by target grade so percentages are clearly distinguishable across grade bands. Verified against the local dev environment: both units created successfully with 32 students and 7 tasks each, and completion percentages visibly differ by grade band in the Students tab.

Adds a permanent technical reference documenting where the Peer Progress Indicator's response fields actually come from in the backend, so PPI-B01, PPI-F01, PPI-S01 and future contributors don't have to rediscover it. Builds on the earlier PPI API discovery task (preserved here as a companion doc), which found the reusable aggregation infrastructure existed but wasn't reachable by students. This document goes further: it reviews the real, unmerged implementation on ppi/student-progress-endpoint (PPI-B01) and maps all 9 response fields to their exact source file/method, availability status, any transformation applied, and the ticket that owns each one. Includes a Mermaid data-flow diagram traced from the actual code (auth -> authorised project -> task validation -> server-side target-grade lookup -> nightly cohort aggregation -> small-cohort suppression -> percentage quantisation -> safe response -> frontend adapter -> existing widget), safe example responses for four business states, and a recorded list of concrete gaps found while reviewing the branch (unset config env vars, no unit has PPI enabled yet, a frontend/backend target-grade parameter mismatch, two frontend branches that independently renamed the same model, and a backfill migration that will blank every existing snapshot on first deploy). Does not implement the endpoint, frontend adapter, or any of the other PPI-* work explicitly out of scope for this ticket.

Quantising into buckets of B only hides the underlying count while B is strictly wider than one student's share of the cohort, 100.0 / cohort_size. A floor of 5 with a 5-point bucket satisfied neither side of that: for every cohort from 5 to 20 the mapping was injective and the returned percentage inverted to an exact submitted count. Move to a floor of 20 and a 10-point bucket, which leaves no cohort at or above the floor from which the count can be recovered, and add two tests that pin the relationship so the constants cannot drift apart again. Also scope the nightly aggregation to units that actually enabled the feature, rather than storing derived cohort statistics for every active unit and never serving them. Docs updated: the bucket size, the example response, the configuration floor, and the genuine-zero row, which is no longer true now that a small count also rounds to zero.

….x-all-features-20260824 release(api): OnTrack 11.0.x all-features handover

…l/test-preference-gating

…ability-paths fix(notifications): cover task availability paths

Both sides added NotificationEmailJob.drain at the same point in submit_portfolio, yours in 81016ac on this branch and mine in the repair that landed on feature/notifications via #65, so it came out as a conflict over one blank line and a comment. Kept the base version, which carries the explanation, so the file matches what is already on the shared branch and a later merge has nothing left to reconcile. 36 runs, 210 assertions, 0 failures across notification_service, notification_portfolio, notification_discussion_request and notification_email_job.

…miting fix(auth): add rate limiting to authentication endpoint

…rence-gating test(notifications): cover preference gating

…-mvp-20260827 # Conflicts: # .github/workflows/codeql.yml

… into closure/api-ontrack-mvp-20260827 # Conflicts: # app/api/peer_progress_api.rb # docs/peer-progress-api.md # docs/peer-progress/data-source-map.md # test/api/peer_progress_api_test.rb

…ure/api-ontrack-mvp-20260827 # Conflicts: # app/models/notification.rb # app/services/notification_service.rb # app/sidekiq/new_task_available_notification_job.rb # app/sidekiq/notification_email_job.rb # config/sidekiq.yml # db/schema.rb # test/api/settings_push_test.rb # test/models/notification_discussion_request_test.rb # test/models/notification_new_task_test.rb # test/services/notification_service_test.rb # test/sidekiq/scheduled_job_test.rb # test/sidekiq/send_new_task_available_notifications_job_test.rb

…ack-mvp-20260827 Close OnTrack MVP API integration and shard the test suite

Two comments were wrong. One grouped Edge under fcm.googleapis.com and the other described *.notify.windows.com as a legacy host used by legacy Edge. The MN-Q01 verification run on 27 Aug 2026 drove Microsoft Edge 151.0.4129.107 (Chromium 151.0.7922.174) on macOS 26.5.2 and it subscribed through wns2-bl2p.notify.windows.com, over three separate trigger runs. So a current Chromium Edge uses WNS, not FCM, and notify.windows.com is not a legacy host. The same run had Chrome 152 on fcm.googleapis.com and Firefox 154 on updates.push.services.mozilla.com, which is what those two lines already said, so they stay as they are. Edge comes off the FCM line, notify.windows.com is described as current, and a short note is added to each block saying a Chromium Edge still lands on WNS. Without it a reader assumes Chromium implies FCM, which is the assumption that put Edge on the wrong line in the first place. Comments only. PUSH_SERVICE_HOSTS and PUSH_SERVICE_HOST_SUFFIXES are untouched, and the allow list was already correct because .notify.windows.com is a suffix entry, so Edge endpoints were accepted the whole time. Strip every comment line from the file and the remaining bytes are identical before and after.

…s-dedicated-push-queue-20260828 Fix Mobile Notifications push queue delivery

…al-security-evidence-20260827 test(security): tighten FILE-S01 evidence and log privacy

…260828

…st-comments-11x-20260827 docs(notifications): correct Edge WNS host guidance on 11.0.x

…ime-followup-20260828 ci: reduce full API test runtime

gnahk66 · 2026-08-30T15:22:57Z

Description

Fixes a cleanup issue in AcceptOverseerJob where the Overseer work directory under tmp/overseer/<task-id>-<assessment-id> was only removed when the job completed successfully.

If an exception occurred after the work directory was created, the existing rescue block re-raised the exception but the directory remained on disk.

This change moves the cleanup into an ensure block:

ensure
FileUtils.rm_rf(work_dir) if work_dir
end

Fixes # (AcceptOverseerJob leaks its overseer work directory on every failure and nothing else ever cleans it)
This ensures the job-specific work directory is removed on both success and failure while preserving the existing exception handling so Sidekiq can still record failures and retry them.

A regression test was added to force an exception after the work directory is created and verify that:

the exception is still re-raised;
the work directory is removed.

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The issue was reproduced manually by forcing an exception inside AcceptOverseerJob#perform after the Overseer work directory had been created.

Before the fix, the exception was raised and the job directory remained under tmp/overseer.

After moving the cleanup into ensure, the same failure was raised but the job-specific work directory was removed.

Regression test:
bin/rails test test/sidekiq/accept_overseer_job_test.rb
Result:
1 runs, 4 assertions, 0 failures, 0 errors, 0 skips

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if appropriate
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
I have created or extended unit tests to address my new additions
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

docs(pwa): document offline behaviour

…payloads test(notifications): assert every event produces a valid push payload

…ls-core security(api): apply Rails 8.0.x security patches

Every other event in this feature hangs off something a person did. A deadline getting closer is nobody doing anything, so there is no model to hook and it has to be swept for. SendDueSoonRemindersJob runs daily at 8am and reminds a student about a task due within three days that they still owe work on. Three days is long enough to act on over a weekend and short enough that it is about this task rather than the rest of the trimester. Daily and at a fixed time because a deadline only moves once a day, and because that also fixes when the emails land: on a shorter interval a student gets theirs at whatever hour their task crossed into the window. The hour follows TZ in the process environment, which development/api.env sets to Australia/Melbourne. Not sending twice is the hard part. The job runs again tomorrow and the task is still due soon tomorrow, so a Notification.exists? guard on user, type, event and link means one reminder per student per task. The index on (user_id, event) is what makes that cheap. Retrying the whole sweep is safe for the same reason. Recipients come from projects, not from Task rows: OnTrack creates a row the first time anyone touches the task, so the students who have not started have no row and they are the ones a reminder is for. The job never calls task_for_task_definition, which creates the row it cannot find, nor task_definitions_and_status, which calls it and runs two queries per definition per project. Definitions are read once per unit and tasks once per project. Per student deadlines come from Webcal.end_date_for_task_definition, the app's existing answer to this and what the calendar feed shows, so extensions and a flexible unit's grade level overrides are handled and cannot drift from what the student sees. discuss and demonstrate are excluded. Both mean the student has submitted and is waiting on a tutor. A failure on one project is logged and collected, and the collected ids are raised at the end so Sidekiq retries. Swallowing them would leave perform successful, and a task due today is filtered out as overdue tomorrow. Not verified end to end: there is no Sidekiq worker in the dev stack (EN-F03), so the schedule entry has not been seen to fire. Tests: 18 runs, 64 assertions, 0 failures, 0 errors.

payload_for now sets a tag, and the operating system does the collapsing. Two notifications sharing a tag means the second replaces the first on screen rather than stacking beneath it. The tag is the event plus the link. The link is the only handle the api has on the subject, and the event is what makes it a conversation rather than a topic: a run of comments on one task is one thing being said repeatedly, and a deadline change to the same task is something else that must not quietly replace it. notification_type is deliberately not used, because it is only the preference category, so task_due_date_changed, task_status_changed, new_task_available and task_due_soon are all `task` and would have shared a tag. The notification id is not in it either, which would collapse nothing at all. link is nullable, and with no link there is nothing to be about, so the tag falls back to the id and that notification collapses with nothing. One shared empty tag would hide every linkless notification behind the newest. renotify is false, so a replacement updates the banner without making a sound again. The case this exists for is a tutor posting five comments on one task in two minutes; the tag already collapses those, and renotify true would put the buzz back on every one of them. tag and renotify are both in NOTIFICATION_OPTION_NAMES in Angular's ngsw-worker.js, so both reach showNotification with no change on the web side. Title and body are untouched, and two of the new tests assert that, since the wording belongs to MN-D01 and MN-D05. Also adds require 'minitest/mock' to the test file. test_helper.rb does not pull it in, so Object#stub does not exist, and test_both_timeouts_are_passed_to_the_gem has been erroring with "undefined method 'stub' for module WebPush" since it was written. One line fixes that test too. Tests: 21 runs, 38 assertions, 0 failures, 0 errors.

…k-session fix(deps): update Rack and Rack Session

…h-dependencies fix(deps): update OAuth2 JWT and Ruby SAML

…p-client fix(deps): update Faraday and Addressable

…a-compat fix(deps): upgrade Puma to 7.2.1 for PROXY protocol fixes

feat(notifications): add due soon reminder job

…tifications feat(notifications): collapse repeat push notifications with a tag

…amplification-risk docs(notifications): add recipient amplification risk review

Adds `rake db:ppi_sample_data`, creating a deterministic minimum test data set for testing the Peer Progress Indicator dashboard: 2 units, 2 classes of 16 students per unit (4 students per target grade — Pass/Credit/Distinction/High Distinction), and 7 tasks per unit with completion rates scaled by target grade so percentages are clearly distinguishable across grade bands. Verified against the local dev environment: both units created successfully with 32 students and 7 tasks each, and completion percentages visibly differ by grade band in the Students tab.

Adds a permanent technical reference documenting where the Peer Progress Indicator's response fields actually come from in the backend, so PPI-B01, PPI-F01, PPI-S01 and future contributors don't have to rediscover it. Builds on the earlier PPI API discovery task (preserved here as a companion doc), which found the reusable aggregation infrastructure existed but wasn't reachable by students. This document goes further: it reviews the real, unmerged implementation on ppi/student-progress-endpoint (PPI-B01) and maps all 9 response fields to their exact source file/method, availability status, any transformation applied, and the ticket that owns each one. Includes a Mermaid data-flow diagram traced from the actual code (auth -> authorised project -> task validation -> server-side target-grade lookup -> nightly cohort aggregation -> small-cohort suppression -> percentage quantisation -> safe response -> frontend adapter -> existing widget), safe example responses for four business states, and a recorded list of concrete gaps found while reviewing the branch (unset config env vars, no unit has PPI enabled yet, a frontend/backend target-grade parameter mismatch, two frontend branches that independently renamed the same model, and a backfill migration that will blank every existing snapshot on first deploy). Does not implement the endpoint, frontend adapter, or any of the other PPI-* work explicitly out of scope for this ticket.

Quantising into buckets of B only hides the underlying count while B is strictly wider than one student's share of the cohort, 100.0 / cohort_size. A floor of 5 with a 5-point bucket satisfied neither side of that: for every cohort from 5 to 20 the mapping was injective and the returned percentage inverted to an exact submitted count. Move to a floor of 20 and a 10-point bucket, which leaves no cohort at or above the floor from which the count can be recovered, and add two tests that pin the relationship so the constants cannot drift apart again. Also scope the nightly aggregation to units that actually enabled the feature, rather than storing derived cohort statistics for every active unit and never serving them. Docs updated: the bucket size, the example response, the configuration floor, and the genuine-zero row, which is no longer true now that a small count also rounds to zero.

….x-all-features-20260824 release(api): OnTrack 11.0.x all-features handover

…l/test-preference-gating

…ability-paths fix(notifications): cover task availability paths

Both sides added NotificationEmailJob.drain at the same point in submit_portfolio, yours in 81016ac on this branch and mine in the repair that landed on feature/notifications via #65, so it came out as a conflict over one blank line and a comment. Kept the base version, which carries the explanation, so the file matches what is already on the shared branch and a later merge has nothing left to reconcile. 36 runs, 210 assertions, 0 failures across notification_service, notification_portfolio, notification_discussion_request and notification_email_job.

…miting fix(auth): add rate limiting to authentication endpoint

…rence-gating test(notifications): cover preference gating

…-mvp-20260827 # Conflicts: # .github/workflows/codeql.yml

… into closure/api-ontrack-mvp-20260827 # Conflicts: # app/api/peer_progress_api.rb # docs/peer-progress-api.md # docs/peer-progress/data-source-map.md # test/api/peer_progress_api_test.rb

…ure/api-ontrack-mvp-20260827 # Conflicts: # app/models/notification.rb # app/services/notification_service.rb # app/sidekiq/new_task_available_notification_job.rb # app/sidekiq/notification_email_job.rb # config/sidekiq.yml # db/schema.rb # test/api/settings_push_test.rb # test/models/notification_discussion_request_test.rb # test/models/notification_new_task_test.rb # test/services/notification_service_test.rb # test/sidekiq/scheduled_job_test.rb # test/sidekiq/send_new_task_available_notifications_job_test.rb

…ack-mvp-20260827 Close OnTrack MVP API integration and shard the test suite

Two comments were wrong. One grouped Edge under fcm.googleapis.com and the other described *.notify.windows.com as a legacy host used by legacy Edge. The MN-Q01 verification run on 27 Aug 2026 drove Microsoft Edge 151.0.4129.107 (Chromium 151.0.7922.174) on macOS 26.5.2 and it subscribed through wns2-bl2p.notify.windows.com, over three separate trigger runs. So a current Chromium Edge uses WNS, not FCM, and notify.windows.com is not a legacy host. The same run had Chrome 152 on fcm.googleapis.com and Firefox 154 on updates.push.services.mozilla.com, which is what those two lines already said, so they stay as they are. Edge comes off the FCM line, notify.windows.com is described as current, and a short note is added to each block saying a Chromium Edge still lands on WNS. Without it a reader assumes Chromium implies FCM, which is the assumption that put Edge on the wrong line in the first place. Comments only. PUSH_SERVICE_HOSTS and PUSH_SERVICE_HOST_SUFFIXES are untouched, and the allow list was already correct because .notify.windows.com is a suffix entry, so Edge endpoints were accepted the whole time. Strip every comment line from the file and the remaining bytes are identical before and after.

…s-dedicated-push-queue-20260828 Fix Mobile Notifications push queue delivery

…al-security-evidence-20260827 test(security): tighten FILE-S01 evidence and log privacy

…260828

…st-comments-11x-20260827 docs(notifications): correct Edge WNS host guidance on 11.0.x

…ime-followup-20260828 ci: reduce full API test runtime

gnahk66 · 2026-08-30T15:22:57Z

Description

Fixes a cleanup issue in AcceptOverseerJob where the Overseer work directory under tmp/overseer/<task-id>-<assessment-id> was only removed when the job completed successfully.

If an exception occurred after the work directory was created, the existing rescue block re-raised the exception but the directory remained on disk.

This change moves the cleanup into an ensure block:

ensure
FileUtils.rm_rf(work_dir) if work_dir
end

Fixes # (AcceptOverseerJob leaks its overseer work directory on every failure and nothing else ever cleans it)
This ensures the job-specific work directory is removed on both success and failure while preserving the existing exception handling so Sidekiq can still record failures and retry them.

A regression test was added to force an exception after the work directory is created and verify that:

the exception is still re-raised;
the work directory is removed.

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The issue was reproduced manually by forcing an exception inside AcceptOverseerJob#perform after the Overseer work directory had been created.

Before the fix, the exception was raised and the job directory remained under tmp/overseer.

After moving the cleanup into ensure, the same failure was raised but the job-specific work directory was removed.

Regression test:
bin/rails test test/sidekiq/accept_overseer_job_test.rb
Result:
1 runs, 4 assertions, 0 failures, 0 errors, 0 skips

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if appropriate
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
I have created or extended unit tests to address my new additions
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

docs(pwa): document offline behaviour

…payloads test(notifications): assert every event produces a valid push payload

…ls-core security(api): apply Rails 8.0.x security patches

Every other event in this feature hangs off something a person did. A deadline getting closer is nobody doing anything, so there is no model to hook and it has to be swept for. SendDueSoonRemindersJob runs daily at 8am and reminds a student about a task due within three days that they still owe work on. Three days is long enough to act on over a weekend and short enough that it is about this task rather than the rest of the trimester. Daily and at a fixed time because a deadline only moves once a day, and because that also fixes when the emails land: on a shorter interval a student gets theirs at whatever hour their task crossed into the window. The hour follows TZ in the process environment, which development/api.env sets to Australia/Melbourne. Not sending twice is the hard part. The job runs again tomorrow and the task is still due soon tomorrow, so a Notification.exists? guard on user, type, event and link means one reminder per student per task. The index on (user_id, event) is what makes that cheap. Retrying the whole sweep is safe for the same reason. Recipients come from projects, not from Task rows: OnTrack creates a row the first time anyone touches the task, so the students who have not started have no row and they are the ones a reminder is for. The job never calls task_for_task_definition, which creates the row it cannot find, nor task_definitions_and_status, which calls it and runs two queries per definition per project. Definitions are read once per unit and tasks once per project. Per student deadlines come from Webcal.end_date_for_task_definition, the app's existing answer to this and what the calendar feed shows, so extensions and a flexible unit's grade level overrides are handled and cannot drift from what the student sees. discuss and demonstrate are excluded. Both mean the student has submitted and is waiting on a tutor. A failure on one project is logged and collected, and the collected ids are raised at the end so Sidekiq retries. Swallowing them would leave perform successful, and a task due today is filtered out as overdue tomorrow. Not verified end to end: there is no Sidekiq worker in the dev stack (EN-F03), so the schedule entry has not been seen to fire. Tests: 18 runs, 64 assertions, 0 failures, 0 errors.

payload_for now sets a tag, and the operating system does the collapsing. Two notifications sharing a tag means the second replaces the first on screen rather than stacking beneath it. The tag is the event plus the link. The link is the only handle the api has on the subject, and the event is what makes it a conversation rather than a topic: a run of comments on one task is one thing being said repeatedly, and a deadline change to the same task is something else that must not quietly replace it. notification_type is deliberately not used, because it is only the preference category, so task_due_date_changed, task_status_changed, new_task_available and task_due_soon are all `task` and would have shared a tag. The notification id is not in it either, which would collapse nothing at all. link is nullable, and with no link there is nothing to be about, so the tag falls back to the id and that notification collapses with nothing. One shared empty tag would hide every linkless notification behind the newest. renotify is false, so a replacement updates the banner without making a sound again. The case this exists for is a tutor posting five comments on one task in two minutes; the tag already collapses those, and renotify true would put the buzz back on every one of them. tag and renotify are both in NOTIFICATION_OPTION_NAMES in Angular's ngsw-worker.js, so both reach showNotification with no change on the web side. Title and body are untouched, and two of the new tests assert that, since the wording belongs to MN-D01 and MN-D05. Also adds require 'minitest/mock' to the test file. test_helper.rb does not pull it in, so Object#stub does not exist, and test_both_timeouts_are_passed_to_the_gem has been erroring with "undefined method 'stub' for module WebPush" since it was written. One line fixes that test too. Tests: 21 runs, 38 assertions, 0 failures, 0 errors.

…k-session fix(deps): update Rack and Rack Session

…h-dependencies fix(deps): update OAuth2 JWT and Ruby SAML

…p-client fix(deps): update Faraday and Addressable

…a-compat fix(deps): upgrade Puma to 7.2.1 for PROXY protocol fixes

feat(notifications): add due soon reminder job

…tifications feat(notifications): collapse repeat push notifications with a tag

…amplification-risk docs(notifications): add recipient amplification risk review

Adds `rake db:ppi_sample_data`, creating a deterministic minimum test data set for testing the Peer Progress Indicator dashboard: 2 units, 2 classes of 16 students per unit (4 students per target grade — Pass/Credit/Distinction/High Distinction), and 7 tasks per unit with completion rates scaled by target grade so percentages are clearly distinguishable across grade bands. Verified against the local dev environment: both units created successfully with 32 students and 7 tasks each, and completion percentages visibly differ by grade band in the Students tab.

Adds a permanent technical reference documenting where the Peer Progress Indicator's response fields actually come from in the backend, so PPI-B01, PPI-F01, PPI-S01 and future contributors don't have to rediscover it. Builds on the earlier PPI API discovery task (preserved here as a companion doc), which found the reusable aggregation infrastructure existed but wasn't reachable by students. This document goes further: it reviews the real, unmerged implementation on ppi/student-progress-endpoint (PPI-B01) and maps all 9 response fields to their exact source file/method, availability status, any transformation applied, and the ticket that owns each one. Includes a Mermaid data-flow diagram traced from the actual code (auth -> authorised project -> task validation -> server-side target-grade lookup -> nightly cohort aggregation -> small-cohort suppression -> percentage quantisation -> safe response -> frontend adapter -> existing widget), safe example responses for four business states, and a recorded list of concrete gaps found while reviewing the branch (unset config env vars, no unit has PPI enabled yet, a frontend/backend target-grade parameter mismatch, two frontend branches that independently renamed the same model, and a backfill migration that will blank every existing snapshot on first deploy). Does not implement the endpoint, frontend adapter, or any of the other PPI-* work explicitly out of scope for this ticket.

Quantising into buckets of B only hides the underlying count while B is strictly wider than one student's share of the cohort, 100.0 / cohort_size. A floor of 5 with a 5-point bucket satisfied neither side of that: for every cohort from 5 to 20 the mapping was injective and the returned percentage inverted to an exact submitted count. Move to a floor of 20 and a 10-point bucket, which leaves no cohort at or above the floor from which the count can be recovered, and add two tests that pin the relationship so the constants cannot drift apart again. Also scope the nightly aggregation to units that actually enabled the feature, rather than storing derived cohort statistics for every active unit and never serving them. Docs updated: the bucket size, the example response, the configuration floor, and the genuine-zero row, which is no longer true now that a small count also rounds to zero.

….x-all-features-20260824 release(api): OnTrack 11.0.x all-features handover

…l/test-preference-gating

…ability-paths fix(notifications): cover task availability paths

Both sides added NotificationEmailJob.drain at the same point in submit_portfolio, yours in 81016ac on this branch and mine in the repair that landed on feature/notifications via #65, so it came out as a conflict over one blank line and a comment. Kept the base version, which carries the explanation, so the file matches what is already on the shared branch and a later merge has nothing left to reconcile. 36 runs, 210 assertions, 0 failures across notification_service, notification_portfolio, notification_discussion_request and notification_email_job.

…miting fix(auth): add rate limiting to authentication endpoint

…rence-gating test(notifications): cover preference gating

…-mvp-20260827 # Conflicts: # .github/workflows/codeql.yml

… into closure/api-ontrack-mvp-20260827 # Conflicts: # app/api/peer_progress_api.rb # docs/peer-progress-api.md # docs/peer-progress/data-source-map.md # test/api/peer_progress_api_test.rb

…ure/api-ontrack-mvp-20260827 # Conflicts: # app/models/notification.rb # app/services/notification_service.rb # app/sidekiq/new_task_available_notification_job.rb # app/sidekiq/notification_email_job.rb # config/sidekiq.yml # db/schema.rb # test/api/settings_push_test.rb # test/models/notification_discussion_request_test.rb # test/models/notification_new_task_test.rb # test/services/notification_service_test.rb # test/sidekiq/scheduled_job_test.rb # test/sidekiq/send_new_task_available_notifications_job_test.rb

…ack-mvp-20260827 Close OnTrack MVP API integration and shard the test suite

Two comments were wrong. One grouped Edge under fcm.googleapis.com and the other described *.notify.windows.com as a legacy host used by legacy Edge. The MN-Q01 verification run on 27 Aug 2026 drove Microsoft Edge 151.0.4129.107 (Chromium 151.0.7922.174) on macOS 26.5.2 and it subscribed through wns2-bl2p.notify.windows.com, over three separate trigger runs. So a current Chromium Edge uses WNS, not FCM, and notify.windows.com is not a legacy host. The same run had Chrome 152 on fcm.googleapis.com and Firefox 154 on updates.push.services.mozilla.com, which is what those two lines already said, so they stay as they are. Edge comes off the FCM line, notify.windows.com is described as current, and a short note is added to each block saying a Chromium Edge still lands on WNS. Without it a reader assumes Chromium implies FCM, which is the assumption that put Edge on the wrong line in the first place. Comments only. PUSH_SERVICE_HOSTS and PUSH_SERVICE_HOST_SUFFIXES are untouched, and the allow list was already correct because .notify.windows.com is a suffix entry, so Edge endpoints were accepted the whole time. Strip every comment line from the file and the remaining bytes are identical before and after.

…s-dedicated-push-queue-20260828 Fix Mobile Notifications push queue delivery

…al-security-evidence-20260827 test(security): tighten FILE-S01 evidence and log privacy

…260828

…st-comments-11x-20260827 docs(notifications): correct Edge WNS host guidance on 11.0.x

…ime-followup-20260828 ci: reduce full API test runtime

gnahk66 · 2026-08-30T15:22:57Z

Description

Fixes a cleanup issue in AcceptOverseerJob where the Overseer work directory under tmp/overseer/<task-id>-<assessment-id> was only removed when the job completed successfully.

If an exception occurred after the work directory was created, the existing rescue block re-raised the exception but the directory remained on disk.

This change moves the cleanup into an ensure block:

ensure
FileUtils.rm_rf(work_dir) if work_dir
end

Fixes # (AcceptOverseerJob leaks its overseer work directory on every failure and nothing else ever cleans it)
This ensures the job-specific work directory is removed on both success and failure while preserving the existing exception handling so Sidekiq can still record failures and retry them.

A regression test was added to force an exception after the work directory is created and verify that:

the exception is still re-raised;
the work directory is removed.

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The issue was reproduced manually by forcing an exception inside AcceptOverseerJob#perform after the Overseer work directory had been created.

Before the fix, the exception was raised and the job directory remained under tmp/overseer.

After moving the cleanup into ensure, the same failure was raised but the job-specific work directory was removed.

Regression test:
bin/rails test test/sidekiq/accept_overseer_job_test.rb
Result:
1 runs, 4 assertions, 0 failures, 0 errors, 0 skips

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if appropriate
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
I have created or extended unit tests to address my new additions
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

docs(pwa): document offline behaviour

…payloads test(notifications): assert every event produces a valid push payload

…ls-core security(api): apply Rails 8.0.x security patches

Every other event in this feature hangs off something a person did. A deadline getting closer is nobody doing anything, so there is no model to hook and it has to be swept for. SendDueSoonRemindersJob runs daily at 8am and reminds a student about a task due within three days that they still owe work on. Three days is long enough to act on over a weekend and short enough that it is about this task rather than the rest of the trimester. Daily and at a fixed time because a deadline only moves once a day, and because that also fixes when the emails land: on a shorter interval a student gets theirs at whatever hour their task crossed into the window. The hour follows TZ in the process environment, which development/api.env sets to Australia/Melbourne. Not sending twice is the hard part. The job runs again tomorrow and the task is still due soon tomorrow, so a Notification.exists? guard on user, type, event and link means one reminder per student per task. The index on (user_id, event) is what makes that cheap. Retrying the whole sweep is safe for the same reason. Recipients come from projects, not from Task rows: OnTrack creates a row the first time anyone touches the task, so the students who have not started have no row and they are the ones a reminder is for. The job never calls task_for_task_definition, which creates the row it cannot find, nor task_definitions_and_status, which calls it and runs two queries per definition per project. Definitions are read once per unit and tasks once per project. Per student deadlines come from Webcal.end_date_for_task_definition, the app's existing answer to this and what the calendar feed shows, so extensions and a flexible unit's grade level overrides are handled and cannot drift from what the student sees. discuss and demonstrate are excluded. Both mean the student has submitted and is waiting on a tutor. A failure on one project is logged and collected, and the collected ids are raised at the end so Sidekiq retries. Swallowing them would leave perform successful, and a task due today is filtered out as overdue tomorrow. Not verified end to end: there is no Sidekiq worker in the dev stack (EN-F03), so the schedule entry has not been seen to fire. Tests: 18 runs, 64 assertions, 0 failures, 0 errors.

payload_for now sets a tag, and the operating system does the collapsing. Two notifications sharing a tag means the second replaces the first on screen rather than stacking beneath it. The tag is the event plus the link. The link is the only handle the api has on the subject, and the event is what makes it a conversation rather than a topic: a run of comments on one task is one thing being said repeatedly, and a deadline change to the same task is something else that must not quietly replace it. notification_type is deliberately not used, because it is only the preference category, so task_due_date_changed, task_status_changed, new_task_available and task_due_soon are all `task` and would have shared a tag. The notification id is not in it either, which would collapse nothing at all. link is nullable, and with no link there is nothing to be about, so the tag falls back to the id and that notification collapses with nothing. One shared empty tag would hide every linkless notification behind the newest. renotify is false, so a replacement updates the banner without making a sound again. The case this exists for is a tutor posting five comments on one task in two minutes; the tag already collapses those, and renotify true would put the buzz back on every one of them. tag and renotify are both in NOTIFICATION_OPTION_NAMES in Angular's ngsw-worker.js, so both reach showNotification with no change on the web side. Title and body are untouched, and two of the new tests assert that, since the wording belongs to MN-D01 and MN-D05. Also adds require 'minitest/mock' to the test file. test_helper.rb does not pull it in, so Object#stub does not exist, and test_both_timeouts_are_passed_to_the_gem has been erroring with "undefined method 'stub' for module WebPush" since it was written. One line fixes that test too. Tests: 21 runs, 38 assertions, 0 failures, 0 errors.

…k-session fix(deps): update Rack and Rack Session

…h-dependencies fix(deps): update OAuth2 JWT and Ruby SAML

…p-client fix(deps): update Faraday and Addressable

…a-compat fix(deps): upgrade Puma to 7.2.1 for PROXY protocol fixes

feat(notifications): add due soon reminder job

…tifications feat(notifications): collapse repeat push notifications with a tag

…amplification-risk docs(notifications): add recipient amplification risk review

Adds `rake db:ppi_sample_data`, creating a deterministic minimum test data set for testing the Peer Progress Indicator dashboard: 2 units, 2 classes of 16 students per unit (4 students per target grade — Pass/Credit/Distinction/High Distinction), and 7 tasks per unit with completion rates scaled by target grade so percentages are clearly distinguishable across grade bands. Verified against the local dev environment: both units created successfully with 32 students and 7 tasks each, and completion percentages visibly differ by grade band in the Students tab.

Adds a permanent technical reference documenting where the Peer Progress Indicator's response fields actually come from in the backend, so PPI-B01, PPI-F01, PPI-S01 and future contributors don't have to rediscover it. Builds on the earlier PPI API discovery task (preserved here as a companion doc), which found the reusable aggregation infrastructure existed but wasn't reachable by students. This document goes further: it reviews the real, unmerged implementation on ppi/student-progress-endpoint (PPI-B01) and maps all 9 response fields to their exact source file/method, availability status, any transformation applied, and the ticket that owns each one. Includes a Mermaid data-flow diagram traced from the actual code (auth -> authorised project -> task validation -> server-side target-grade lookup -> nightly cohort aggregation -> small-cohort suppression -> percentage quantisation -> safe response -> frontend adapter -> existing widget), safe example responses for four business states, and a recorded list of concrete gaps found while reviewing the branch (unset config env vars, no unit has PPI enabled yet, a frontend/backend target-grade parameter mismatch, two frontend branches that independently renamed the same model, and a backfill migration that will blank every existing snapshot on first deploy). Does not implement the endpoint, frontend adapter, or any of the other PPI-* work explicitly out of scope for this ticket.

Quantising into buckets of B only hides the underlying count while B is strictly wider than one student's share of the cohort, 100.0 / cohort_size. A floor of 5 with a 5-point bucket satisfied neither side of that: for every cohort from 5 to 20 the mapping was injective and the returned percentage inverted to an exact submitted count. Move to a floor of 20 and a 10-point bucket, which leaves no cohort at or above the floor from which the count can be recovered, and add two tests that pin the relationship so the constants cannot drift apart again. Also scope the nightly aggregation to units that actually enabled the feature, rather than storing derived cohort statistics for every active unit and never serving them. Docs updated: the bucket size, the example response, the configuration floor, and the genuine-zero row, which is no longer true now that a small count also rounds to zero.

….x-all-features-20260824 release(api): OnTrack 11.0.x all-features handover

…l/test-preference-gating

…ability-paths fix(notifications): cover task availability paths

Both sides added NotificationEmailJob.drain at the same point in submit_portfolio, yours in 81016ac on this branch and mine in the repair that landed on feature/notifications via #65, so it came out as a conflict over one blank line and a comment. Kept the base version, which carries the explanation, so the file matches what is already on the shared branch and a later merge has nothing left to reconcile. 36 runs, 210 assertions, 0 failures across notification_service, notification_portfolio, notification_discussion_request and notification_email_job.

…miting fix(auth): add rate limiting to authentication endpoint

…rence-gating test(notifications): cover preference gating

…-mvp-20260827 # Conflicts: # .github/workflows/codeql.yml

… into closure/api-ontrack-mvp-20260827 # Conflicts: # app/api/peer_progress_api.rb # docs/peer-progress-api.md # docs/peer-progress/data-source-map.md # test/api/peer_progress_api_test.rb

…ure/api-ontrack-mvp-20260827 # Conflicts: # app/models/notification.rb # app/services/notification_service.rb # app/sidekiq/new_task_available_notification_job.rb # app/sidekiq/notification_email_job.rb # config/sidekiq.yml # db/schema.rb # test/api/settings_push_test.rb # test/models/notification_discussion_request_test.rb # test/models/notification_new_task_test.rb # test/services/notification_service_test.rb # test/sidekiq/scheduled_job_test.rb # test/sidekiq/send_new_task_available_notifications_job_test.rb

…ack-mvp-20260827 Close OnTrack MVP API integration and shard the test suite

Two comments were wrong. One grouped Edge under fcm.googleapis.com and the other described *.notify.windows.com as a legacy host used by legacy Edge. The MN-Q01 verification run on 27 Aug 2026 drove Microsoft Edge 151.0.4129.107 (Chromium 151.0.7922.174) on macOS 26.5.2 and it subscribed through wns2-bl2p.notify.windows.com, over three separate trigger runs. So a current Chromium Edge uses WNS, not FCM, and notify.windows.com is not a legacy host. The same run had Chrome 152 on fcm.googleapis.com and Firefox 154 on updates.push.services.mozilla.com, which is what those two lines already said, so they stay as they are. Edge comes off the FCM line, notify.windows.com is described as current, and a short note is added to each block saying a Chromium Edge still lands on WNS. Without it a reader assumes Chromium implies FCM, which is the assumption that put Edge on the wrong line in the first place. Comments only. PUSH_SERVICE_HOSTS and PUSH_SERVICE_HOST_SUFFIXES are untouched, and the allow list was already correct because .notify.windows.com is a suffix entry, so Edge endpoints were accepted the whole time. Strip every comment line from the file and the remaining bytes are identical before and after.

…s-dedicated-push-queue-20260828 Fix Mobile Notifications push queue delivery

…al-security-evidence-20260827 test(security): tighten FILE-S01 evidence and log privacy

…260828

…st-comments-11x-20260827 docs(notifications): correct Edge WNS host guidance on 11.0.x

…ime-followup-20260828 ci: reduce full API test runtime

gnahk66 · 2026-08-30T15:22:57Z

Description

Fixes a cleanup issue in AcceptOverseerJob where the Overseer work directory under tmp/overseer/<task-id>-<assessment-id> was only removed when the job completed successfully.

If an exception occurred after the work directory was created, the existing rescue block re-raised the exception but the directory remained on disk.

This change moves the cleanup into an ensure block:

ensure
FileUtils.rm_rf(work_dir) if work_dir
end

Fixes # (AcceptOverseerJob leaks its overseer work directory on every failure and nothing else ever cleans it)
This ensures the job-specific work directory is removed on both success and failure while preserving the existing exception handling so Sidekiq can still record failures and retry them.

A regression test was added to force an exception after the work directory is created and verify that:

the exception is still re-raised;
the work directory is removed.

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The issue was reproduced manually by forcing an exception inside AcceptOverseerJob#perform after the Overseer work directory had been created.

Before the fix, the exception was raised and the job directory remained under tmp/overseer.

After moving the cleanup into ensure, the same failure was raised but the job-specific work directory was removed.

Regression test:
bin/rails test test/sidekiq/accept_overseer_job_test.rb
Result:
1 runs, 4 assertions, 0 failures, 0 errors, 0 skips

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if appropriate
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
I have created or extended unit tests to address my new additions
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

docs(pwa): document offline behaviour

…payloads test(notifications): assert every event produces a valid push payload

…ls-core security(api): apply Rails 8.0.x security patches

Every other event in this feature hangs off something a person did. A deadline getting closer is nobody doing anything, so there is no model to hook and it has to be swept for. SendDueSoonRemindersJob runs daily at 8am and reminds a student about a task due within three days that they still owe work on. Three days is long enough to act on over a weekend and short enough that it is about this task rather than the rest of the trimester. Daily and at a fixed time because a deadline only moves once a day, and because that also fixes when the emails land: on a shorter interval a student gets theirs at whatever hour their task crossed into the window. The hour follows TZ in the process environment, which development/api.env sets to Australia/Melbourne. Not sending twice is the hard part. The job runs again tomorrow and the task is still due soon tomorrow, so a Notification.exists? guard on user, type, event and link means one reminder per student per task. The index on (user_id, event) is what makes that cheap. Retrying the whole sweep is safe for the same reason. Recipients come from projects, not from Task rows: OnTrack creates a row the first time anyone touches the task, so the students who have not started have no row and they are the ones a reminder is for. The job never calls task_for_task_definition, which creates the row it cannot find, nor task_definitions_and_status, which calls it and runs two queries per definition per project. Definitions are read once per unit and tasks once per project. Per student deadlines come from Webcal.end_date_for_task_definition, the app's existing answer to this and what the calendar feed shows, so extensions and a flexible unit's grade level overrides are handled and cannot drift from what the student sees. discuss and demonstrate are excluded. Both mean the student has submitted and is waiting on a tutor. A failure on one project is logged and collected, and the collected ids are raised at the end so Sidekiq retries. Swallowing them would leave perform successful, and a task due today is filtered out as overdue tomorrow. Not verified end to end: there is no Sidekiq worker in the dev stack (EN-F03), so the schedule entry has not been seen to fire. Tests: 18 runs, 64 assertions, 0 failures, 0 errors.

payload_for now sets a tag, and the operating system does the collapsing. Two notifications sharing a tag means the second replaces the first on screen rather than stacking beneath it. The tag is the event plus the link. The link is the only handle the api has on the subject, and the event is what makes it a conversation rather than a topic: a run of comments on one task is one thing being said repeatedly, and a deadline change to the same task is something else that must not quietly replace it. notification_type is deliberately not used, because it is only the preference category, so task_due_date_changed, task_status_changed, new_task_available and task_due_soon are all `task` and would have shared a tag. The notification id is not in it either, which would collapse nothing at all. link is nullable, and with no link there is nothing to be about, so the tag falls back to the id and that notification collapses with nothing. One shared empty tag would hide every linkless notification behind the newest. renotify is false, so a replacement updates the banner without making a sound again. The case this exists for is a tutor posting five comments on one task in two minutes; the tag already collapses those, and renotify true would put the buzz back on every one of them. tag and renotify are both in NOTIFICATION_OPTION_NAMES in Angular's ngsw-worker.js, so both reach showNotification with no change on the web side. Title and body are untouched, and two of the new tests assert that, since the wording belongs to MN-D01 and MN-D05. Also adds require 'minitest/mock' to the test file. test_helper.rb does not pull it in, so Object#stub does not exist, and test_both_timeouts_are_passed_to_the_gem has been erroring with "undefined method 'stub' for module WebPush" since it was written. One line fixes that test too. Tests: 21 runs, 38 assertions, 0 failures, 0 errors.

…k-session fix(deps): update Rack and Rack Session

…h-dependencies fix(deps): update OAuth2 JWT and Ruby SAML

…p-client fix(deps): update Faraday and Addressable

…a-compat fix(deps): upgrade Puma to 7.2.1 for PROXY protocol fixes

feat(notifications): add due soon reminder job

…tifications feat(notifications): collapse repeat push notifications with a tag

…amplification-risk docs(notifications): add recipient amplification risk review

Adds `rake db:ppi_sample_data`, creating a deterministic minimum test data set for testing the Peer Progress Indicator dashboard: 2 units, 2 classes of 16 students per unit (4 students per target grade — Pass/Credit/Distinction/High Distinction), and 7 tasks per unit with completion rates scaled by target grade so percentages are clearly distinguishable across grade bands. Verified against the local dev environment: both units created successfully with 32 students and 7 tasks each, and completion percentages visibly differ by grade band in the Students tab.

Adds a permanent technical reference documenting where the Peer Progress Indicator's response fields actually come from in the backend, so PPI-B01, PPI-F01, PPI-S01 and future contributors don't have to rediscover it. Builds on the earlier PPI API discovery task (preserved here as a companion doc), which found the reusable aggregation infrastructure existed but wasn't reachable by students. This document goes further: it reviews the real, unmerged implementation on ppi/student-progress-endpoint (PPI-B01) and maps all 9 response fields to their exact source file/method, availability status, any transformation applied, and the ticket that owns each one. Includes a Mermaid data-flow diagram traced from the actual code (auth -> authorised project -> task validation -> server-side target-grade lookup -> nightly cohort aggregation -> small-cohort suppression -> percentage quantisation -> safe response -> frontend adapter -> existing widget), safe example responses for four business states, and a recorded list of concrete gaps found while reviewing the branch (unset config env vars, no unit has PPI enabled yet, a frontend/backend target-grade parameter mismatch, two frontend branches that independently renamed the same model, and a backfill migration that will blank every existing snapshot on first deploy). Does not implement the endpoint, frontend adapter, or any of the other PPI-* work explicitly out of scope for this ticket.

Quantising into buckets of B only hides the underlying count while B is strictly wider than one student's share of the cohort, 100.0 / cohort_size. A floor of 5 with a 5-point bucket satisfied neither side of that: for every cohort from 5 to 20 the mapping was injective and the returned percentage inverted to an exact submitted count. Move to a floor of 20 and a 10-point bucket, which leaves no cohort at or above the floor from which the count can be recovered, and add two tests that pin the relationship so the constants cannot drift apart again. Also scope the nightly aggregation to units that actually enabled the feature, rather than storing derived cohort statistics for every active unit and never serving them. Docs updated: the bucket size, the example response, the configuration floor, and the genuine-zero row, which is no longer true now that a small count also rounds to zero.

….x-all-features-20260824 release(api): OnTrack 11.0.x all-features handover

…l/test-preference-gating

…ability-paths fix(notifications): cover task availability paths

Both sides added NotificationEmailJob.drain at the same point in submit_portfolio, yours in 81016ac on this branch and mine in the repair that landed on feature/notifications via #65, so it came out as a conflict over one blank line and a comment. Kept the base version, which carries the explanation, so the file matches what is already on the shared branch and a later merge has nothing left to reconcile. 36 runs, 210 assertions, 0 failures across notification_service, notification_portfolio, notification_discussion_request and notification_email_job.

…miting fix(auth): add rate limiting to authentication endpoint

…rence-gating test(notifications): cover preference gating

…-mvp-20260827 # Conflicts: # .github/workflows/codeql.yml

… into closure/api-ontrack-mvp-20260827 # Conflicts: # app/api/peer_progress_api.rb # docs/peer-progress-api.md # docs/peer-progress/data-source-map.md # test/api/peer_progress_api_test.rb

…ure/api-ontrack-mvp-20260827 # Conflicts: # app/models/notification.rb # app/services/notification_service.rb # app/sidekiq/new_task_available_notification_job.rb # app/sidekiq/notification_email_job.rb # config/sidekiq.yml # db/schema.rb # test/api/settings_push_test.rb # test/models/notification_discussion_request_test.rb # test/models/notification_new_task_test.rb # test/services/notification_service_test.rb # test/sidekiq/scheduled_job_test.rb # test/sidekiq/send_new_task_available_notifications_job_test.rb

…ack-mvp-20260827 Close OnTrack MVP API integration and shard the test suite

Two comments were wrong. One grouped Edge under fcm.googleapis.com and the other described *.notify.windows.com as a legacy host used by legacy Edge. The MN-Q01 verification run on 27 Aug 2026 drove Microsoft Edge 151.0.4129.107 (Chromium 151.0.7922.174) on macOS 26.5.2 and it subscribed through wns2-bl2p.notify.windows.com, over three separate trigger runs. So a current Chromium Edge uses WNS, not FCM, and notify.windows.com is not a legacy host. The same run had Chrome 152 on fcm.googleapis.com and Firefox 154 on updates.push.services.mozilla.com, which is what those two lines already said, so they stay as they are. Edge comes off the FCM line, notify.windows.com is described as current, and a short note is added to each block saying a Chromium Edge still lands on WNS. Without it a reader assumes Chromium implies FCM, which is the assumption that put Edge on the wrong line in the first place. Comments only. PUSH_SERVICE_HOSTS and PUSH_SERVICE_HOST_SUFFIXES are untouched, and the allow list was already correct because .notify.windows.com is a suffix entry, so Edge endpoints were accepted the whole time. Strip every comment line from the file and the remaining bytes are identical before and after.

…s-dedicated-push-queue-20260828 Fix Mobile Notifications push queue delivery

…al-security-evidence-20260827 test(security): tighten FILE-S01 evidence and log privacy

…260828

…st-comments-11x-20260827 docs(notifications): correct Edge WNS host guidance on 11.0.x

…ime-followup-20260828 ci: reduce full API test runtime

gnahk66 · 2026-08-30T15:22:57Z

Description

Fixes a cleanup issue in AcceptOverseerJob where the Overseer work directory under tmp/overseer/<task-id>-<assessment-id> was only removed when the job completed successfully.

If an exception occurred after the work directory was created, the existing rescue block re-raised the exception but the directory remained on disk.

This change moves the cleanup into an ensure block:

ensure
FileUtils.rm_rf(work_dir) if work_dir
end

Fixes # (AcceptOverseerJob leaks its overseer work directory on every failure and nothing else ever cleans it)
This ensures the job-specific work directory is removed on both success and failure while preserving the existing exception handling so Sidekiq can still record failures and retry them.

A regression test was added to force an exception after the work directory is created and verify that:

the exception is still re-raised;
the work directory is removed.

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The issue was reproduced manually by forcing an exception inside AcceptOverseerJob#perform after the Overseer work directory had been created.

Before the fix, the exception was raised and the job directory remained under tmp/overseer.

After moving the cleanup into ensure, the same failure was raised but the job-specific work directory was removed.

Regression test:
bin/rails test test/sidekiq/accept_overseer_job_test.rb
Result:
1 runs, 4 assertions, 0 failures, 0 errors, 0 skips

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if appropriate
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
I have created or extended unit tests to address my new additions
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

docs(pwa): document offline behaviour

…payloads test(notifications): assert every event produces a valid push payload

…ls-core security(api): apply Rails 8.0.x security patches

Every other event in this feature hangs off something a person did. A deadline getting closer is nobody doing anything, so there is no model to hook and it has to be swept for. SendDueSoonRemindersJob runs daily at 8am and reminds a student about a task due within three days that they still owe work on. Three days is long enough to act on over a weekend and short enough that it is about this task rather than the rest of the trimester. Daily and at a fixed time because a deadline only moves once a day, and because that also fixes when the emails land: on a shorter interval a student gets theirs at whatever hour their task crossed into the window. The hour follows TZ in the process environment, which development/api.env sets to Australia/Melbourne. Not sending twice is the hard part. The job runs again tomorrow and the task is still due soon tomorrow, so a Notification.exists? guard on user, type, event and link means one reminder per student per task. The index on (user_id, event) is what makes that cheap. Retrying the whole sweep is safe for the same reason. Recipients come from projects, not from Task rows: OnTrack creates a row the first time anyone touches the task, so the students who have not started have no row and they are the ones a reminder is for. The job never calls task_for_task_definition, which creates the row it cannot find, nor task_definitions_and_status, which calls it and runs two queries per definition per project. Definitions are read once per unit and tasks once per project. Per student deadlines come from Webcal.end_date_for_task_definition, the app's existing answer to this and what the calendar feed shows, so extensions and a flexible unit's grade level overrides are handled and cannot drift from what the student sees. discuss and demonstrate are excluded. Both mean the student has submitted and is waiting on a tutor. A failure on one project is logged and collected, and the collected ids are raised at the end so Sidekiq retries. Swallowing them would leave perform successful, and a task due today is filtered out as overdue tomorrow. Not verified end to end: there is no Sidekiq worker in the dev stack (EN-F03), so the schedule entry has not been seen to fire. Tests: 18 runs, 64 assertions, 0 failures, 0 errors.

payload_for now sets a tag, and the operating system does the collapsing. Two notifications sharing a tag means the second replaces the first on screen rather than stacking beneath it. The tag is the event plus the link. The link is the only handle the api has on the subject, and the event is what makes it a conversation rather than a topic: a run of comments on one task is one thing being said repeatedly, and a deadline change to the same task is something else that must not quietly replace it. notification_type is deliberately not used, because it is only the preference category, so task_due_date_changed, task_status_changed, new_task_available and task_due_soon are all `task` and would have shared a tag. The notification id is not in it either, which would collapse nothing at all. link is nullable, and with no link there is nothing to be about, so the tag falls back to the id and that notification collapses with nothing. One shared empty tag would hide every linkless notification behind the newest. renotify is false, so a replacement updates the banner without making a sound again. The case this exists for is a tutor posting five comments on one task in two minutes; the tag already collapses those, and renotify true would put the buzz back on every one of them. tag and renotify are both in NOTIFICATION_OPTION_NAMES in Angular's ngsw-worker.js, so both reach showNotification with no change on the web side. Title and body are untouched, and two of the new tests assert that, since the wording belongs to MN-D01 and MN-D05. Also adds require 'minitest/mock' to the test file. test_helper.rb does not pull it in, so Object#stub does not exist, and test_both_timeouts_are_passed_to_the_gem has been erroring with "undefined method 'stub' for module WebPush" since it was written. One line fixes that test too. Tests: 21 runs, 38 assertions, 0 failures, 0 errors.

…k-session fix(deps): update Rack and Rack Session

…h-dependencies fix(deps): update OAuth2 JWT and Ruby SAML

…p-client fix(deps): update Faraday and Addressable

…a-compat fix(deps): upgrade Puma to 7.2.1 for PROXY protocol fixes

feat(notifications): add due soon reminder job

…tifications feat(notifications): collapse repeat push notifications with a tag

…amplification-risk docs(notifications): add recipient amplification risk review

Adds `rake db:ppi_sample_data`, creating a deterministic minimum test data set for testing the Peer Progress Indicator dashboard: 2 units, 2 classes of 16 students per unit (4 students per target grade — Pass/Credit/Distinction/High Distinction), and 7 tasks per unit with completion rates scaled by target grade so percentages are clearly distinguishable across grade bands. Verified against the local dev environment: both units created successfully with 32 students and 7 tasks each, and completion percentages visibly differ by grade band in the Students tab.

Adds a permanent technical reference documenting where the Peer Progress Indicator's response fields actually come from in the backend, so PPI-B01, PPI-F01, PPI-S01 and future contributors don't have to rediscover it. Builds on the earlier PPI API discovery task (preserved here as a companion doc), which found the reusable aggregation infrastructure existed but wasn't reachable by students. This document goes further: it reviews the real, unmerged implementation on ppi/student-progress-endpoint (PPI-B01) and maps all 9 response fields to their exact source file/method, availability status, any transformation applied, and the ticket that owns each one. Includes a Mermaid data-flow diagram traced from the actual code (auth -> authorised project -> task validation -> server-side target-grade lookup -> nightly cohort aggregation -> small-cohort suppression -> percentage quantisation -> safe response -> frontend adapter -> existing widget), safe example responses for four business states, and a recorded list of concrete gaps found while reviewing the branch (unset config env vars, no unit has PPI enabled yet, a frontend/backend target-grade parameter mismatch, two frontend branches that independently renamed the same model, and a backfill migration that will blank every existing snapshot on first deploy). Does not implement the endpoint, frontend adapter, or any of the other PPI-* work explicitly out of scope for this ticket.

Quantising into buckets of B only hides the underlying count while B is strictly wider than one student's share of the cohort, 100.0 / cohort_size. A floor of 5 with a 5-point bucket satisfied neither side of that: for every cohort from 5 to 20 the mapping was injective and the returned percentage inverted to an exact submitted count. Move to a floor of 20 and a 10-point bucket, which leaves no cohort at or above the floor from which the count can be recovered, and add two tests that pin the relationship so the constants cannot drift apart again. Also scope the nightly aggregation to units that actually enabled the feature, rather than storing derived cohort statistics for every active unit and never serving them. Docs updated: the bucket size, the example response, the configuration floor, and the genuine-zero row, which is no longer true now that a small count also rounds to zero.

….x-all-features-20260824 release(api): OnTrack 11.0.x all-features handover

…l/test-preference-gating

…ability-paths fix(notifications): cover task availability paths

Both sides added NotificationEmailJob.drain at the same point in submit_portfolio, yours in 81016ac on this branch and mine in the repair that landed on feature/notifications via #65, so it came out as a conflict over one blank line and a comment. Kept the base version, which carries the explanation, so the file matches what is already on the shared branch and a later merge has nothing left to reconcile. 36 runs, 210 assertions, 0 failures across notification_service, notification_portfolio, notification_discussion_request and notification_email_job.

…miting fix(auth): add rate limiting to authentication endpoint

…rence-gating test(notifications): cover preference gating

…-mvp-20260827 # Conflicts: # .github/workflows/codeql.yml

… into closure/api-ontrack-mvp-20260827 # Conflicts: # app/api/peer_progress_api.rb # docs/peer-progress-api.md # docs/peer-progress/data-source-map.md # test/api/peer_progress_api_test.rb

…ure/api-ontrack-mvp-20260827 # Conflicts: # app/models/notification.rb # app/services/notification_service.rb # app/sidekiq/new_task_available_notification_job.rb # app/sidekiq/notification_email_job.rb # config/sidekiq.yml # db/schema.rb # test/api/settings_push_test.rb # test/models/notification_discussion_request_test.rb # test/models/notification_new_task_test.rb # test/services/notification_service_test.rb # test/sidekiq/scheduled_job_test.rb # test/sidekiq/send_new_task_available_notifications_job_test.rb

…ack-mvp-20260827 Close OnTrack MVP API integration and shard the test suite

Two comments were wrong. One grouped Edge under fcm.googleapis.com and the other described *.notify.windows.com as a legacy host used by legacy Edge. The MN-Q01 verification run on 27 Aug 2026 drove Microsoft Edge 151.0.4129.107 (Chromium 151.0.7922.174) on macOS 26.5.2 and it subscribed through wns2-bl2p.notify.windows.com, over three separate trigger runs. So a current Chromium Edge uses WNS, not FCM, and notify.windows.com is not a legacy host. The same run had Chrome 152 on fcm.googleapis.com and Firefox 154 on updates.push.services.mozilla.com, which is what those two lines already said, so they stay as they are. Edge comes off the FCM line, notify.windows.com is described as current, and a short note is added to each block saying a Chromium Edge still lands on WNS. Without it a reader assumes Chromium implies FCM, which is the assumption that put Edge on the wrong line in the first place. Comments only. PUSH_SERVICE_HOSTS and PUSH_SERVICE_HOST_SUFFIXES are untouched, and the allow list was already correct because .notify.windows.com is a suffix entry, so Edge endpoints were accepted the whole time. Strip every comment line from the file and the remaining bytes are identical before and after.

…s-dedicated-push-queue-20260828 Fix Mobile Notifications push queue delivery

…al-security-evidence-20260827 test(security): tighten FILE-S01 evidence and log privacy

…260828

…st-comments-11x-20260827 docs(notifications): correct Edge WNS host guidance on 11.0.x

…ime-followup-20260828 ci: reduce full API test runtime

gnahk66 · 2026-08-30T15:22:57Z

Description

Fixes a cleanup issue in AcceptOverseerJob where the Overseer work directory under tmp/overseer/<task-id>-<assessment-id> was only removed when the job completed successfully.

If an exception occurred after the work directory was created, the existing rescue block re-raised the exception but the directory remained on disk.

This change moves the cleanup into an ensure block:

ensure
FileUtils.rm_rf(work_dir) if work_dir
end

Fixes # (AcceptOverseerJob leaks its overseer work directory on every failure and nothing else ever cleans it)
This ensures the job-specific work directory is removed on both success and failure while preserving the existing exception handling so Sidekiq can still record failures and retry them.

A regression test was added to force an exception after the work directory is created and verify that:

the exception is still re-raised;
the work directory is removed.

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The issue was reproduced manually by forcing an exception inside AcceptOverseerJob#perform after the Overseer work directory had been created.

Before the fix, the exception was raised and the job directory remained under tmp/overseer.

After moving the cleanup into ensure, the same failure was raised but the job-specific work directory was removed.

Regression test:
bin/rails test test/sidekiq/accept_overseer_job_test.rb
Result:
1 runs, 4 assertions, 0 failures, 0 errors, 0 skips

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if appropriate
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
I have created or extended unit tests to address my new additions
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

docs(pwa): document offline behaviour

…payloads test(notifications): assert every event produces a valid push payload

…ls-core security(api): apply Rails 8.0.x security patches

Every other event in this feature hangs off something a person did. A deadline getting closer is nobody doing anything, so there is no model to hook and it has to be swept for. SendDueSoonRemindersJob runs daily at 8am and reminds a student about a task due within three days that they still owe work on. Three days is long enough to act on over a weekend and short enough that it is about this task rather than the rest of the trimester. Daily and at a fixed time because a deadline only moves once a day, and because that also fixes when the emails land: on a shorter interval a student gets theirs at whatever hour their task crossed into the window. The hour follows TZ in the process environment, which development/api.env sets to Australia/Melbourne. Not sending twice is the hard part. The job runs again tomorrow and the task is still due soon tomorrow, so a Notification.exists? guard on user, type, event and link means one reminder per student per task. The index on (user_id, event) is what makes that cheap. Retrying the whole sweep is safe for the same reason. Recipients come from projects, not from Task rows: OnTrack creates a row the first time anyone touches the task, so the students who have not started have no row and they are the ones a reminder is for. The job never calls task_for_task_definition, which creates the row it cannot find, nor task_definitions_and_status, which calls it and runs two queries per definition per project. Definitions are read once per unit and tasks once per project. Per student deadlines come from Webcal.end_date_for_task_definition, the app's existing answer to this and what the calendar feed shows, so extensions and a flexible unit's grade level overrides are handled and cannot drift from what the student sees. discuss and demonstrate are excluded. Both mean the student has submitted and is waiting on a tutor. A failure on one project is logged and collected, and the collected ids are raised at the end so Sidekiq retries. Swallowing them would leave perform successful, and a task due today is filtered out as overdue tomorrow. Not verified end to end: there is no Sidekiq worker in the dev stack (EN-F03), so the schedule entry has not been seen to fire. Tests: 18 runs, 64 assertions, 0 failures, 0 errors.

payload_for now sets a tag, and the operating system does the collapsing. Two notifications sharing a tag means the second replaces the first on screen rather than stacking beneath it. The tag is the event plus the link. The link is the only handle the api has on the subject, and the event is what makes it a conversation rather than a topic: a run of comments on one task is one thing being said repeatedly, and a deadline change to the same task is something else that must not quietly replace it. notification_type is deliberately not used, because it is only the preference category, so task_due_date_changed, task_status_changed, new_task_available and task_due_soon are all `task` and would have shared a tag. The notification id is not in it either, which would collapse nothing at all. link is nullable, and with no link there is nothing to be about, so the tag falls back to the id and that notification collapses with nothing. One shared empty tag would hide every linkless notification behind the newest. renotify is false, so a replacement updates the banner without making a sound again. The case this exists for is a tutor posting five comments on one task in two minutes; the tag already collapses those, and renotify true would put the buzz back on every one of them. tag and renotify are both in NOTIFICATION_OPTION_NAMES in Angular's ngsw-worker.js, so both reach showNotification with no change on the web side. Title and body are untouched, and two of the new tests assert that, since the wording belongs to MN-D01 and MN-D05. Also adds require 'minitest/mock' to the test file. test_helper.rb does not pull it in, so Object#stub does not exist, and test_both_timeouts_are_passed_to_the_gem has been erroring with "undefined method 'stub' for module WebPush" since it was written. One line fixes that test too. Tests: 21 runs, 38 assertions, 0 failures, 0 errors.

…k-session fix(deps): update Rack and Rack Session

…h-dependencies fix(deps): update OAuth2 JWT and Ruby SAML

…p-client fix(deps): update Faraday and Addressable

…a-compat fix(deps): upgrade Puma to 7.2.1 for PROXY protocol fixes

feat(notifications): add due soon reminder job

…tifications feat(notifications): collapse repeat push notifications with a tag

…amplification-risk docs(notifications): add recipient amplification risk review

Adds `rake db:ppi_sample_data`, creating a deterministic minimum test data set for testing the Peer Progress Indicator dashboard: 2 units, 2 classes of 16 students per unit (4 students per target grade — Pass/Credit/Distinction/High Distinction), and 7 tasks per unit with completion rates scaled by target grade so percentages are clearly distinguishable across grade bands. Verified against the local dev environment: both units created successfully with 32 students and 7 tasks each, and completion percentages visibly differ by grade band in the Students tab.

Adds a permanent technical reference documenting where the Peer Progress Indicator's response fields actually come from in the backend, so PPI-B01, PPI-F01, PPI-S01 and future contributors don't have to rediscover it. Builds on the earlier PPI API discovery task (preserved here as a companion doc), which found the reusable aggregation infrastructure existed but wasn't reachable by students. This document goes further: it reviews the real, unmerged implementation on ppi/student-progress-endpoint (PPI-B01) and maps all 9 response fields to their exact source file/method, availability status, any transformation applied, and the ticket that owns each one. Includes a Mermaid data-flow diagram traced from the actual code (auth -> authorised project -> task validation -> server-side target-grade lookup -> nightly cohort aggregation -> small-cohort suppression -> percentage quantisation -> safe response -> frontend adapter -> existing widget), safe example responses for four business states, and a recorded list of concrete gaps found while reviewing the branch (unset config env vars, no unit has PPI enabled yet, a frontend/backend target-grade parameter mismatch, two frontend branches that independently renamed the same model, and a backfill migration that will blank every existing snapshot on first deploy). Does not implement the endpoint, frontend adapter, or any of the other PPI-* work explicitly out of scope for this ticket.

Quantising into buckets of B only hides the underlying count while B is strictly wider than one student's share of the cohort, 100.0 / cohort_size. A floor of 5 with a 5-point bucket satisfied neither side of that: for every cohort from 5 to 20 the mapping was injective and the returned percentage inverted to an exact submitted count. Move to a floor of 20 and a 10-point bucket, which leaves no cohort at or above the floor from which the count can be recovered, and add two tests that pin the relationship so the constants cannot drift apart again. Also scope the nightly aggregation to units that actually enabled the feature, rather than storing derived cohort statistics for every active unit and never serving them. Docs updated: the bucket size, the example response, the configuration floor, and the genuine-zero row, which is no longer true now that a small count also rounds to zero.

….x-all-features-20260824 release(api): OnTrack 11.0.x all-features handover

…l/test-preference-gating

…ability-paths fix(notifications): cover task availability paths

Both sides added NotificationEmailJob.drain at the same point in submit_portfolio, yours in 81016ac on this branch and mine in the repair that landed on feature/notifications via #65, so it came out as a conflict over one blank line and a comment. Kept the base version, which carries the explanation, so the file matches what is already on the shared branch and a later merge has nothing left to reconcile. 36 runs, 210 assertions, 0 failures across notification_service, notification_portfolio, notification_discussion_request and notification_email_job.

…miting fix(auth): add rate limiting to authentication endpoint

…rence-gating test(notifications): cover preference gating

…-mvp-20260827 # Conflicts: # .github/workflows/codeql.yml

… into closure/api-ontrack-mvp-20260827 # Conflicts: # app/api/peer_progress_api.rb # docs/peer-progress-api.md # docs/peer-progress/data-source-map.md # test/api/peer_progress_api_test.rb

…ure/api-ontrack-mvp-20260827 # Conflicts: # app/models/notification.rb # app/services/notification_service.rb # app/sidekiq/new_task_available_notification_job.rb # app/sidekiq/notification_email_job.rb # config/sidekiq.yml # db/schema.rb # test/api/settings_push_test.rb # test/models/notification_discussion_request_test.rb # test/models/notification_new_task_test.rb # test/services/notification_service_test.rb # test/sidekiq/scheduled_job_test.rb # test/sidekiq/send_new_task_available_notifications_job_test.rb

…ack-mvp-20260827 Close OnTrack MVP API integration and shard the test suite

Two comments were wrong. One grouped Edge under fcm.googleapis.com and the other described *.notify.windows.com as a legacy host used by legacy Edge. The MN-Q01 verification run on 27 Aug 2026 drove Microsoft Edge 151.0.4129.107 (Chromium 151.0.7922.174) on macOS 26.5.2 and it subscribed through wns2-bl2p.notify.windows.com, over three separate trigger runs. So a current Chromium Edge uses WNS, not FCM, and notify.windows.com is not a legacy host. The same run had Chrome 152 on fcm.googleapis.com and Firefox 154 on updates.push.services.mozilla.com, which is what those two lines already said, so they stay as they are. Edge comes off the FCM line, notify.windows.com is described as current, and a short note is added to each block saying a Chromium Edge still lands on WNS. Without it a reader assumes Chromium implies FCM, which is the assumption that put Edge on the wrong line in the first place. Comments only. PUSH_SERVICE_HOSTS and PUSH_SERVICE_HOST_SUFFIXES are untouched, and the allow list was already correct because .notify.windows.com is a suffix entry, so Edge endpoints were accepted the whole time. Strip every comment line from the file and the remaining bytes are identical before and after.

…s-dedicated-push-queue-20260828 Fix Mobile Notifications push queue delivery

…al-security-evidence-20260827 test(security): tighten FILE-S01 evidence and log privacy

…260828

…st-comments-11x-20260827 docs(notifications): correct Edge WNS host guidance on 11.0.x

…ime-followup-20260828 ci: reduce full API test runtime

gnahk66 · 2026-08-30T15:22:57Z

Description

Fixes a cleanup issue in AcceptOverseerJob where the Overseer work directory under tmp/overseer/<task-id>-<assessment-id> was only removed when the job completed successfully.

If an exception occurred after the work directory was created, the existing rescue block re-raised the exception but the directory remained on disk.

This change moves the cleanup into an ensure block:

ensure
FileUtils.rm_rf(work_dir) if work_dir
end

Fixes # (AcceptOverseerJob leaks its overseer work directory on every failure and nothing else ever cleans it)
This ensures the job-specific work directory is removed on both success and failure while preserving the existing exception handling so Sidekiq can still record failures and retry them.

A regression test was added to force an exception after the work directory is created and verify that:

the exception is still re-raised;
the work directory is removed.

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The issue was reproduced manually by forcing an exception inside AcceptOverseerJob#perform after the Overseer work directory had been created.

Before the fix, the exception was raised and the job directory remained under tmp/overseer.

After moving the cleanup into ensure, the same failure was raised but the job-specific work directory was removed.

Regression test:
bin/rails test test/sidekiq/accept_overseer_job_test.rb
Result:
1 runs, 4 assertions, 0 failures, 0 errors, 0 skips

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if appropriate
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
I have created or extended unit tests to address my new additions
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

docs(pwa): document offline behaviour

…payloads test(notifications): assert every event produces a valid push payload

…ls-core security(api): apply Rails 8.0.x security patches

Every other event in this feature hangs off something a person did. A deadline getting closer is nobody doing anything, so there is no model to hook and it has to be swept for. SendDueSoonRemindersJob runs daily at 8am and reminds a student about a task due within three days that they still owe work on. Three days is long enough to act on over a weekend and short enough that it is about this task rather than the rest of the trimester. Daily and at a fixed time because a deadline only moves once a day, and because that also fixes when the emails land: on a shorter interval a student gets theirs at whatever hour their task crossed into the window. The hour follows TZ in the process environment, which development/api.env sets to Australia/Melbourne. Not sending twice is the hard part. The job runs again tomorrow and the task is still due soon tomorrow, so a Notification.exists? guard on user, type, event and link means one reminder per student per task. The index on (user_id, event) is what makes that cheap. Retrying the whole sweep is safe for the same reason. Recipients come from projects, not from Task rows: OnTrack creates a row the first time anyone touches the task, so the students who have not started have no row and they are the ones a reminder is for. The job never calls task_for_task_definition, which creates the row it cannot find, nor task_definitions_and_status, which calls it and runs two queries per definition per project. Definitions are read once per unit and tasks once per project. Per student deadlines come from Webcal.end_date_for_task_definition, the app's existing answer to this and what the calendar feed shows, so extensions and a flexible unit's grade level overrides are handled and cannot drift from what the student sees. discuss and demonstrate are excluded. Both mean the student has submitted and is waiting on a tutor. A failure on one project is logged and collected, and the collected ids are raised at the end so Sidekiq retries. Swallowing them would leave perform successful, and a task due today is filtered out as overdue tomorrow. Not verified end to end: there is no Sidekiq worker in the dev stack (EN-F03), so the schedule entry has not been seen to fire. Tests: 18 runs, 64 assertions, 0 failures, 0 errors.

payload_for now sets a tag, and the operating system does the collapsing. Two notifications sharing a tag means the second replaces the first on screen rather than stacking beneath it. The tag is the event plus the link. The link is the only handle the api has on the subject, and the event is what makes it a conversation rather than a topic: a run of comments on one task is one thing being said repeatedly, and a deadline change to the same task is something else that must not quietly replace it. notification_type is deliberately not used, because it is only the preference category, so task_due_date_changed, task_status_changed, new_task_available and task_due_soon are all `task` and would have shared a tag. The notification id is not in it either, which would collapse nothing at all. link is nullable, and with no link there is nothing to be about, so the tag falls back to the id and that notification collapses with nothing. One shared empty tag would hide every linkless notification behind the newest. renotify is false, so a replacement updates the banner without making a sound again. The case this exists for is a tutor posting five comments on one task in two minutes; the tag already collapses those, and renotify true would put the buzz back on every one of them. tag and renotify are both in NOTIFICATION_OPTION_NAMES in Angular's ngsw-worker.js, so both reach showNotification with no change on the web side. Title and body are untouched, and two of the new tests assert that, since the wording belongs to MN-D01 and MN-D05. Also adds require 'minitest/mock' to the test file. test_helper.rb does not pull it in, so Object#stub does not exist, and test_both_timeouts_are_passed_to_the_gem has been erroring with "undefined method 'stub' for module WebPush" since it was written. One line fixes that test too. Tests: 21 runs, 38 assertions, 0 failures, 0 errors.

…k-session fix(deps): update Rack and Rack Session

…h-dependencies fix(deps): update OAuth2 JWT and Ruby SAML

…p-client fix(deps): update Faraday and Addressable

…a-compat fix(deps): upgrade Puma to 7.2.1 for PROXY protocol fixes

feat(notifications): add due soon reminder job

…tifications feat(notifications): collapse repeat push notifications with a tag

…amplification-risk docs(notifications): add recipient amplification risk review

Adds `rake db:ppi_sample_data`, creating a deterministic minimum test data set for testing the Peer Progress Indicator dashboard: 2 units, 2 classes of 16 students per unit (4 students per target grade — Pass/Credit/Distinction/High Distinction), and 7 tasks per unit with completion rates scaled by target grade so percentages are clearly distinguishable across grade bands. Verified against the local dev environment: both units created successfully with 32 students and 7 tasks each, and completion percentages visibly differ by grade band in the Students tab.

Adds a permanent technical reference documenting where the Peer Progress Indicator's response fields actually come from in the backend, so PPI-B01, PPI-F01, PPI-S01 and future contributors don't have to rediscover it. Builds on the earlier PPI API discovery task (preserved here as a companion doc), which found the reusable aggregation infrastructure existed but wasn't reachable by students. This document goes further: it reviews the real, unmerged implementation on ppi/student-progress-endpoint (PPI-B01) and maps all 9 response fields to their exact source file/method, availability status, any transformation applied, and the ticket that owns each one. Includes a Mermaid data-flow diagram traced from the actual code (auth -> authorised project -> task validation -> server-side target-grade lookup -> nightly cohort aggregation -> small-cohort suppression -> percentage quantisation -> safe response -> frontend adapter -> existing widget), safe example responses for four business states, and a recorded list of concrete gaps found while reviewing the branch (unset config env vars, no unit has PPI enabled yet, a frontend/backend target-grade parameter mismatch, two frontend branches that independently renamed the same model, and a backfill migration that will blank every existing snapshot on first deploy). Does not implement the endpoint, frontend adapter, or any of the other PPI-* work explicitly out of scope for this ticket.

Quantising into buckets of B only hides the underlying count while B is strictly wider than one student's share of the cohort, 100.0 / cohort_size. A floor of 5 with a 5-point bucket satisfied neither side of that: for every cohort from 5 to 20 the mapping was injective and the returned percentage inverted to an exact submitted count. Move to a floor of 20 and a 10-point bucket, which leaves no cohort at or above the floor from which the count can be recovered, and add two tests that pin the relationship so the constants cannot drift apart again. Also scope the nightly aggregation to units that actually enabled the feature, rather than storing derived cohort statistics for every active unit and never serving them. Docs updated: the bucket size, the example response, the configuration floor, and the genuine-zero row, which is no longer true now that a small count also rounds to zero.

….x-all-features-20260824 release(api): OnTrack 11.0.x all-features handover

…l/test-preference-gating

…ability-paths fix(notifications): cover task availability paths

Both sides added NotificationEmailJob.drain at the same point in submit_portfolio, yours in 81016ac on this branch and mine in the repair that landed on feature/notifications via #65, so it came out as a conflict over one blank line and a comment. Kept the base version, which carries the explanation, so the file matches what is already on the shared branch and a later merge has nothing left to reconcile. 36 runs, 210 assertions, 0 failures across notification_service, notification_portfolio, notification_discussion_request and notification_email_job.

…miting fix(auth): add rate limiting to authentication endpoint

…rence-gating test(notifications): cover preference gating

…-mvp-20260827 # Conflicts: # .github/workflows/codeql.yml

… into closure/api-ontrack-mvp-20260827 # Conflicts: # app/api/peer_progress_api.rb # docs/peer-progress-api.md # docs/peer-progress/data-source-map.md # test/api/peer_progress_api_test.rb

…ure/api-ontrack-mvp-20260827 # Conflicts: # app/models/notification.rb # app/services/notification_service.rb # app/sidekiq/new_task_available_notification_job.rb # app/sidekiq/notification_email_job.rb # config/sidekiq.yml # db/schema.rb # test/api/settings_push_test.rb # test/models/notification_discussion_request_test.rb # test/models/notification_new_task_test.rb # test/services/notification_service_test.rb # test/sidekiq/scheduled_job_test.rb # test/sidekiq/send_new_task_available_notifications_job_test.rb

…ack-mvp-20260827 Close OnTrack MVP API integration and shard the test suite

Two comments were wrong. One grouped Edge under fcm.googleapis.com and the other described *.notify.windows.com as a legacy host used by legacy Edge. The MN-Q01 verification run on 27 Aug 2026 drove Microsoft Edge 151.0.4129.107 (Chromium 151.0.7922.174) on macOS 26.5.2 and it subscribed through wns2-bl2p.notify.windows.com, over three separate trigger runs. So a current Chromium Edge uses WNS, not FCM, and notify.windows.com is not a legacy host. The same run had Chrome 152 on fcm.googleapis.com and Firefox 154 on updates.push.services.mozilla.com, which is what those two lines already said, so they stay as they are. Edge comes off the FCM line, notify.windows.com is described as current, and a short note is added to each block saying a Chromium Edge still lands on WNS. Without it a reader assumes Chromium implies FCM, which is the assumption that put Edge on the wrong line in the first place. Comments only. PUSH_SERVICE_HOSTS and PUSH_SERVICE_HOST_SUFFIXES are untouched, and the allow list was already correct because .notify.windows.com is a suffix entry, so Edge endpoints were accepted the whole time. Strip every comment line from the file and the remaining bytes are identical before and after.

…s-dedicated-push-queue-20260828 Fix Mobile Notifications push queue delivery

…al-security-evidence-20260827 test(security): tighten FILE-S01 evidence and log privacy

…260828

…st-comments-11x-20260827 docs(notifications): correct Edge WNS host guidance on 11.0.x

…ime-followup-20260828 ci: reduce full API test runtime

maplefoxgitand others added 30 commits August 15, 2026 23:44

Merge pull request#23from ontrack-features-t2-2026/push/pwa-offline
e1edf48
docs(pwa): document offline behaviour

test(notifications): assert every event produces a valid push payload
d286b89

Merge pull request#25from ontrack-features-t2-2026/push/test-event-…
3220368
…payloads test(notifications): assert every event produces a valid push payload

fix(deps): apply Rails 8.0.x security patches
54e2599

fix(deps): update Rack and Rack Session
57922d7

fix(deps): update OAuth2 JWT and Ruby SAML
0c9420c

fix(deps): update Faraday and Addressable
3cdfb6f

fix(deps): upgrade Puma to 7.2.1
efc8fa4

Merge pull request#26from ontrack-features-t2-2026/security/api-rai…
12318df
…ls-core security(api): apply Rails 8.0.x security patches

Merge pull request#27from ontrack-features-t2-2026/security/api-rac…
e89f03e
…k-session fix(deps): update Rack and Rack Session

Merge pull request#28from ontrack-features-t2-2026/security/api-aut…
83eec1d
…h-dependencies fix(deps): update OAuth2 JWT and Ruby SAML

Merge pull request#29from ontrack-features-t2-2026/security/api-htt…
997e92a
…p-client fix(deps): update Faraday and Addressable

Merge pull request#30from ontrack-features-t2-2026/security/api-pum…
490792a
…a-compat fix(deps): upgrade Puma to 7.2.1 for PROXY protocol fixes

Merge pull request#31from ontrack-features-t2-2026/email/due-soon
ae25c31
feat(notifications): add due soon reminder job

Merge pull request#32from ontrack-features-t2-2026/push/collapse-no…
395238a
…tifications feat(notifications): collapse repeat push notifications with a tag

docs(notifications): add recipient amplification risk review
97fdddb

fix(api): secure settings api against unauthenticated access
9a6b914

fix(ppi): quantise student progress percentages
814ceeb

fix(ppi): invalidate snapshots after grade changes
f99c2c3

fix(db): sync peer progress schema and collation
a8c0b18

style(ppi): satisfy rubocop
457ea76

fix(ppi): finalise privacy state safeguards
62ee298

fix(settings): separate public bootstrap from protected flags
cc17f4e

docs(notifications): correct amplification risk analysis
b7a6a29

Merge pull request#34from ontrack-features-t2-2026/email/recipient-…
d3be7c7
…amplification-risk docs(notifications): add recipient amplification risk review

maplefoxgitand others added 30 commits August 27, 2026 13:27

fix(ci): restore required RuboCop context
fc97680

Merge pull request#60from ontrack-features-t2-2026/integration/11.0…
eded7da
….x-all-features-20260824 release(api): OnTrack 11.0.x all-features handover

Merge remote-tracking branch 'origin/feature/notifications' into emai…
4cabc55
…l/test-preference-gating

test(notifications): drain queued email assertions
81016ac

Merge pull request#65from ontrack-features-t2-2026/email/task-avail…
f0f0e8f
…ability-paths fix(notifications): cover task availability paths

fix(auth): enforce shared authentication rate limits
b578424

Merge pull request#76from ontrack-features-t2-2026/fix/auth-rate-li…
cb03f80
…miting fix(auth): add rate limiting to authentication endpoint

Merge pull request#52from ontrack-features-t2-2026/email/test-prefe…
ad70886
…rence-gating test(notifications): cover preference gating

Merge remote-tracking branch 'origin/11.0.x' into closure/api-ontrack…
d16545a
…-mvp-20260827 # Conflicts: # .github/workflows/codeql.yml

Merge remote-tracking branch 'origin/feature/peer-progress-indicator'…
ef32dbb
… into closure/api-ontrack-mvp-20260827 # Conflicts: # app/api/peer_progress_api.rb # docs/peer-progress-api.md # docs/peer-progress/data-source-map.md # test/api/peer_progress_api_test.rb

test(security): assert upload validation paths
6c8ebff

fix(cpd): bound previous-unit project queries
d920892

ci: expand deterministic API test matrix
f25945d

test(notifications): reconcile protected push settings
c95f553

test(notifications): isolate portfolio receipt lookup
6c74dbb

Merge pull request#77from ontrack-features-t2-2026/closure/api-ontr…
6b45974
…ack-mvp-20260827 Close OnTrack MVP API integration and shard the test suite

test(security): tighten FILE-S01 evidence
42d4eba

fix(notifications): dedicate the push queue
b977a54

Merge pull request#95from ontrack-features-t2-2026/fix/notification…
1003df4
…s-dedicated-push-queue-20260828 Fix Mobile Notifications push queue delivery

Merge pull request#80from ontrack-features-t2-2026/fix/api-pr72-fin…
062e095
…al-security-evidence-20260827 test(security): tighten FILE-S01 evidence and log privacy

Merge remote-tracking branch 'origin/11.0.x' into refresh/api-pr79-20…
b5c015e
…260828

docs(notifications): scope the observed Edge endpoint
d4f00ef

Merge pull request#79from ontrack-features-t2-2026/docs/edge-wns-ho…
ea265b1
…st-comments-11x-20260827 docs(notifications): correct Edge WNS host guidance on 11.0.x

ci: reduce API test shard runtime
723b233

Merge pull request#96from ontrack-features-t2-2026/perf/api-ci-runt…
c085538
…ime-followup-20260828 ci: reduce full API test runtime

fix: clean up overseer work directory on failure
28d400f

style: fix overseer job formatting
c044d02