ENG-3302: Add hook skeleton for reply polling and notifications

[JadeCara]

Ticket ENG-3302

Description Of Changes

Adds inert hook skeletons in Fides OSS that Fidesplus will later use to inject business logic for IMAP reply polling and DSR lifecycle notifications. Follows the established Jira polling pattern (service/jira/polling_task.py).

Reply polling — a single scheduled Celery task that polls an IMAP mailbox on an interval. Straightforward poll pattern.

Notifications — hybrid design with two delivery paths:

1. Event-driven (primary): send_dsr_notification.delay(privacy_request_id, event_type) — called directly by DSR lifecycle code for immediate delivery when a state change occurs.
2. Sweep (secondary): sweep_notifications — scheduled task that runs on an interval to catch missed or failed notifications.

Each hook provides a register_* function, a Celery task with Redis lock (sweep only — the event-driven task doesn't need one), and APScheduler wiring for the scheduled tasks. All are no-ops until Fidesplus registers implementations.

Relationship to messaging/: These new packages (correspondence/ and notifications/) are domain features, not transport. They decide what and when to send; the existing messaging/ package handles how (email/SMS delivery via SES, Twilio, etc.). Correspondence will use messaging as its transport layer but is intentionally separate — it owns threading, reply-to tokens, inbound reply processing, and delivery tracking, which don't belong in a generic transport package.

Code Changes

• Added src/fides/service/correspondence/ package with reply_polling_task.py (scheduled poll)
• Added src/fides/service/notifications/ package with notification_task.py (event-driven notify + scheduled sweep_notifications)
• Added reply_polling_interval_minutes and notification_interval_minutes config fields to ExecutionSettings
• Wired initiate_reply_polling() and initiate_notification_task() into the application lifespan in main.py
• 22 tests covering registration, no-op, delegation, lock contention, exception propagation, scheduler wiring, and config defaults

Steps to Confirm

1. Start Fides with nox -s "dev(slim)" -- fides-pkg and confirm it starts without errors
2. Check the startup logs for Initiating scheduler for reply mailbox polling and Initiating scheduler for DSR notification sweep — both should appear
3. Wait for the polling interval (default 3min for reply polling, 5min for notification sweep) and confirm the tasks fire with the no-op debug logs: Reply mailbox polling: no service registered, skipping and DSR notification sweep: no service registered, skipping
4. Optionally override the interval via env var (e.g. FIDES__EXECUTION__REPLY_POLLING_INTERVAL_MINUTES=1) and confirm the task fires on the new interval

Pre-Merge Checklist

• Issue requirements met
• All CI pipelines succeeded
• CHANGELOG.md updated
  • Add a db-migration label
  • Add a high-risk label
  • Updates unreleased work already in Changelog
• UX feedback:
  • All UX related changes have been reviewed by a designer
  • No UX review needed
• Followup issues:
  • Followup issues created
  • No followup issues
• Database migrations:
  • Ensure that your downrev is up to date
  • Ensure that your downgrade() migration is correct
  • No migrations
• Documentation:
  • Documentation complete
  • Documentation issue created
  • New client scopes documentation
  • No documentation updates required

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ready	Preview, Comment	May 13, 2026 8:20pm

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
fides-privacy-center	Ignored		May 13, 2026 8:20pm

[JadeCara]

/code-review

[claude]

Code Review — PR #8158: Hook skeletons for reply mailbox polling and DSR notifications

This PR adds two new Celery task skeletons following the established Jira polling pattern. The implementation is clean, well-tested, and the no-op / registration design is solid. A few observations below.

Summary of findings

Correctness improvement (minor inconsistency)
Both new modules correctly use raise RuntimeError to guard against a non-running scheduler, which is better than the existing Jira task's assert scheduler.running — assert statements are silently disabled when Python runs with -O. Worth aligning the Jira task to use the same pattern.

Module naming concern
src/fides/service/notifications/ sits alongside src/fides/service/messaging/, which already handles outbound notification delivery. The names are close enough that future contributors may be unsure which package to extend. A more scoped name or an explicit disambiguation comment in __init__.py would help.

Module-level global assumption
_service_fn is effectively "set once at startup, read many times." This is safe under CPython's GIL, but the assumption is implicit. A brief comment stating the expected lifecycle would make the intent clear.

Test gaps

• Both test files hardcode the expected default interval values (== 3, == 5). These will silently drift if the defaults are changed.
• Neither test file covers the case where the registered service function raises an exception. Since that exception propagates to Celery as a task failure, the behavior should be documented and tested.

What looks good

• The two-file structure (task + __init__.py docstring) mirrors the Jira pattern well.
• Lock timeout (600 s), coalesce=True, and replace_existing=True are all consistent with the Jira task.
• Modern type annotations (Callable[[Session], None] | None over Optional[Callable]) are an improvement over the Jira baseline.
• Test coverage for lock-not-acquired, no-service-registered, scheduler-not-running, and test-mode-skip paths is thorough.
• Config defaults and env-prefix wiring look correct.

---

🔬 Codegraph: connected (49703 nodes)

---

💡 Write /code-review in a comment to re-run this review.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.50%. Comparing base (a1fc5a6) to head (9ca0e51).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8158      +/-   ##
==========================================
+ Coverage   85.47%   85.50%   +0.02%     
==========================================
  Files         651      653       +2     
  Lines       42434    42513      +79     
  Branches     4982     4991       +9     
==========================================
+ Hits        36272    36350      +78     
  Misses       5055     5055              
- Partials     1107     1108       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[JadeCara]

/code-review

[claude]

Code Review: PR #8158 — Hook Skeleton for Reply Polling and Notifications

The overall structure is clean and follows the Jira polling precedent well. The dual-path hybrid design (event-driven notify + scheduled sweep_notifications) is well-reasoned, and the test coverage is thorough. Two critical issues should be fixed before merge; several suggestions would strengthen the cross-repo contract before Fidesplus builds on top of this.

---

Critical (Should Fix Before Merge)

1. Generic task name notify (notification_task.py:62)
The bare name notify is extremely common and will be ambiguous in Redis queues and Celery monitoring. More critically, Fidesplus will import this task object by name to call .delay() — if the task is renamed or moved later, callers break silently at runtime rather than at import time. Rename to send_dsr_notification or dispatch_dsr_notification now, before the cross-repo dependency is established.

2. Eager Session import (reply_polling_task.py:14, notification_task.py:22)
Session is only referenced in module-level type annotations but is imported live, forcing SQLAlchemy to be loaded in any process that imports these modules — including lightweight Celery beat workers that never touch the DB. Fix with TYPE_CHECKING:

from __future__ import annotations
from typing import TYPE_CHECKING
if TYPE_CHECKING:
    from sqlalchemy.orm import Session

---

Suggestions

3. Inconsistent error handling (reply_polling_task.py:67) — New code uses raise RuntimeError(...) where the existing Jira task uses assert. The RuntimeError approach is actually better (asserts are stripped by -O), but the inconsistency looks like an oversight. Either update the Jira task in this PR or add a comment explaining the divergence.

4. event_type: str should be a typed enum/Literal (notification_task.py:62) — A bare str means a typo at the Fidesplus call site will be silently accepted and produce a no-op at runtime. Defining DSREventType = Literal["request_completed", "request_approved", ...] in the OSS layer now (before Fidesplus builds on it) is much cheaper than a cross-repo change later.

5. Process-model safety documentation (reply_polling_task.py:26) — The existing comment only addresses GIL/thread safety. Celery workers typically use process-based concurrency (fork), and post-fork registration won't propagate to sibling workers. Fidesplus must register before workers spawn. This constraint should be documented explicitly in the comment.

6. Missing comment on why notify has no lock (notification_task.py:61) — sweep_notifications has a lock; notify intentionally doesn't (concurrent per-request execution is expected). Without a comment, a future engineer may add a lock here in error.

---

Nits

7. Magic lock timeout (reply_polling_task.py:23, notification_task.py:31) — 600 seconds is not self-explanatory. A short inline comment (# 10 min: accommodates slow IMAP connections) would make the reasoning clear.

8. Re-registration not tested (test_reply_polling_task.py:18, test_notification_task.py) — The registration tests only verify the happy path. A single test that calls register_* twice and confirms the second call overwrites the first would document expected behavior and prevent surprises during Fidesplus test isolation or hot-reload.

---

🔬 Codegraph: unavailable

💡 Write /code-review in a comment to re-run this review.

[dsill-ethyca]

Tiny description nit: the PR body still references notify.delay(privacy_request_id, event_type), but the task was renamed to send_dsr_notification during review (correctly — notify was too generic). Code is fine; just worth updating the description so future readers don't grep for a function that doesn't exist.