fix: own a DI scope per fire-and-forget handover push send - #1669
Merged
Conversation
The three fire-and-forget push sends captured the request-scoped IPushNotificationService/DbContext; after the gRPC request completed the scope was disposed and SendToSiteAsync threw ObjectDisposedException (live-verified on tenant 855). Each send now runs in its own scope via IServiceScopeFactory. Also elevates the two silent-skip log lines to Information so skipped sends are visible at default log level. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four regression tests inside ContentHandoverServiceTests (CI shard allowlist) pinning: push service resolved from an owned scope which is disposed after the send; accept/reject payloads target the requester; push failures are swallowed with the scope still disposed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a production ObjectDisposedException by ensuring the handover-related fire-and-forget push notifications do not capture request-scoped services (e.g., IPushNotificationService/DbContext) after the gRPC request completes. Instead, each background send now creates and disposes its own DI scope.
Changes:
- Refactors
ContentHandoverServiceto resolveIPushNotificationServicefrom a new scope per push-send usingIServiceScopeFactory, and routes the three fire-and-forget sends through scope-owning async methods. - Elevates two “silent skip” push-notification log messages from Debug to Information.
- Updates and adds regression tests to validate the scope-owning push-send contract and disposal behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/PushNotificationService/PushNotificationService.cs | Promotes push-skip logs (Firebase not configured / no tokens) to Information for improved visibility. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/ContentHandoverService/ContentHandoverService.cs | Avoids capturing request-scoped services by creating an owned DI scope per background push send via IServiceScopeFactory. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PushNotificationIntegrationTests.cs | Updates test wiring for ContentHandoverService constructor change (scope factory instead of direct push service). |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ContentHandoverServiceTests.cs | Adds regression tests that assert push sends resolve from an owned scope and that the scope is disposed even on failure. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ContentHandoverRemovedRowTests.cs | Updates test wiring for the new ContentHandoverService constructor dependency (scope factory). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the live ObjectDisposedException on tenant 855: the three fire-and-forget handover push sends captured the request-scoped IPushNotificationService/DbContext, which was disposed when the gRPC request completed. Each send now creates and owns its scope via IServiceScopeFactory; the two silent-skip log lines are elevated to Information; four regression tests pin the contract.
Spec: flutter-adhoc/docs/superpowers/specs/2026-08-02-flutter-time-push-fixes-design.md
🤖 Generated with Claude Code