Skip to content

feat(gateway): add ServiceRequest hydration and work item management#19

Merged
rsalus merged 60 commits intomainfrom
feature/integration-gateway-servicerequest-hydration
Feb 2, 2026
Merged

feat(gateway): add ServiceRequest hydration and work item management#19
rsalus merged 60 commits intomainfrom
feature/integration-gateway-servicerequest-hydration

Conversation

@rsalus
Copy link
Contributor

@rsalus rsalus commented Feb 1, 2026

Summary

Implements ServiceRequest hydration, work item state management, CRUD endpoints, and Alba integration tests for the Gateway API. This enables the prior authorization workflow to detect treatments requiring PA via ServiceRequest resources, track work items through their lifecycle, and provides full work item management via REST API with Athena sandbox configuration.

Changes

  • ServiceRequest Hydration — Add ServiceRequestInfo model, extend IFhirClient with SearchServiceRequestsAsync, and include ServiceRequests in parallel FhirDataAggregator fetch
  • Work Item Management — Create WorkItem model with WorkItemStatus enum, IWorkItemStore interface, and thread-safe InMemoryWorkItemStore implementation
  • CRUD Endpoints — Add POST, GET, PUT endpoints for work items with list filters and status updates
  • Rehydrate Endpoint — Add POST /api/work-items/{id}/rehydrate with optional AccessToken to re-fetch and re-analyze
  • Alba Integration Tests — Add 11 Alba-based integration tests for all WorkItem CRUD endpoints
  • Program.cs Refactor — Move CORS, OpenAPI, HealthChecks to fluent extension methods
  • Athena Configuration — Add appsettings.Development.json with sandbox URLs, update AppHost for Athena secrets

Test Plan

Added 50 new TUnit tests including 11 Alba integration tests. All 221 tests pass with 0 errors.


Results: Tests 221 ✓ · Build 0 errors
Design: docs/designs/2026-01-29-athenahealth-pivot-mvp.md
Plans: gateway-servicerequest-hydration.md, workitem-crud-preview.md

rsalus and others added 28 commits January 31, 2026 20:32
Add FHIR ServiceRequest resource model for representing orders/referrals
that require prior authorization. Includes required properties (Id, Status,
Code) and optional properties (EncounterId, AuthoredOn).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add request model for the re-hydration endpoint that allows users to
trigger a re-fetch of clinical data when a work item has missing information.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Introduces a WorkItemStatus enum to represent the lifecycle states of
prior authorization work items: ReadyForReview, MissingData,
PayerRequirementsNotMet, Submitted, and NoPaRequired.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add method signature for searching ServiceRequest FHIR resources (orders/referrals)
for a patient, with optional encounter filtering.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Define CRUD operations for work item persistence
- Support query by ID and encounter
- Include status update with validation hook

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add ServiceRequest search with optional encounter filtering
- Extract CPT codes from CodeableConcept structure
- Parse encounter reference to extract ID
- Follow existing FhirClient patterns

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Include SearchServiceRequestsAsync in parallel clinical data fetch
- Populate ServiceRequests in ClinicalBundle result
- Log service request count with other clinical data

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add thread-safe in-memory storage for work items
- Support CRUD operations via ConcurrentDictionary
- Include query by encounter ID

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add POST /api/work-items/{id}/rehydrate endpoint
- Re-fetch clinical data via FhirDataAggregator
- Re-analyze with IntelligenceClient
- Update work item status based on analysis result
- Register IWorkItemStore in DI container

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add IWorkItemStore registration as singleton
- Enable work item persistence for MVP
- Add DI resolution test

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add sealed modifier to WorkItem record
- Remove duplicate IWorkItemStore DI registration
- Fix race condition in InMemoryWorkItemStore.UpdateStatusAsync with atomic TryUpdate
- Document placeholder token with issue reference for tracking

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rsalus and others added 2 commits January 31, 2026 22:53
Sets launchUrl to scalar/v1 so API docs open by default.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds JsonStringEnumConverter so API accepts and returns enum names
like "Submitted" instead of integers like 3.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@apps/gateway/Gateway.API/DependencyExtensions.cs`:
- Around line 25-39: The AddCorsPolicy method currently hardcodes
WithOrigins("http://localhost:5173"); change it to read allowed origins from
configuration instead: accept an IConfiguration (or resolve it from services)
and retrieve a configured origins list (e.g., comma- or array-based setting) and
pass that into policy.WithOrigins(...) (or policy.SetIsOriginAllowed when using
dynamic logic). Update AddCorsPolicy to use the configuration key (e.g.,
"Cors:AllowedOrigins") and fall back to a safe default if missing, keeping the
rest of the policy (AllowAnyHeader, AllowAnyMethod, AllowCredentials) intact.

The work item ID already comes from the route parameter /{id}/rehydrate.
Having it in the request body was redundant and caused 400 errors
when sending an empty body.

Now POST /api/work-items/{id}/rehydrate accepts:
- Empty body: {}
- Optional access token: {"accessToken": "..."}

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Fix all issues with AI agents
In `@docs/designs/2026-01-31-workitem-crud-preview.md`:
- Around line 344-366: The RehydrateRequest model still declares WorkItemId but
the implementation uses the route id; remove the WorkItemId property and its XML
doc from the RehydrateRequest record in Gateway.API/Models (class
RehydrateRequest) and update any example snippets that instantiate or reference
RehydrateRequest to stop passing WorkItemId—keep the AccessToken? property and
its comment as the only field, and adjust comments/examples to reflect that the
work item id comes from the route rather than the request body.
- Around line 33-47: The fenced code block under "Base Path" (containing
`/api/work-items`) needs a language identifier (e.g., use ```text) to satisfy
MD040/MD060, and the "Endpoints" table should be formatted with proper spacing:
ensure a blank line before and after the table and add a single space around
pipes/cell content so the table is not compact; apply the same fixes to the
other occurrences referenced in the comment.

