FOUNDATIONS: Add Unit Tests For Sdk Services Clients And Brokers

[cjdutoit]

Adds the first of three test tranches required to bring the solution in line with The Standard (.agents/skills/the-standard-testing, the-standard-cancellation-patterns, the-standard-foundations), following the Glory2Him.Core reference implementation for timeout and cancellation handling.

What this adds

188 unit tests across two previously empty test projects:

Suite	Coverage
CareIdentityServiceTests	logic, validations, exceptions, timeouts, cancellations
PdsServiceTests	logic, validations, exceptions, timeouts, cancellations
PdsOrchestrationServiceTests	logic, validations, exceptions, timeouts, cancellations
CareIdentityServiceProcessingServiceTests	logic, validations, exceptions, timeouts, cancellations
CareIdentityServiceClientTests	logic, exceptions, timeouts, cancellations
PersonalDemographicsServiceClientTests	logic, exceptions, timeouts, cancellations
SessionApiPlatform{State,Token}BrokerTests, ServiceCollectionExtensionsTests	ASP.NET Core session storage wiring

Every layer has both halves of the cancellation contract under test:

• Timeout — an OperationCanceledException raised while the caller has not cancelled becomes a logged dependency exception wrapping Timeout{Layer}Exception → TimeoutException("The dependency operation timed out.") (tsc-csharp-cp-012, tsc-csharp-cp-018).
• True cancellation — an OperationCanceledException raised while the caller has cancelled is rethrown unchanged and never wrapped, and nothing is logged (tsc-csharp-cp-013, tsc-csharp-cp-019).

Every exception test also asserts the exception was logged via SameExceptionAs.

Source changes required to make those tests pass

Cancellation and timeout

• Added the timeout-guarded catch ahead of the plain rethrow in every TryCatch and client catch chain (tsc-csharp-cp-011, tsc-csharp-cp-014). Previously cancellation was swallowed by catch (Exception) and rewrapped as a service exception.
• Added Timeout{Layer}Exception models for the foundation, processing, orchestration and client layers.
• Added cancellationToken.ThrowIfCancellationRequested() at the start of every cancellable operation (tsc-csharp-cp-005).

Deviation from the reference, and why. Glory2Him.Core discriminates on the exception's own token:

catch (OperationCanceledException operationCanceledException)
    when (operationCanceledException.CancellationToken.IsCancellationRequested is false)

