ENG-3324: System data-stewards change hook for inheritance

[adamsachs]

Ticket ENG-3324

📦 Now bundles FE work from #8015 (ENG-3330) — the inherit_system_stewards toggle in the monitor configuration forms. Merged in via feat/monitor-steward-inheritance--ENG-3330 so the BE hooks and the FE control ship together. The conflict in ConfigureMonitorForm.tsx was resolved against current main's handleNextClicked shape (the flat isEditing ? {...} : {...} ternary), with inherit_system_stewards: values.inherit_system_stewards threaded into both payload branches. #8015 should be closed in favor of this PR.

🔗 Coupled with fidesplus #3564 — the propagation PR that consumes both hook registries. The fidesplus requirements.txt there pins to this branch; both should merge together (or this one first).

Description Of Changes

Adds two temporary single-purpose callback registries under fides.api that let fidesplus react when state backing monitor stewardship inheritance changes, plus the FE toggle that exposes the inherit_system_stewards flag on the monitor configuration forms. Both registries follow the same shim posture, are loudly documented, and will be deleted when the event framework (#8096) lands.

1. system_steward_change_hooks — fires when a user is added/removed as a data steward (system manager) of a system. Instrumented in the three v1 routes that mutate the systemmanager join.
2. system_connection_config_link_change_hooks — fires when the system_integration_link row backing a ConnectionConfig is added, replaced, or removed. Instrumented in the SystemIntegrationLinkService mutators (set_links / delete_link), driven from the corresponding PUT / DELETE routes under /connection/{key}/system-links.
3. Monitor configuration FE — Inherit system stewards toggle on both ConfigureMonitorForm (databases / detection-discovery monitors) and ConfigureWebsiteMonitorForm. Defaults true on create, reflects the saved value on edit. Replaces the old "default stewards to the linked system's data_stewards" behavior on the website monitor form — that auto-population now happens server-side via the inherit flag.

Now that #7888 (the data-model PR) has merged, this PR is based directly on main. Pairs with the fidesplus propagation PR (ENG-3324/monitor-steward-inheritance-propagation, fidesplus #3564), which registers the inheritance-reconcile callbacks at app startup.

The hook modules are loudly documented as TEMPORARY scaffolding for the event framework (#8096), with cutover instructions in their docstrings and a "do not generalize this into a hooks package" note to keep us from accreting more shims around them. They also call out the FK-cascade caveat: deleting a System or ConnectionConfig outright bypasses Python and won't fire either hook — consumers must rely on a periodic reconciler as the convergence safety net for those.

Code Changes

Steward-change registry

• src/fides/api/system_steward_change_hooks.py — single-purpose registry exposing register_system_steward_change_hook(callback) and notify_system_stewards_changed(background_tasks, system_id). Failure-isolated (each hook wrapped in try/except, logged not raised). Idempotent registration. ~50 LOC.
• src/fides/api/v1/endpoints/system.py:bulk_assign_steward — injects BackgroundTasks, calls notify_system_stewards_changed after each successful set_as_system_manager.
• src/fides/api/v1/endpoints/user_endpoints.py:update_managed_systems — same pattern; fires on both adds and removes (it loops over a diff).
• src/fides/api/v1/endpoints/user_endpoints.py:remove_user_as_system_manager — fires after the remove.
• src/fides/api/v1/endpoints/user_permission_endpoints.py:update_user_permissions — when this route demotes a user to APPROVER and consequently removes all their system-manager assignments, fires once per affected system.
• Model methods (FidesUser.set/remove_as_system_manager) are unchanged. Test/conftest callers don't trigger propagation by accident.

Link-change registry

• src/fides/api/system_connection_config_link_change_hooks.py — same shim shape as the steward registry; ~50 LOC.
• src/fides/system_integration_link/service.py — set_links and delete_link accept an optional background_tasks; when provided and a write actually happened, they fire the link-change hooks.
• src/fides/system_integration_link/routes.py — PUT and DELETE system-links routes inject BackgroundTasks and pass it through to the service.

Monitor configuration FE (from #8015)

• clients/admin-ui/src/types/api/models/EditableMonitorConfig.ts — adds inherit_system_stewards?: boolean | null to the type.
• clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorForm.tsx — new Inherit system stewards Form.Item (Switch), interface field, initial-value logic (defaults true on create), and inherit_system_stewards threaded into the upsert payload for both edit and create.
• clients/admin-ui/src/features/integrations/configure-monitor/ConfigureWebsiteMonitorForm.tsx — mirror of the above for website monitors. Replaces the old "auto-populate stewards from systemData.data_stewards when no explicit stewards" behavior, since the server now handles inheritance.

Tests

• tests/api/test_system_steward_change_hooks.py — 5 unit tests for the steward registry (idempotency, multi-hook dispatch, failure isolation, empty-registry no-op).
• tests/api/test_system_connection_config_link_change_hooks.py — 5 mirror tests for the link registry.
• tests/system_integration_link/test_service.py — 4 service-level tests confirming dispatch fires on writes when background_tasks is supplied and stays silent otherwise / on no-op deletes.

Steps to Confirm

Best confirmed end-to-end alongside the paired fidesplus propagation PR (#3564), but the BE registries and the FE toggle are independently testable.

Backend registries (isolated)

1. pytest tests/api/test_system_steward_change_hooks.py tests/api/test_system_connection_config_link_change_hooks.py tests/system_integration_link/test_service.py — 19 tests pass.

FE toggle (admin-ui)

2. From clients/admin-ui, start the dev server. Open the integration monitor configuration modal (both database / discovery monitors and website monitors). Confirm the Inherit system stewards toggle is visible, defaults to on when creating a new monitor, and reflects the saved value when editing an existing one.
3. Toggle it off, save, reopen — confirm the toggle persists as off.

End-to-end (requires the paired fidesplus branch + a seeded property/integration)

4. With both branches checked out: create a System and MonitorConfig with inherit_system_stewards=true (use the new FE toggle). POST /api/v1/system/assign-steward to add a user → on the fidesplus side, the monitor's GET /stewards should reflect the user as inherited within a tick (BG task fires after response).
5. DELETE /api/v1/user/{user_id}/system-manager/{system_key} → the inherited row disappears.
6. DELETE /api/v1/connection/{conn_key}/system-links/{system_fides_key} on the monitor's connection → all inherited rows on that monitor disappear (no longer linked to a system).
7. PUT /api/v1/connection/{conn_key}/system-links to relink → inherited rows return.

Pre-Merge Checklist

• Issue requirements met
• All CI pipelines succeeded
• CHANGELOG.md updated
  • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration (n/a)
  • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
  • Updates unreleased work already in Changelog, no new entry necessary
• UX feedback:
  • All UX related changes have been reviewed by a designer
  • No UX review needed
• Followup issues:
  • Followup issues created (both hook modules are intended to be removed when the event framework lands — track separately)
  • No followup issues
• Database migrations:
  • Ensure that your downrev is up to date with the latest revision on main
  • Ensure that your downgrade() migration is correct and works
    • If a downgrade migration is not possible for this change, please call this out in the PR description!
  • No migrations
• Documentation:
  • Documentation complete, PR opened in fidesdocs
  • Documentation issue created in fidesdocs
  • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
  • No documentation updates required

🤖 Generated with Claude Code

[vercel]

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

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ignored	Preview	May 15, 2026 4:45pm
fides-privacy-center	Ignored		May 15, 2026 4:45pm

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.62%. Comparing base (4d36209) to head (a9bda8c).
⚠️ Report is 133 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8172      +/-   ##
==========================================
+ Coverage   84.97%   85.62%   +0.65%     
==========================================
  Files         631      660      +29     
  Lines       41239    42901    +1662     
  Branches     4787     5022     +235     
==========================================
+ Hits        35041    36734    +1693     
+ Misses       5113     5063      -50     
- Partials     1085     1104      +19     

☔ 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.

[adamsachs]

/code-review

[claude]

Code Review — ENG-3324: System↔ConnectionConfig link change hooks

Overall the approach is clean and well-scoped. The two shim modules are ~50 LOC each, clearly temporary, loudly documented (especially the cascade-delete caveat and the "don't generalize this" warning), and come with solid unit tests. The failure-isolation pattern in notify_* is good defensive practice. A few things worth addressing before merge:

---

Substantive

set_links fires hook even on logical no-ops (see inline)
The docstring says "when a write actually happened," but the guard is just background_tasks is not None. Since the implementation always does delete_all + re-insert, a DB write always occurs when existing links are present or new ones are supplied. However, calling set_links([], background_tasks=bg) when no links existed fires the hook despite zero state change. A secondary case: set_links([same_link]) deletes and recreates the same row, fires the hook, and triggers unnecessary consumer reconciliation. Both are false positives that cause fidesplus to run inheritance reconciliation when nothing changed.

The fix is straightforward: track whether had_existing or results before calling notify_*. At minimum the docstring should accurately describe the actual firing condition.

Missing test for the set_links([])-with-no-prior-links edge case (see inline)
The new TestLinkChangeHookDispatch class doesn't cover this path.

---

Minor

No changelog fragment
changelog/8172-*.yaml doesn't exist. The checklist item is unchecked — if this is intentional (internal plumbing, no user-visible change), mark it explicitly; otherwise add a fragment under Added or Changed.

_HOOKS thread-safety (see inline)
Fine for startup-time registration, but worth a one-line comment that registration must complete before request handling begins.

update_user_permissions hook call is asymmetric (see inline)
Nit-level: the removal path is hooked but a future promotion path would need explicit instrumentation. A short comment would prevent the oversight.

---

Positive highlights

• The autouse isolated_registry fixture pattern in tests is the right approach — avoids cross-test pollution without exposing an unregister API.
• test_delete_link_does_not_fire_when_link_missing correctly asserts the hook stays silent when SystemIntegrationLinkNotFoundError is raised (exception path exits before the hook call).
• background_tasks is optional in the service methods, keeping internal callers (tests, scripts) clean.
• The cascade-delete caveat in the docstring is a valuable heads-up for future readers.

🔬 Codegraph: connected (50452 nodes)

---

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

[github-actions]

Title	Lines	Statements	Branches	Functions
admin-ui		6.89% (3150/45706)	6.26% (1655/26434)	4.78% (647/13521)
fides-js		79.17% (1977/2497)	66.25% (1249/1885)	73.31% (349/476)
privacy-center		82.53% (364/441)	79.74% (189/237)	74.07% (60/81)

[vcruces]

Looks good! Clean, well-scoped temporary scaffolding

[vcruces]

Good clarification!