In `@docs/pitch/design-reconciliation.md`:
- Line 27: The fenced code blocks near the "#### 1. Updated
`AthenaPollingService`" section (and the other fenced block around line 105) are
missing language tags and trigger markdownlint MD040; update the triple-backtick
fences surrounding those code blocks to include an appropriate language
identifier (e.g., ```text or ```mermaid) so the blocks are tagged, making the
fenced blocks adjacent to the "AthenaPollingService" heading and the block at
the later section compliant.

In `@docs/plans/2026-01-31-gateway-servicerequest-hydration.md`:
- Line 27: Locate the fenced code block that contains the line "Run: `dotnet
test` - MUST FAIL" and add a language tag (for example, use `text`) to its
opening ``` fence so markdownlint MD040 is satisfied; also find the fenced block
intended for a diagram/flow and add the appropriate language tag (for example,
`mermaid`) to that opening ``` fence to fix the second occurrence flagged (the
block around the diagram content).

In `@docs/plans/2026-01-31-workitem-crud-preview.md`:
- Around line 289-316: Add explicit language tags to the four fenced code blocks
shown in the diff to satisfy markdownlint MD040: append a language (e.g., `text`
or `mermaid`) to the block containing "Task 013 (CreateWorkItemRequest)  |  Task
014 (UpdateStatusRequest)  |  Task 015 (WorkItemListResponse)  |  Task 016
(RehydrateRequest)  |  Task 017 (GetAllAsync)", the block under "Sequential
Chain: Endpoints" that contains "Group 1 Complete → Task 018 (CRUD endpoint
methods) → Task 019 (Route registration)", the block labeled "Parallel after
Task 016" containing "Task 016 (RehydrateRequest update) → Task 020 (Use
AccessToken in RehydrateAsync)", and the "Execution Graph" fenced block showing
the ASCII timeline; choose `text` for plain text blocks or `mermaid` if you
prefer diagram rendering and update those three/four triple-backtick openings
accordingly.
🧹 Nitpick comments (2)
docs/debugging/AllResources-20260131104348.txt (1)

1-392: Consider whether log files belong in version control.

This appears to be a captured runtime log from a debugging session. While it may be useful for immediate troubleshooting, log files typically become stale quickly and aren't ideal for long-term version control. Consider these alternatives:

  • Add docs/debugging/*.txt or similar patterns to .gitignore
  • Use ephemeral debugging sessions or container logs that don't persist in git
  • If you need to preserve specific debugging scenarios, consider creating a runbook or troubleshooting guide instead, documenting the issue and resolution rather than raw logs

Also note that the log contains local filesystem paths (/home/reedsalus/Documents/code/prior-auth/) which may not be relevant to other developers.

apps/gateway/Gateway.API.Tests/Integration/WorkItemEndpointsIntegrationTests.cs (1)

11-12: Consider sealing the test class for guideline consistency.
The coding standard says “sealed by default”; applying it here keeps tests aligned with project conventions.

rsalus and others added 2 commits January 31, 2026 23:48
Replace direct IConfiguration parameter passing with modern .NET patterns:
- Use BindConfiguration() for options binding (resolves IConfiguration from DI)
- Use ConfigureHttpClient((sp, client) => ...) for deferred HTTP client config
- Add IConfigureOptions<HybridCacheOptions> for HybridCache configuration
- Add IFhirTokenProvider for server-side FHIR token management
- Add API key authentication with X-API-Key header validation

Also removes accessToken from all FHIR interfaces - tokens now managed
internally by FhirHttpClient via IFhirTokenProvider.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Addresses CodeRabbit feedback - previously the helper returned silently
when timeout elapsed, making failures harder to diagnose.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@apps/gateway/Gateway.API/Services/Polling/AthenaPollingService.cs`:
- Around line 169-179: The catch for InvalidOperationException around
_fhirClient.SearchAsync is too broad and hides configuration errors; update
error handling in AthenaPollingService so it only swallows token-acquisition
failures from the token provider: have IFhirTokenProvider throw a specific
exception type (e.g., TokenAcquisitionException) for token errors and change the
catch to only catch that type (or inspect and rethrow non-token
InvalidOperationExceptions), and ensure configuration-related exceptions
(missing PrivateKeyPath, invalid token strategy) are not caught but allowed to
bubble up or be rethrown for proper logging/alerting.
🧹 Nitpick comments (8)
apps/gateway/Gateway.API/Services/Fhir/FhirHttpClient.cs (2)