That does not hold for HTTP. When HttpClient.Timeout elapses, .NET throws a TaskCanceledException whose CancellationToken is cancellation-requested (it carries the handler's internal timeout token), so the guard does not match and a genuine dependency timeout escapes to the caller as a raw TaskCanceledException. This SDK therefore threads the caller's token into TryCatch and guards on that instead:

catch (OperationCanceledException)
    when (cancellationToken.IsCancellationRequested is false)

Same semantics, and correct for both storage-style and HTTP-style dependencies. The end-to-end timeout tests in #27 (slow WireMock endpoint, 500 ms HttpClient.Timeout, no caller cancellation) fail against the reference guard and pass against this one.

Logging

The SDK had no logging at all, so CreateAndLog*Async had nothing to call. Added ILoggingBroker / LoggingBroker over ILogger<T>, registered via TryAddSingleton so a host can substitute its own, and renamed the exception factories to CreateAndLog*Async with LogErrorAsync calls throughout.

Other Standard violations fixed along the way

• CareIdentityService's outer TryCatch re-wrapped exceptions its own inner TryCatch had already mapped. It now rethrows its own exception types unchanged.
• A CSRF state mismatch threw a raw InvalidOperationException, surfacing a caller-side tampering check as a server fault. Now InvalidStateCareIdentityServiceException → validation exception.
• CareIdentityServiceProcessingService let all foundation exceptions fall through to a service exception. It now maps them to processing dependency / dependency-validation exceptions.
• PDS validation was dead code: ValidateOnSearchPatientsAsync was never called and its parameters did not match the API. Replaced with ValidateOnSearchPatients(accessToken, searchCriteria) on the foundation and ValidateOnSearchPatients(searchCriteria) on the orchestration (ts-foundations-002).
• Clients mapped dependency-validation exceptions onto *ClientValidationException; they now use *ClientDependencyValidationException.
• Removed the unused IApiPlatformTokenBroker dependency from PdsOrchestrationService.
• InternalsVisibleTo pointed at NHSDigital.ApiPlatform.Client.Tests.Unit, which is not a project in this solution.

Note for reviewers

These are behavioural changes to a published package. Callers who previously caught CareIdentityServiceServiceException on a timeout will now see CareIdentityServiceDependencyException; cancellation now surfaces as OperationCanceledException; and the service constructors take an extra ILoggingBroker. Worth a version bump before release.

Acceptance tests follow in #27, integration tests in #28.

Closes #29

---

Review remediation (added after the multi-agent review of this stack)

A 147-agent adversarial review of #26 → #27 → #28 raised 46 findings; 24 survived three-lens
verification. All of them are addressed in this stack. The ones that changed this PR:

The timeout guards above the foundation layer were dead code. CareIdentityService and
PdsService already convert any OperationCanceledException raised while the caller's token is
live into a dependency exception — so the only OCE that can escape them is a genuine cancellation,
and the identical guard at the processing, orchestration and client layers could never match. Four
of the six Timeout*Exception types I added were therefore unconstructable, and the unit tests
asserting those chains passed only because Moq threw a raw OCE that the real layer below cannot
produce. A live probe against a hanging endpoint returned
ClientDependencyException -> TimeoutCareIdentityServiceException -> TimeoutException, not the
chain the tests claimed. The dead guards, the unreachable catch (TimeoutException) blocks and the
four unusable exception types are gone, and the upper-layer tests now assert the chain production
actually produces.

Dependency 4xx is no longer reported as a dependency failure. Both foundation services funnelled
every non-success status into *DependencyException, leaving the DependencyValidation category
unreachable. HttpRequestException.StatusCode is now discriminated: 4xx becomes
*DependencyValidationException wrapping a new Invalid*DependencyException, 5xx and transport
failures stay *DependencyException.

NhsNumber is escaped before it reaches the PDS URL. It was the one search field concatenated
raw into the FHIR path while every other field went through Uri.EscapeDataString, so caller input
could steer an authenticated, bearer-token-bearing request elsewhere on the API host. Covered by a
test that feeds it ../../Practitioner/1?scope=all.

Tests that could not fail, or did not reach their target, are repaired. The four
ShouldNotWrapOperationCanceledExceptionRaisedByABroker tests pre-cancelled the token and so tripped
ThrowIfCancellationRequested before ever calling the broker; they now let the broker raise the
cancellation. IfStoredTokenHasExpired stored no token at all; it now stores an expired one, and the
60-second refresh skew has tests on both sides of the boundary.

Coverage gaps closed: the OAuth token-exchange form body (grant_type, code, redirect_uri,
client_id, client_secret) and the refresh-token grant are now asserted; so is the
Authorization: Bearer header on userinfo; so is Uri.EscapeDataString actually escaping something;
so is the refresh-token expiry round trip. All four client methods are exercised against every
exception category instead of one method per category.

Also: exception messages now match their category (dependency failures say "contact support",
not "fix the errors and try again"); ILoggingBroker is on the regenerated dependency graph; and
.github/workflows/build.yml no longer swallows failures — it looped dotnet test inside one
pwsh step, so only the last project's exit code counted and a failure in any earlier project
went green.

Consumer upgrade guide — 0.2.0.3 to 0.3.0.0

Version bumped to 0.3.0.0 on both packages. Breaking for anyone catching SDK exceptions:

Situation	0.2.0.3 threw	0.3.0.0 throws
CIS2/PDS returns 5xx or the connection fails	*ClientServiceException	*ClientDependencyException
CIS2/PDS returns 4xx	*ClientServiceException	*ClientDependencyValidationException
The dependency times out	*ClientServiceException	*ClientDependencyException wrapping Timeout*Exception -> TimeoutException
The caller cancels	*ClientServiceException	OperationCanceledException, unwrapped
CSRF state mismatch on callback	*ClientServiceException (wrapping InvalidOperationException)	*ClientDependencyValidationException
Already-cancelled token passed in	ran anyway	throws immediately

Service constructors take an additional ILoggingBroker. If you construct the services directly
rather than through AddApiPlatformSdkCore, you will need to supply one.

[Copilot]

Pull request overview

This PR adds substantial unit test coverage for the SDK’s Care Identity and PDS foundation/orchestration/processing/client layers, and updates SDK exception + cancellation behavior to align with “The Standard” requirements (notably: cancellation must not be wrapped, and timeouts/HTTP failures should map to dependency exceptions).

Changes:

• Added extensive unit tests across previously empty test projects for logic, validation, exception mapping, and cancellation propagation.
• Updated Care Identity and PDS services (and higher layers) to rethrow OperationCanceledException unwrapped and to map TimeoutException / HttpRequestException to dependency exceptions.
• Wired PDS search validation to the actual SearchCriteria model, removed an unused orchestration dependency, and corrected InternalsVisibleTo entries.

Reviewed changes

Copilot reviewed 55 out of 55 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
NHSDigital.ApiPlatform.Sdk/Services/Processings/CareIdentityServices/CareIdentityServiceProcessingService.Exceptions.cs	Processing-layer exception mapping updated (cancellation passthrough + foundation exception translation).
NHSDigital.ApiPlatform.Sdk/Services/Processings/CareIdentityServices/CareIdentityServiceProcessingService.cs	Added early cancellation checks to processing operations.
NHSDigital.ApiPlatform.Sdk/Services/Orchestrations/Pds/PdsOrchestrationService.Validations.cs	Replaced unused/incorrect validation with SearchCriteria-based validation + access-token validation.
NHSDigital.ApiPlatform.Sdk/Services/Orchestrations/Pds/PdsOrchestrationService.Exceptions.cs	Added orchestration exception mapping for null criteria + cancellation passthrough.
NHSDigital.ApiPlatform.Sdk/Services/Orchestrations/Pds/PdsOrchestrationService.cs	Orchestration now validates inputs, checks cancellation early, and removes unused dependency.
NHSDigital.ApiPlatform.Sdk/Services/Foundations/Pds/PdsService.Validations.cs	Foundation validation now validates accessToken + SearchCriteria (including null criteria).
NHSDigital.ApiPlatform.Sdk/Services/Foundations/Pds/PdsService.Exceptions.cs	Added dependency exception mapping for timeout/HTTP failures + cancellation passthrough.
NHSDigital.ApiPlatform.Sdk/Services/Foundations/Pds/PdsService.cs	Added early cancellation check and wired validation call in SearchPatientsAsync.
NHSDigital.ApiPlatform.Sdk/Services/Foundations/CareIdentityServices/CareIdentityService.Exceptions.cs	Added dependency exception mapping for timeout/HTTP failures + cancellation passthrough; fixed variable naming.
NHSDigital.ApiPlatform.Sdk/Services/Foundations/CareIdentityServices/CareIdentityService.cs	Added early cancellation checks to cancellable foundation operations.
NHSDigital.ApiPlatform.Sdk/NHSDigital.ApiPlatform.Sdk.csproj	Corrected/expanded InternalsVisibleTo for test assemblies.
NHSDigital.ApiPlatform.Sdk/Models/Orchestrations/Pds/Exceptions/NullSearchCriteriaPdsOrchestrationException.cs	New orchestration exception type for null search criteria.
NHSDigital.ApiPlatform.Sdk/Models/Foundations/Pds/Exceptions/NullSearchCriteriaPdsServiceException.cs	New foundation exception type for null search criteria.
NHSDigital.ApiPlatform.Sdk/Models/Foundations/Pds/Exceptions/FailedPdsServiceDependencyException.cs	New foundation dependency failure wrapper exception.
NHSDigital.ApiPlatform.Sdk/Models/Foundations/CareIdentityServices/Exceptions/FailedCareIdentityServiceDependencyException.cs	New foundation dependency failure wrapper exception.
NHSDigital.ApiPlatform.Sdk/Clients/PersonalDemographicsServices/PersonalDemographicsServiceClient.cs	Client cancellation passthrough + dependency-validation exception mapping.
NHSDigital.ApiPlatform.Sdk/Clients/CareIdentityServices/CareIdentityServiceClient.cs	Client cancellation passthrough + dependency-validation mapping + unexpected exception wrapping.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Processings/CareIdentityServices/CareIdentityServiceProcessingServiceTests.Validations.cs	New processing validation tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Processings/CareIdentityServices/CareIdentityServiceProcessingServiceTests.Logic.cs	New processing logic tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Processings/CareIdentityServices/CareIdentityServiceProcessingServiceTests.Exceptions.cs	New processing exception-mapping tests (incl. timeout).
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Processings/CareIdentityServices/CareIdentityServiceProcessingServiceTests.cs	Processing test fixture + shared data.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Processings/CareIdentityServices/CareIdentityServiceProcessingServiceTests.Cancellations.cs	New processing cancellation propagation tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Orchestrations/Pds/PdsOrchestrationServiceTests.Validations.SearchPatients.cs	New orchestration validation tests for search patients.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Orchestrations/Pds/PdsOrchestrationServiceTests.Logic.SearchPatients.cs	New orchestration logic tests for search patients.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Orchestrations/Pds/PdsOrchestrationServiceTests.Exceptions.SearchPatients.cs	New orchestration exception-mapping tests (incl. timeout).
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Orchestrations/Pds/PdsOrchestrationServiceTests.cs	Orchestration test fixture + shared data.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Orchestrations/Pds/PdsOrchestrationServiceTests.Cancellations.SearchPatients.cs	New orchestration cancellation propagation tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/Pds/PdsServiceTests.Validations.SearchPatients.cs	New PDS foundation validation tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/Pds/PdsServiceTests.Logic.SearchPatients.cs	New PDS foundation URL/header/role logic tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/Pds/PdsServiceTests.Exceptions.SearchPatients.cs	New PDS foundation exception-mapping tests (incl. unsuccessful responses).
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/Pds/PdsServiceTests.cs	PDS foundation test fixture + shared data.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/Pds/PdsServiceTests.Cancellations.SearchPatients.cs	New PDS foundation cancellation propagation tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/CareIdentityServices/CareIdentityServiceTests.Validations.cs	New Care Identity foundation validation tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/CareIdentityServices/CareIdentityServiceTests.Logic.Logout.cs	New Care Identity logout behavior tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/CareIdentityServices/CareIdentityServiceTests.Logic.GetUserInfo.cs	New Care Identity user-info retrieval tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/CareIdentityServices/CareIdentityServiceTests.Logic.GetAccessToken.cs	New Care Identity access-token retrieval/refresh tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/CareIdentityServices/CareIdentityServiceTests.Logic.Callback.cs	New Care Identity callback token/state/role persistence tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/CareIdentityServices/CareIdentityServiceTests.Logic.BuildLoginUrl.cs	New Care Identity login URL and CSRF storage tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/CareIdentityServices/CareIdentityServiceTests.Exceptions.cs	New Care Identity exception-mapping tests (incl. dependency timeout/HTTP).
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/CareIdentityServices/CareIdentityServiceTests.cs	Care Identity foundation test fixture + shared data.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/CareIdentityServices/CareIdentityServiceTests.Cancellations.cs	New Care Identity cancellation propagation tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Clients/PersonalDemographicsServices/PersonalDemographicsServiceClientTests.Logic.SearchPatients.cs	New PDS client logic tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Clients/PersonalDemographicsServices/PersonalDemographicsServiceClientTests.Exceptions.SearchPatients.cs	New PDS client exception-mapping tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Clients/PersonalDemographicsServices/PersonalDemographicsServiceClientTests.cs	PDS client test fixture + shared data.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Clients/PersonalDemographicsServices/PersonalDemographicsServiceClientTests.Cancellations.SearchPatients.cs	New PDS client cancellation propagation tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Clients/CareIdentityServices/CareIdentityServiceClientTests.Logic.cs	New Care Identity client logic tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Clients/CareIdentityServices/CareIdentityServiceClientTests.Exceptions.cs	New Care Identity client exception-mapping tests.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Clients/CareIdentityServices/CareIdentityServiceClientTests.cs	Care Identity client test fixture + shared data.
NHSDigital.ApiPlatform.Sdk.Tests.Unit/Clients/CareIdentityServices/CareIdentityServiceClientTests.Cancellations.cs	New Care Identity client cancellation propagation tests.
NHSDigital.ApiPlatform.Sdk.AspNetCore.Tests.Unit/ServiceCollectionExtensionsTests.cs	New DI wiring tests for ASP.NET Core integration.
NHSDigital.ApiPlatform.Sdk.AspNetCore.Tests.Unit/Brokers/Storages/SessionApiPlatformTokenBrokerTests.cs	New session token broker behavior tests.
NHSDigital.ApiPlatform.Sdk.AspNetCore.Tests.Unit/Brokers/Storages/SessionApiPlatformStateBrokerTests.cs	New session state broker behavior tests.
NHSDigital.ApiPlatform.Sdk.AspNetCore.Tests.Unit/Brokers/Storages/FakeSession.cs	Test helper for ASP.NET Core session-based brokers.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 55 out of 55 changed files in this pull request and generated no new comments.

Suppressed comments (4)

NHSDigital.ApiPlatform.Sdk/Services/Foundations/Pds/PdsService.cs:68

• SearchCriteria.DateOfBirth is a string, but the interpolation uses a date format specifier (:yyyy-MM-dd). This will not compile and also disagrees with the unit tests that expect the raw string value in the query.
  NHSDigital.ApiPlatform.Sdk/Services/Foundations/Pds/PdsService.cs:76
• GetActiveRoleAsync returns string?, but this assigns it to a non-nullable string. This introduces nullable-reference warnings and misrepresents the possible null value.
  NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Orchestrations/Pds/PdsOrchestrationServiceTests.Exceptions.SearchPatients.cs:22
• This method signature line is 121 characters long, exceeding the 120-character limit (line 21). Wrap the declaration so each physical line is �120 characters.

        public async Task ShouldThrowDependencyValidationExceptionOnSearchPatientsIfDependencyValidationErrorOccursAsync(
            Xeption dependencyValidationException)

NHSDigital.ApiPlatform.Sdk/Services/Foundations/CareIdentityServices/CareIdentityService.Validations.cs:35

• expectedState is retrieved as string? but ValidateStateMatches requires a non-nullable string, forcing a nullable conversion warning at the call site. Making the parameter nullable matches actual usage and keeps the validation logic unchanged.

[Copilot]

Pull request overview

Copilot reviewed 66 out of 66 changed files in this pull request and generated no new comments.

Suppressed comments (6)

NHSDigital.ApiPlatform.Sdk/Services/Foundations/Pds/PdsService.cs:72

• SearchCriteria.DateOfBirth is a string (Models/Foundations/Pds/SearchCriteria.cs), so the interpolation format specifier :yyyy-MM-dd is ignored at runtime. This is misleading and may suggest formatting is being enforced when it isn't. Prefer appending the string value as-is (or parse/validate explicitly if formatting is required).
  NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/CareIdentityServices/CareIdentityServiceTests.Timeouts.cs:115
• Line 115 exceeds the 120-character limit (121 characters). Wrap the method declaration so each physical line is <= 120 characters.

        public async Task ShouldThrowDependencyExceptionOnGetAccessTokenIfOperationCanceledExceptionOccursAndLogItAsync()

NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Foundations/Pds/PdsServiceTests.Timeouts.SearchPatients.cs:20

• Line 20 exceeds the 120-character limit (121 characters). Wrap the method declaration so each physical line is <= 120 characters.

        public async Task ShouldThrowDependencyExceptionOnSearchPatientsIfOperationCanceledExceptionOccursAndLogItAsync()

NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Orchestrations/Pds/PdsOrchestrationServiceTests.Cancellations.SearchPatients.cs:19

• Line 19 exceeds the 120-character limit (121 characters). Wrap the method declaration so each physical line is <= 120 characters.

        public async Task ShouldThrowDependencyExceptionOnSearchPatientsIfOperationCanceledExceptionOccursAndLogItAsync()

NHSDigital.ApiPlatform.Sdk.Tests.Unit/Services/Orchestrations/Pds/PdsOrchestrationServiceTests.Exceptions.SearchPatients.cs:21

• Line 21 exceeds the 120-character limit (121 characters). Wrap the method declaration so each physical line is <= 120 characters.

        public async Task ShouldThrowDependencyValidationExceptionOnSearchPatientsIfDependencyValidationErrorOccursAsync(

NHSDigital.ApiPlatform.Sdk/Models/Clients/Pds/Exceptions/TimeoutPersonalDemographicsServiceClientException.cs:13

• Line 13 exceeds the 120-character limit (124 characters). Wrap the constructor signature so each physical line is <= 120 characters.

[Copilot]

Pull request overview

Copilot reviewed 66 out of 66 changed files in this pull request and generated no new comments.

Suppressed comments (3)

NHSDigital.ApiPlatform.Sdk/Services/Foundations/Pds/PdsService.cs:72

• SearchCriteria.DateOfBirth is a string, but the interpolated value uses a date format specifier (:yyyy-MM-dd). That format specifier is ignored for strings (and suggests this is a DateTime/DateOnly), which makes the URL construction misleading. Consider removing the format specifier (and URL-encoding the value for consistency with the other query parameters).
  NHSDigital.ApiPlatform.Sdk/Services/Orchestrations/Pds/PdsOrchestrationService.Exceptions.cs:177
• The dependency exception message says "fix the errors and try again", which reads like a validation error. For dependency failures this is not actionable; consider changing it to something like "...please contact support" to match the other dependency exception messages in the SDK.
  NHSDigital.ApiPlatform.Sdk/Services/Orchestrations/Pds/PdsOrchestrationService.Exceptions.cs:140
• The dependency exception message says "fix the errors and try again", which reads like a validation error. For dependency/timeout failures this is not actionable and is inconsistent with the other dependency messages in the SDK (e.g., "...please contact support"). Consider updating the message for PdsOrchestrationDependencyException created in the timeout path.

This issue also appears on line 174 of the same file.

[Copilot]

Pull request overview

Copilot reviewed 66 out of 66 changed files in this pull request and generated no new comments.

Suppressed comments (5)

NHSDigital.ApiPlatform.Sdk/Services/Orchestrations/Pds/PdsOrchestrationService.Exceptions.cs:177

• This is a dependency exception path, but the message says "fix the errors and try again". That wording is typically reserved for validation errors; for dependency failures callers usually can’t remediate and the message should direct them to contact support.
  NHSDigital.ApiPlatform.Sdk/Services/Orchestrations/Pds/PdsOrchestrationService.Exceptions.cs:138
• This is a dependency exception path, but the message says "fix the errors and try again". That wording is typically reserved for validation errors; for dependency failures callers usually can’t remediate and the message should direct them to contact support.

This issue also appears on line 174 of the same file.
NHSDigital.ApiPlatform.Sdk/Services/Foundations/CareIdentityServices/CareIdentityService.cs:49

• Missing blank line between the constructor and the next method declaration; this breaks the project’s member-separation formatting convention and reduces readability.
  NHSDigital.ApiPlatform.Sdk/Clients/CareIdentityServices/CareIdentityServiceClient.cs:239
• The dependency-validation exception factory uses the same message as the pure validation exception ("validation error occurred"). This makes logs and surfaced errors ambiguous; consider stating "dependency validation" in the message to match the exception type.
  NHSDigital.ApiPlatform.Sdk/Clients/PersonalDemographicsServices/PersonalDemographicsServiceClient.cs:105
• The dependency-validation exception factory uses the same message as the pure validation exception ("validation error occurred"). This makes logs and surfaced errors ambiguous; consider stating "dependency validation" in the message to match the exception type.

[Copilot]

Pull request overview

Copilot reviewed 71 out of 71 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 72 out of 72 changed files in this pull request and generated no new comments.

Suppressed comments (1)

NHSDigital.ApiPlatform.Sdk/Services/Foundations/Pds/PdsService.cs:80

• GetActiveRoleAsync returns string? (see IApiPlatformTokenBroker), but activeRoleId is declared as non-nullable string. This can produce nullable reference warnings and obscures the fact that null is a valid value here.