Joseph Doherty
9e7697359e
Register row 45. NotificationOutboxActor.ResolveAdapters() cached adapter instances in an actor-lifetime IServiceScope (_adaptersScope), but both EmailNotificationDeliveryAdapter and SmsNotificationDeliveryAdapter are AddScoped and hold a scoped INotificationRepository over one EF DbContext. MaxParallelDeliveries (default 4) then drove up to four concurrent deliveries through that single adapter and single context, which central-b logged continuously on the rig as "InvalidOperationException: A second operation was started on this context instance before a previous operation completed". The collided delivery's row is never written and the wasted attempt counts against its retry budget, so sustained contention could park a legitimate notification. The per-delivery scope DeliverGatedAsync already opened for the outbox repository — added for exactly this hazard — did not cover the adapter. _adaptersCache, _adaptersScope and ResolveAdapters() are deleted. Delivery adapters are now resolved inside the per-delivery scope via a static ResolveAdapter(IServiceProvider, NotificationType), preserving DI's last-wins resolution semantics (the enumeration runs to completion rather than short-circuiting) and the null-adapter park path. Nothing shared is lost: the Email adapter builds a fresh SMTP client per attempt from an injected factory and the SMS adapter draws pooled connections from the singleton IHttpClientFactory, so no adapter carried state worth preserving across deliveries. The Central UI's NotificationChannelCatalog is an independent consumer of the same scoped registrations and is untouched, so the adapter-gated Type selector still enumerates exactly the deliverable channels. Tests: Dispatch_ResolvesAdaptersOnce_AcrossMultipleSweeps (which pinned the superseded caching behaviour) is replaced by ParallelDeliveries_EachGetTheirOwnAdapterAndDbContext — two parallel deliveries through a scoped repository whose simulated DbContext throws EF's exact second-operation exception on overlap, with a rendezvous that guarantees the overlap window — and Dispatch_ResolvesAdapterPerDelivery_NotOncePerActorLifetime. Both fail against the pre-fix actor (stash-verified: 1 of 2 rows persisted; 1 distinct adapter instead of 4) and pass after. Parallel-delivery ordering/throughput semantics are unchanged. Docs: Component-NotificationOutbox.md gains the scoped-adapter contract under Delivery Adapters (resolve per delivery; adapters carry no cross-delivery state; genuinely shared resources belong in a singleton/factory), and the NotificationOutbox-006 finding is marked SUPERSEDED with the reasoning. Solution build clean; NotificationOutbox (153), AuditLog (383), integration NotificationOutbox flow (2) and CentralUI notification (14) suites green.
Code Reviews
Comprehensive, per-module code reviews of the ScadaBridge codebase. Each module (one
buildable project under src/) has its own folder containing a findings.md. This
README is the aggregated index — the single place to see all outstanding work.
Generated by
regen-readme.pyfrom the per-modulefindings.mdfiles. Do not edit by hand — edit the findings files and re-run the script.
How it works
- Reviews are performed one module at a time against a fixed checklist.
- Every finding is recorded in the module's
findings.mdwith a severity and status. - Findings are never deleted — they are closed by changing their status, keeping a full audit trail.
- This README aggregates every pending finding (
Open/In Progress) across all modules.
See REVIEW-PROCESS.md for the full procedure: the review checklist, severity definitions, finding format, and how to mark items resolved.
Layout
code-reviews/
├── README.md # this file — process overview + pending findings
├── REVIEW-PROCESS.md # how to perform a review and track findings
├── regen-readme.py # regenerates this README from the findings files
├── _template/findings.md # copy-this template for a module review
└── <Module>/findings.md # one folder per src/ project
Baseline review — 2026-05-16
All 19 modules were reviewed at commit 9c60592 (241 findings: 6 Critical, 46 High,
100 Medium, 89 Low). The tables below track what remains open as findings are
resolved and re-triaged; findings discovered after the baseline are appended to their
module file and counted in Total.
| Severity | Open findings |
|---|---|
| Critical | 0 |
| High | 0 |
| Medium | 0 |
| Low | 0 |
| Total | 0 |
Module Status
| Module | Last reviewed | Commit | Open (C/H/M/L) | Open | Total |
|---|---|---|---|---|---|
| AuditLog | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 17 |
| CLI | 2026-06-19 | d6ead8ae |
0/0/0/0 | 0 | 24 |
| CentralUI | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 37 |
| ClusterInfrastructure | 2026-06-20 | 4307c381 |
0/0/0/0 | 0 | 15 |
| Commons | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 26 |
| Communication | 2026-06-20 | 4307c381 |
0/0/0/0 | 0 | 24 |
| ConfigurationDatabase | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 27 |
| DataConnectionLayer | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 27 |
| DeploymentManager | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 27 |
| ExternalSystemGateway | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 26 |
| HealthMonitoring | 2026-06-20 | 4307c381 |
0/0/0/0 | 0 | 25 |
| Host | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 26 |
| InboundAPI | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 31 |
| KpiHistory | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 6 |
| ManagementService | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 26 |
| NotificationOutbox | 2026-06-19 | d6ead8ae |
0/0/0/0 | 0 | 13 |
| NotificationService | 2026-06-20 | 4307c381 |
0/0/0/0 | 0 | 28 |
| ScriptAnalysis | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 9 |
| Security | 2026-06-20 | 4307c381 |
0/0/0/0 | 0 | 25 |
| SiteCallAudit | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 9 |
| SiteEventLogging | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 27 |
| SiteRuntime | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 33 |
| StoreAndForward | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 28 |
| TemplateEngine | 2026-06-24 | 1f9de8a2 |
0/0/0/0 | 0 | 25 |
| Transport | 2026-06-19 | d6ead8ae |
0/0/0/0 | 0 | 15 |
Pending Findings
Every Open / In Progress finding across all modules, highest severity first.
Resolved findings drop off this list but remain recorded in their module's
findings.md (see REVIEW-PROCESS.md §4–§5). Full detail —
description, location, recommendation — lives in the module's findings.md.
Critical (0)
None open.
High (0)
None open.
Medium (0)
None open.
Low (0)
None open.