25-30: Consider adding null guards for constructor parameters.

Defensive null validation on injected dependencies ensures fail-fast behavior and clearer error messaging during DI misconfiguration. This is a common C# best practice.

🛡️ Proposed fix
 public FhirHttpClient(HttpClient httpClient, IFhirTokenProvider tokenProvider, ILogger<FhirHttpClient> logger)
 {
-    _httpClient = httpClient;
-    _tokenProvider = tokenProvider;
-    _logger = logger;
+    _httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient));
+    _tokenProvider = tokenProvider ?? throw new ArgumentNullException(nameof(tokenProvider));
+    _logger = logger ?? throw new ArgumentNullException(nameof(logger));
 }

Or using the modern .NET 6+ pattern:

 public FhirHttpClient(HttpClient httpClient, IFhirTokenProvider tokenProvider, ILogger<FhirHttpClient> logger)
 {
+    ArgumentNullException.ThrowIfNull(httpClient);
+    ArgumentNullException.ThrowIfNull(tokenProvider);
+    ArgumentNullException.ThrowIfNull(logger);
     _httpClient = httpClient;
     _tokenProvider = tokenProvider;
     _logger = logger;
 }

145-171: ReadBinaryAsync bypasses ConfigureRequest, omitting the Accept header.

Other methods use ConfigureRequest which sets both the Authorization and Accept: application/fhir+json headers. This method sets Authorization directly and omits Accept. If this is intentional (binary content shouldn't request FHIR+JSON), it's fine—but worth a brief inline comment to clarify the design choice for future maintainers.

📝 Suggested clarifying comment
     var accessToken = await _tokenProvider.GetTokenAsync(ct);

     using var request = new HttpRequestMessage(HttpMethod.Get, $"Binary/{id}");
+    // Note: Intentionally not using ConfigureRequest - binary resources don't use application/fhir+json
     request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", accessToken);
apps/gateway/Gateway.API.Tests/Services/FhirTokenProviderTests.cs (1)

32-55: Consider using TUnit's built-in exception assertion.

The manual try-catch works but TUnit provides cleaner patterns for asserting exceptions.

✨ Cleaner exception assertion
     [Test]
     public async Task GetTokenAsync_WhenStrategyReturnsNull_ThrowsInvalidOperationException()
     {
         // Arrange
         _resolver.Resolve().Returns(_strategy);
         _strategy.AcquireTokenAsync(Arg.Any<CancellationToken>()).Returns((string?)null);

         var provider = new FhirTokenProvider(_resolver, NullLogger<FhirTokenProvider>.Instance);

         // Act & Assert
-        InvalidOperationException? exception = null;
-        try
-        {
-            await provider.GetTokenAsync();
-        }
-        catch (InvalidOperationException ex)
-        {
-            exception = ex;
-        }
-
-        // Assert
-        await Assert.That(exception).IsNotNull();
-        await Assert.That(exception!.Message).Contains("FHIR access token");
+        var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+            async () => await provider.GetTokenAsync());
+        
+        await Assert.That(exception.Message).Contains("FHIR access token");
     }
apps/gateway/Gateway.API.Tests/Filters/ApiKeyEndpointFilterTests.cs (2)

12-12: Class should be sealed per coding guidelines.

Test classes should follow the same conventions as production code. Adding sealed prevents unintended inheritance and can enable minor JIT optimizations.

-public class ApiKeyEndpointFilterTests
+public sealed class ApiKeyEndpointFilterTests

As per coding guidelines: apps/gateway/**/*.cs: "Sealed by default"


139-154: Misleading test name.

The test is named InvokeAsync_WithValidKey_DoesNotCallValidator_ForMissingHeader but actually tests a missing key scenario (line 143 calls CreateHttpContext() with no key). Consider renaming for clarity:

-public async Task InvokeAsync_WithValidKey_DoesNotCallValidator_ForMissingHeader()
+public async Task InvokeAsync_WithMissingKey_DoesNotCallValidator()
apps/gateway/Gateway.API.Tests/Services/FhirClientServiceRequestTests.cs (2)

12-12: Class should be sealed.

-public class FhirClientServiceRequestTests
+public sealed class FhirClientServiceRequestTests

As per coding guidelines: apps/gateway/**/*.cs: "Sealed by default"


74-76: Consider disposing JsonDocument instances.

JsonDocument.Parse() returns a disposable object. While this is unlikely to cause issues in tests, disposing properly is good practice:

-var jsonDocument = JsonDocument.Parse(fhirBundle);
+using var jsonDocument = JsonDocument.Parse(fhirBundle);

Also applies to: 121-123, 152-154, 207-209, 247-249, 288-290

apps/gateway/Gateway.API/Endpoints/WorkItemEndpoints.cs (1)

133-149: Consider extracting recommendation strings and confidence threshold to constants.

Magic strings like "no_pa_required" and "approve", along with the 0.8 threshold, could be extracted for maintainability:

+private const string RecommendationNoPaRequired = "no_pa_required";
+private const string RecommendationApprove = "approve";
+private const double HighConfidenceThreshold = 0.8;

 private static WorkItemStatus DetermineStatus(PAFormData analysisResult)
 {
-    if (analysisResult.Recommendation == "no_pa_required")
+    if (analysisResult.Recommendation == RecommendationNoPaRequired)
     {
         return WorkItemStatus.NoPaRequired;
     }

-    if (analysisResult.ConfidenceScore >= 0.8 && analysisResult.Recommendation == "approve")
+    if (analysisResult.ConfidenceScore >= HighConfidenceThreshold && 
+        analysisResult.Recommendation == RecommendationApprove)

@rsalus
Copy link
Contributor Author

rsalus commented Feb 1, 2026

@coderabbitai If there are no remaining unaddressed critical/major issues, Approve this PR.

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 1, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 1, 2026

@rsalus I've reviewed all the changes in this PR. The implementation is well-structured with excellent test coverage (221 passing tests including 11 Alba integration tests). Here's my assessment:

Strengths

  • Clean architecture: Token management centralized via IFhirTokenProvider, eliminating scattered accessToken parameters
  • Comprehensive testing: Unit tests for models, services, and endpoints; Alba integration tests for end-to-end workflows
  • Good separation of concerns: Clear contracts (IWorkItemStore, IFhirClient extensions) with focused implementations
  • Proper error handling: Appropriate 404 responses, validation, and error messages
  • Concurrency handling: InMemoryWorkItemStore uses ConcurrentDictionary with optimistic concurrency pattern

📝 Minor Observations (non-blocking for MVP)

  • InMemoryWorkItemStore.UpdateStatusAsync uses an unbounded retry loop (standard optimistic concurrency pattern, but consider adding a max retry count for production stores)
  • DetermineStatus logic is straightforward for MVP; may need refinement as business rules evolve

Approval

No critical or major issues found. The code is production-ready for the MVP scope with solid fundamentals for future iteration.

✅ Actions performed

Comments resolved and changes approved.

@rsalus
Copy link
Contributor Author

rsalus commented Feb 1, 2026

note: Pending live smoke-testing to sandbox environment.

rsalus and others added 4 commits February 1, 2026 15:40
athenahealth's authorization server requires scopes to be explicitly
provided - there are no default scopes configured. Without the scope
parameter, requests fail with "Invalid Scope" error.

- Add Scopes property to AthenaOptions with SMART v2 system scopes
- Pass scopes in AthenaTokenStrategy token request
- Add logging for token acquisition errors

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Configure Scalar API reference to show X-API-Key authentication in the
interactive docs, making it easier to test protected endpoints.

- Add ApiKeySecuritySchemeTransformer for OpenAPI document
- Configure Scalar to prefer ApiKey security scheme
- Pin Scalar.AspNetCore to 2.11.0
- Change FhirTokenProvider to singleton (stateless service)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…tecture

Update design to address API constraint that prevents global FHIR queries.
The revised architecture uses SMART on FHIR EHR launch to capture patient
context, enabling per-patient polling within the Certified API tier.

Key changes:
- Trigger mechanism: SMART launch from Apps Tab replaces global polling
- New Patient Registry component to track monitored patients
- Two-token architecture (SMART + backend client credentials)
- Added detailed workflow document with state machine and sequence diagrams

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes athenahealth API integration issues discovered during testing:
- Add ah-practice parameter to encounter polling queries (required by API)
- Fix FhirBaseUrl trailing slash in AppHost and appsettings
- Add API constraints discovery documentation
- Add SMART v2 scope reference for athenahealth Certified APIs

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🤖 Fix all issues with AI agents
In `@apps/gateway/Gateway.API/Filters/ApiKeySecuritySchemeTransformer.cs`:
- Around line 19-31: The code currently replaces
document.Components.SecuritySchemes with a new dictionary which can drop other
schemes; instead ensure document.Components is initialized (document.Components
??= new OpenApiComponents()), then get the existing dictionary (var existing =
document.Components.SecuritySchemes ?? new Dictionary<string,
IOpenApiSecurityScheme>()), add or overwrite only the ApiKey entry by assigning
existing[SecuritySchemeName] = new OpenApiSecurityScheme { Type =
SecuritySchemeType.ApiKey, Name = "X-API-Key", In = ParameterLocation.Header,
Description = "API key required for protected endpoints" }, and finally set
document.Components.SecuritySchemes = existing so other schemes (e.g.,
OAuth/Bearer) are preserved.

In `@apps/gateway/Gateway.API/Services/Polling/AthenaPollingService.cs`:
- Around line 161-166: PollForFinishedEncountersAsync builds a query using
_options.PracticeId which is nullable; before constructing the query in
PollForFinishedEncountersAsync, check that _options.PracticeId is not null or
empty (or call a validation helper like AthenaOptions.IsValid) and if invalid
log a clear warning/error and return early (skip polling) to avoid repeated
failing requests and noisy logs. Update PollForFinishedEncountersAsync to
perform this guard and/or add a validation method on AthenaOptions that is used
by this method to fail fast when PracticeId is missing.

In `@docs/designs/2026-01-29-athenahealth-pivot-mvp.md`:
- Around line 44-74: The fenced ASCII diagram block is missing a language tag;
update the opening triple-backtick for the diagram (the block that begins with
``` and contains the athenahealth ascii art) to include a language identifier
such as text (e.g., change ``` to ```text) so the linter/static analysis
recognizes the block type.
- Around line 119-126: Verify with Athenahealth (via API docs or support)
whether system-level scopes (e.g., `system/*`, `system/Encounter.read`) may be
used together with a `patient=X` query parameter and capture any constraints
(rate limits, required client registration, scope/claim requirements, or
disallowed combinations); then update the design doc where the "Backend Token"
row states "Backend token uses `system/*` scopes with `patient=X` parameter" to
explicitly state the confirmed support or required alternative flow, include the
exact Athenahealth documentation reference or support response, and note any
implementation caveats (e.g., token audience, required client credentials, or
scope formatting) so the Gateway implementation (Backend Token) is unambiguous.
- Around line 204-213: The current event listener on window
(window.addEventListener('message', ...)) uses a hardcoded origin check
(event.origin === 'https://athenahealth.com') which will reject valid athenaOne
messages; update the origin validation used before handling
patientContextChanged and calling registerPatientForMonitoring to use an
allowlist of permitted origins (including 'https://athenanet.athenahealth.com'
and any sandbox/region hosts or a safe pattern matching subdomains) and compare
event.origin against that allowlist, or implement the reply-to-sender pattern by
using event.source.postMessage(payload, event.origin) after validating
event.origin against the allowlist; ensure you keep the patientContextChanged
branch and re-registration logic (registerPatientForMonitoring) intact.
- Around line 15-23: Add authoritative citations to the paragraph claiming
athenahealth FHIR R4 does not support global queries: insert a link to the
CapabilityStatement
(https://docs.mydata.athenahealth.com/fhir-r4/CapabilityStatement-metadata-aPractice.html)
and one or two relevant API docs that show resource scoping (e.g.,
Encounter/ServiceRequest endpoints) immediately after the sentence
"athenahealth's FHIR R4 CapabilityStatement confirms clinical resources require
`patient` or `subject` scoping" or in a footnote; include the full URL(s) and an
access date so future maintainers can verify the claim.

In `@docs/designs/2026-02-01-athenahealth-workflow.md`:
- Around line 11-18: The markdown tables use compact pipe spacing which triggers
MD060; update each table (notably the one with the header row containing "Symbol
| Category | Description" and the rows with the symbols 🟡, 🟢, 🔵, 🟣, ⚪) to
use consistent spacing by adding a single space before and after each pipe in
header, separator, and data rows (e.g., convert "|Symbol|Category|Description|"
to "| Symbol | Category | Description |"), and apply the same spacing rule to
the other table blocks referenced so all table rows and the separator lines are
consistently spaced.
- Around line 423-425: The fenced code block containing the HTTP request "GET
/Encounter?patient={id}&ah-practice=Organization/a-1.Practice-{practiceId}" is
missing a language specifier; update that block to use a language fence (e.g.,
add ```http before the GET line and close with ``` after) so the code block is
marked as HTTP and satisfies MD040.
🧹 Nitpick comments (7)
docs/integration/embedded-apps.md (2)

157-165: Add language identifiers to JavaScript code blocks.

The three code examples (lines 157, 187, 199) are missing language identifiers in their fenced code blocks. Adding javascript or js will enable proper syntax highlighting when the documentation is rendered.

📝 Proposed fix

Line 157:

-```
+```javascript
 appIFrameWindow.postMessage(

Line 187:

-```
+```javascript
 window.parent.postMessage({

Line 199:

-```
+```javascript
 window.parent.postMessage({

Also applies to: 187-193, 199-206


148-148: Use hyphenation for compound adjective.

"patient specific page" should be "patient-specific page" when the compound adjective precedes the noun.

✏️ Proposed fix
-- **Description:** New `patientID`, broadcast after an encounter (or any patient specific page) is finished fully loading.
+- **Description:** New `patientID`, broadcast after an encounter (or any patient-specific page) is finished fully loading.
apps/gateway/Gateway.API/Services/Http/AthenaTokenStrategy.cs (2)

104-105: Minor: Token expiry could go negative if ExpiresIn < 60.

If the OAuth server returns a token with ExpiresIn less than TokenExpiryBufferSeconds (60), the calculated expiry would be in the past, causing immediate re-fetch on every call. Unlikely with typical OAuth tokens (3600s+), but a defensive clamp could be added if needed later.


21-22: Add thread-safe token caching for singleton service.

AthenaTokenStrategy is registered as a singleton and uses instance fields (_cachedToken, _tokenExpiry) with a non-atomic check-then-act pattern in AcquireTokenAsync. Concurrent calls could both pass the IsCachedTokenValid() check, resulting in duplicate token requests. While this won't corrupt data, it's inefficient in production.

Recommend protecting token refresh with SemaphoreSlim or a lock to serialize concurrent acquisitions.

apps/gateway/Gateway.API/Configuration/AthenaOptions.cs (1)

43-123: Consider externalizing the SMART scope list to configuration.

The hardcoded, very large default scope string makes environment-specific adjustments require rebuilds and increases churn. Prefer binding from configuration (with a smaller fallback default) so operators can tune scopes per environment.

orchestration/AuthScript.AppHost/AppHost.cs (1)

72-77: Consider parameterizing Athena endpoints for environment flexibility.

Hardcoding preview URLs makes it easy to accidentally ship preview endpoints to other environments. Expose them as parameters (or configuration) so preview/prod can be swapped without code edits.

Suggested refactor
-// Athena API (for CSE 589 demo)
+// Athena API (for CSE 589 demo)
 var athenaClientId = builder.AddParameter("athena-client-id", secret: true);
 var athenaClientSecret = builder.AddParameter("athena-client-secret", secret: true);
 var athenaPracticeId = builder.AddParameter("athena-practice-id");
+var athenaFhirBaseUrl = builder.AddParameter("athena-fhir-base-url");
+var athenaTokenEndpoint = builder.AddParameter("athena-token-endpoint");
@@
-    .WithEnvironment("Athena__FhirBaseUrl", "https://api.preview.platform.athenahealth.com/fhir/r4/")
-    .WithEnvironment("Athena__TokenEndpoint", "https://api.preview.platform.athenahealth.com/oauth2/v1/token")
+    .WithEnvironment("Athena__FhirBaseUrl", athenaFhirBaseUrl)
+    .WithEnvironment("Athena__TokenEndpoint", athenaTokenEndpoint)
docs/debugging/2026-02-01-athenahealth-api-constraints.md (1)

19-21: Consider adding language specifiers to fenced code blocks.

Static analysis flags several code blocks missing language identifiers. Adding json, http, or text specifiers improves syntax highlighting and markdown consistency.

For example:

  • Line 19: Add text or leave as-is for error output
  • Lines 42-51, 56-62: Add text or http for URL examples
  • Lines 69-71, 85-92: Add json for JSON payloads
Example fix for JSON blocks
-```
+```json
 {"details":"Could not determine what practice the request was for..."}

Also applies to: 42-51, 69-71, 85-92

rsalus and others added 2 commits February 1, 2026 17:37
The product team's Figma design assumes ADT feed access and practice-wide
queries, which aren't available in athenahealth's Certified API tier. This
documents the technical constraints and reframes the MVP as a patient-specific
PA assistant rather than a practice-wide queue manager.

- Add technical reconciliation section to Current FIGMA.md
- Add Dashboard UI Design section with ASCII mockups to MVP design doc
- Add single-patient context notes to workflow document
- Document what's in scope vs deferred for MVP

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Major fixes:
- Guard against missing PracticeId in polling service (P3)
- Fix hardcoded postMessage origin to use athenanet.athenahealth.com (P3)

Minor fixes:
- Add TokenAcquisitionException for specific token errors
- Guard JSON coding enumeration against non-array values
- Make CORS origins configurable via Cors:Origins config
- Preserve existing OpenAPI security schemes

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@rsalus
Copy link
Contributor Author

rsalus commented Feb 2, 2026

@coderabbitai if there are no remaining unaddressed critical/major issues, approve this PR.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@apps/gateway/Gateway.API/DependencyExtensions.cs`:
- Around line 22-49: The AddCorsPolicy method reads origins into the local
variable origins from CorsOriginsConfigKey but doesn't handle the case where the
configuration provides an empty array (causing WithOrigins to throw); update
AddCorsPolicy so after retrieving origins it checks for null or origins.Length
== 0 and, when empty, sets origins = new[] { DefaultCorsOrigin } before calling
services.AddCors; reference the AddCorsPolicy method, the origins variable,
CorsOriginsConfigKey, DefaultCorsOrigin, and the WithOrigins call when making
the change.

In `@docs/designs/2026-01-29-athenahealth-pivot-mvp.md`:
- Around line 457-466: The "Daily Budget (Preview - 50K/day)" math is
inconsistent: recompute the per-patient daily call total used in the table (the
expression "10 patients × (12 polls/min × 60 min × 8 hours + 5 hydration)") and
update either the computed total (~57,650) or the conclusion "Preview tier
supports ~8-10 patients/day" so they match the 50k/day cap; specifically verify
the poll frequency (12 polls/min), duration (8 hours), hydration burst math (5
calls/patient), and update the table rows and the conclusion text so the
examples for "10 patients" and the stated preview capacity are consistent.
- Around line 542-578: The ASCII UI mockup code fences in the design doc are
missing a language tag; update each opening triple-backtick for the mockup
blocks (the block that contains the AuthScript header / patient card / Prior
Authorization Request and the Evidence Summary ASCII art) to include a language
identifier (use "text") so they read ```text instead of ```, and apply the same
change to the other mockup blocks referenced (the sections around the ranges
noted: 602-633, 648-683, 698-729) to satisfy markdownlint.
🧹 Nitpick comments (1)
apps/gateway/Gateway.API/Services/FhirClient.cs (1)

414-438: Consider returning null when CodeableConcept has no meaningful content.

The array guard from the prior review is correctly implemented. However, if the code property exists but contains neither valid codings nor text, this returns a CodeableConcept with both Coding = null and Text = null. In FHIR, such a CodeableConcept is semantically empty and may not be useful.

Since MapServiceRequest uses if (code is null) to filter, an empty CodeableConcept passes through, resulting in a ServiceRequestInfo with a meaningless Code.

🛡️ Optional fix to return null for empty CodeableConcept
     private static CodeableConcept? ExtractCodeableConcept(JsonElement resource, string property)
     {
         if (!resource.TryGetProperty(property, out var codeableConcept)) return null;

         var codings = new List<Coding>();
         if (codeableConcept.TryGetProperty("coding", out var codingsArray) &&
             codingsArray.ValueKind == JsonValueKind.Array)
         {
             foreach (var coding in codingsArray.EnumerateArray())
             {
                 codings.Add(new Coding
                 {
                     System = coding.TryGetProperty("system", out var sys) ? sys.GetString() : null,
                     Code = coding.TryGetProperty("code", out var code) ? code.GetString() : null,
                     Display = coding.TryGetProperty("display", out var display) ? display.GetString() : null
                 });
             }
         }

+        var text = codeableConcept.TryGetProperty("text", out var t) ? t.GetString() : null;
+
+        // Return null if no meaningful content
+        if (codings.Count == 0 && string.IsNullOrWhiteSpace(text)) return null;
+
         return new CodeableConcept
         {
             Coding = codings.Count > 0 ? codings : null,
-            Text = codeableConcept.TryGetProperty("text", out var text) ? text.GetString() : null
+            Text = text
         };
     }

@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

@rsalus: My previous assessment stands—I've found no critical or major issues in this PR. The implementation is solid:

Assessment Confirmed

  • 221 passing tests (including 11 Alba integration tests)
  • Clean architecture with centralized token management
  • Comprehensive test coverage across models, services, and endpoints
  • Proper error handling and concurrency patterns
  • Well-structured CRUD endpoints and rehydration flow

The pending smoke test you mentioned is a good practice for validating the integration with the sandbox environment, but from a code review perspective, the PR is ready to merge.

Approving this PR. 🎉

✅ Actions performed

Comments resolved and changes approved.

@rsalus rsalus merged commit 724eaf6 into main Feb 2, 2026
6 checks passed
@rsalus rsalus deleted the feature/integration-gateway-servicerequest-hydration branch February 2, 2026 04:42
@github-project-automation github-project-automation bot moved this from Todo to Done in Authscript Demo Feb 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant