[ADR-170] Implement LayoutProcessingService with SQS polling and orchestration pipeline

[jodavis]

Jira: https://jodasoft.atlassian.net/browse/ADR-170

What changed

• Added AdaptiveRemote.Backend.LayoutProcessingService project with LayoutProcessingOrchestrator background service: polls SQS, fetches raw layout, compiles via stub, validates via stub, stores result or writes back validation failure
• Added ILayoutCompilerClient, ILayoutValidationClient interfaces to AdaptiveRemote.Contracts; stub implementations (StubLayoutCompilerClient, StubLayoutValidationClient, StubNotificationPublisher) injected — real implementations deferred to Tasks 7–9
• Wired RawLayoutService to use real SqsLayoutProcessingTrigger; provisioned SQS queue and DLQ (max receive count 3, 14-day DLQ retention) in LocalStack init scripts and docker-compose.yml; fixed LocalStackFixture to ensure SQS is enabled, and added ConfigureHttpJsonOptions in RawLayoutService for correct JSON deserialization

Test plan

scripts/validate-tests passes — unit tests (success path, validation failure path, retry behavior) and headless E2E tests all green.

🤖 Generated with Claude Code

[github-actions]

Test Results

440 tests  +6   440 ✅ +6   2m 23s ⏱️ -18s
 18 suites +2     0 💤 ±0 
 18 files   +2     0 ❌ ±0 

Results for commit e2d7ae4. ± Comparison against base commit 106f115.

♻️ This comment has been updated with latest results.

[jodavis]

src/AdaptiveRemote.Backend.LayoutProcessingService/Logging/MessageLogger.cs:38 — Typo in method name and log message: SqsPollingStoped should be SqsPollingStop**ped**. The misspelling will be locked into the public API once this ships. Fix both the method name and the Message string.

[jodavis]

src/AdaptiveRemote.Backend.LayoutProcessingService/Logging/MessageLogger.cs:65 — SqsMessageArrivedInDlq is fired when ApproximateReceiveCount > 1, which means the message has been retried — it has not necessarily arrived in the DLQ yet. The name and message text are factually wrong. This fires at receive count 2 while the message is still on the main queue; it only enters the DLQ after receive count 3. Rename to something like SqsMessageRetry and update the message text to reflect "message is being retried; rawLayoutId={RawLayoutId} approximateReceiveCount={ApproximateReceiveCount}".

[jodavis]

src/AdaptiveRemote.Backend.LayoutProcessingService/Program.cs:106 — ILayoutProcessingTrigger / SqsLayoutProcessingTrigger is registered in LayoutProcessingService's DI container but is never consumed by anything in this service. LayoutProcessingOrchestrator polls the queue — it doesn't send to it. The trigger belongs exclusively in RawLayoutService (which is already wired correctly). This registration is dead code and should be removed, along with the corresponding SqsLayoutProcessingTrigger.cs file in the LayoutProcessingService project.

[jodavis]

src/AdaptiveRemote.Backend.LayoutProcessingService/Program.cs:77 — HttpRawLayoutRepository and HttpCompiledLayoutRepository are typed HttpClient clients (registered via AddHttpClient<T>, which uses a Transient lifetime), but then immediately re-registered as Singleton wrappers via sp.GetRequiredService<T>(). This is the classic captive dependency / socket exhaustion problem: a single HttpMessageHandler is captured forever, bypassing the IHttpClientFactory rotation that prevents DNS staleness and socket exhaustion. The standard fix is to register the interface directly through AddHttpClient: builder.Services.AddHttpClient<IRawLayoutRepository, HttpRawLayoutRepository>(...) and drop the separate Singleton re-registration. This requires removing the dual-interface pattern (one class implementing both IRawLayoutRepository and IRawLayoutStatusWriter) or splitting it into two classes.

[jodavis]

test/AdaptiveRemote.Backend.LayoutProcessingService.Tests/Services/LayoutProcessingOrchestratorTests.cs — Missing test for the unrecognized-message-body path. ProcessMessageAsync has specific logic to handle a message whose body is not a valid GUID: it logs SqsUnrecognizedMessageError and deletes the message. This is a distinct behavior branch (not covered by any existing test) and should have a unit test verifying that (a) the message is deleted, and (b) no pipeline steps are invoked.

[jodavis]

test/AdaptiveRemote.Backend.ApiTests/Support/ServiceFixture.cs:148 — The E2E health test for LayoutProcessingService starts the service without configuring RawLayoutService__BaseUrl or CompiledLayoutService__BaseUrl. The service will start and the health endpoint will return 200, but LayoutProcessingOrchestrator is running in the background and will immediately start polling SQS and attempting HTTP calls to the unconfigured upstream services. This could produce spurious error log lines that cause the the service logs contain no warnings or errors assertion to fail intermittently. Either set dummy BaseUrl values in the test fixture for LayoutProcessingService, or verify the test is actually passing consistently in CI.

[jodavis]

src/AdaptiveRemote.Backend.LayoutProcessingService/Logging/MessageLogger.cs:65 — SqsMessageArrivedInDlq is also marked LogLevel.Error. At receive count 2 the message is being retried normally — that's an informational or warning event, not an error. Error implies something has gone wrong in the service itself. Change to LogLevel.Warning.

[jodavis]

src/AdaptiveRemote.Backend.LayoutProcessingService/Logging/MessageLogger.cs:13 — Event IDs 1300–1305 collide with AdaptiveRemote.App/Logging/MessageLogger.cs which already occupies 1301–1305 (ConversationController state/speech messages). The CLAUDE.md convention treats event IDs as a project-wide namespace. The LayoutProcessingService range should start after the last claimed App ID; the next free block above 1305 that doesn't overlap existing backend ranges (1100–1109 CompiledLayoutService, 1200–1220 RawLayoutService) would be 1400–1499 or similar. Pick a block and document it in the CLAUDE.md table (see next comment).

[jodavis]

CLAUDE.md:37 — The event ID range table stops at 1000–1099 BroadlinkCommandService but three backend services now use IDs above 1099. The table needs rows for at least: 1100–1199 CompiledLayoutService, 1200–1299 RawLayoutService, and whatever range is assigned to LayoutProcessingService (see previous comment). Without these entries, the next developer cannot determine which range is free.

[jodavis]

src/AdaptiveRemote.Backend.LayoutProcessingService/Configuration/SqsSettings.cs:23 — DlqUrl is declared and configured in appsettings.Development.json but is never read anywhere in the service. Either remove it (the DLQ is a queue-side configuration, not a consumer-side concern) or add a comment explaining what it will be used for. As-is it is dead configuration.

[jodavis]

src/AdaptiveRemote.Backend.LayoutProcessingService/Logging/MessageLogger.cs:83 — SqsUnrecognizedMessageError is LogLevel.Error, but the orchestrator handles the case correctly — it deletes the poison message and moves on. That is not an error condition from the service's perspective; it is at most a Warning (the message is unexpected but the system is behaving correctly). Log level should be LogLevel.Warning.

[jodavis]

test/AdaptiveRemote.Backend.LayoutProcessingService.Tests/Services/LayoutProcessingOrchestratorTests.cs:36 — CLAUDE.md requires [TestCleanup] for mock verification. This test class has [TestInitialize] but no [TestCleanup]. All mock Verify calls are currently inline at the end of each test method, which is fine, but CLAUDE.md's pattern calls for a [TestCleanup] method — at minimum one that calls _mockSqs.VerifyNoOtherCalls() etc. to catch unexpected interactions. Without it, each test must exhaustively list every Times.Never assertion, and omissions are silent.

[jodavis]

src/AdaptiveRemote.Backend.LayoutProcessingService/Services/HttpCompiledLayoutRepository.cs:21 — GetActiveForUserAsync(string userId, ...) accepts userId but discards it: the HTTP call goes to /layouts/compiled/active with no user-scoping. The same applies to ListByUserAsync (line 38) calling /layouts/compiled. Currently harmless because the stubs in CompiledLayoutService ignore userId too, but these implementations will silently return wrong-user data once the real DynamoDB backend lands. At minimum add a TODO comment; ideally pass userId as a query parameter or path segment consistent with the upstream API contract.

[jodavis-claude]

src/AdaptiveRemote.Backend.LayoutProcessingService/Logging/MessageLogger.cs:70 — RawLayoutNotFound is logged at Error level, but a missing raw layout is not an error in the service itself — it's a stale or dangling SQS message. The message is deleted immediately without any retry. Treating this as Warning would be more accurate and prevent false alerts in production monitoring.

[jodavis-claude]

test/AdaptiveRemote.Backend.LayoutProcessingService.Tests/Services/LayoutProcessingOrchestratorTests.cs:138 — Log assertions use _mockLogger.Messages.Should().Contain(...) with raw string matching instead of the project's MockLogger.VerifyMessages pattern. CLAUDE.md says: "In tests, verify log output using MockLogger.VerifyMessages(log => { log.MethodName(...); })". The VerifyMessages overload that accepts an Action<MessageLogger> is app-specific (it wraps the internal MessageLogger), but the base VerifyMessages(params string[]) is available to all test projects and enforces ordered, exact prefix matching. The current approach only checks containment and doesn't enforce ordering, which is weaker. Use _mockLogger.VerifyMessages(...) or at minimum document why the ordered variant is inappropriate for this background-service test.

[jodavis-claude]

src/AdaptiveRemote.Backend.RawLayoutService/Repositories/DynamoDbRawLayoutRepository.cs:27 — GetAsync uses a full-table ScanAsync to find a layout by ID because the DynamoDB table has UserId as the partition key and Id as the sort key, making cross-user lookups require either a scan or a GSI. The comment acknowledges this ("In production, this would require a GSI"), but the same pattern is repeated in DeleteAsync (line 107) and UpdateValidationResultAsync (line 128), both of which first call GetAsync — resulting in a scan inside a scan (two scans per write). For UpdateValidationResultAsync, which is on the hot path called by LayoutProcessingOrchestrator for every validation failure, this is worth noting explicitly with a TODO. The existing MVP comment only covers GetAsync.

[jodavis-claude]

src/AdaptiveRemote.Backend.LayoutProcessingService/Services/HttpCompiledLayoutRepository.cs:22 — GetActiveForUserAsync and ListByUserAsync both ignore the userId parameter and call /layouts/compiled/active / /layouts/compiled without it. Each has a TODO comment noting that the stub currently ignores userId, but this is a silent data correctness hazard: if the real DynamoDB backend is wired up on the CompiledLayoutService side before these TODO comments are addressed, the LayoutProcessingOrchestrator will silently read back the wrong user's compiled layout. The TODO should reference the specific ADR / task that will fix it so it cannot be forgotten.

[jodavis-claude]

src/_spec_LayoutCustomizationService.md:1 — The spec file is still present after implementation. CLAUDE.md says: "Once implementation is complete, replace the spec with a _doc_*.md file (drop implementation detail; link to source instead)". The three new backend services (LayoutProcessingService, RawLayoutService, CompiledLayoutService) also have no _doc_*.md files of their own. The quality gate requires affected _doc_*.md files to be updated — this gate is not met.

[jodavis-claude]

src/AdaptiveRemote.Backend.LayoutProcessingService/Program.cs:71 — The service account token registration is duplicated: the entire AddHttpClient block for IRawLayoutRepository and IRawLayoutStatusWriter is copy-pasted twice (with handler / without handler) rather than extracting the base URL configuration into a shared action. The two branches differ only in whether AddHttpMessageHandler is chained. Consider registering ServiceAccountTokenHandler conditionally and always calling the same AddHttpClient block, or extracting a helper method, to avoid the duplication.

[jodavis-claude]

src/AdaptiveRemote.Backend.RawLayoutService/Endpoints/LayoutEndpoints.cs:39 — The /layouts/raw/{id}/validation-result PATCH endpoint has .RequireAuthorization(), but it does not enforce that the caller is the LayoutProcessingService (a machine identity) rather than an end-user. A regular authenticated user JWT satisfies RequireAuthorization() as written, so any user could POST arbitrary validation results to their own (or, given the scan-by-ID implementation, potentially any) raw layout. This should either use a separate authorization policy that checks for a service-account claim, or the note about this limitation should appear in the docs.

[jodavis-claude]

src/AdaptiveRemote.Backend.LayoutProcessingService/Services/LayoutProcessingOrchestrator.cs:111 — After a successful processing run (compile, validate, store, notify), the SQS message is deleted after logging SqsMessageProcessedSuccessfully (line 202) rather than before. If DeleteMessageAsync fails, the success log has already been emitted, making it misleading — the message will be retried even though the pipeline completed. Consider deleting the message first and only logging success once the delete is confirmed, or rename/reposition the log to "pipeline completed; deleting message" to make the ordering intent clear.

[jodavis-claude]

src/AdaptiveRemote.Backend.RawLayoutService/Repositories/DynamoDbRawLayoutRepository.cs:65 — ListByUserAsync filters on UserId server-side via KeyConditionExpression, then filters again client-side with .Where(l => l.UserId == userId) (line 69). The DynamoDB query already scopes to the partition key, so the in-memory .Where is redundant. Remove it.

[jodavis-claude]

DynamoDbRawLayoutRepository.cs:27 and :65 — Pre-existing issue not introduced by this PR; the scan pattern and the redundant Where clause both predate ADR-170. Fixing DynamoDB query patterns in RawLayoutService is out of scope for this task, which focuses only on LayoutProcessingService. These should be addressed in a dedicated RawLayoutService hardening task.

[jodavis-claude]

_spec_LayoutCustomizationService.md / doc.md — The spec covers the entire LayoutCustomizationService feature, of which ADR-170 is one task among several. The CLAUDE.md convention is to replace a spec with a doc.md once implementation is complete. The implementation is not complete: compiler, validation, and notification clients are still stub-level, and DynamoDB backends are not yet wired up. Converting the spec to a doc.md before all tasks are done would lose the design intent that is still being actively used as a reference. Individual doc.md files will be created when each backend service is fully implemented in later tasks.

[jodavis-claude]

LayoutEndpoints.cs:39 — The RequireAuthorization() on the PATCH /layouts/raw/{id}/validation-result endpoint intentionally does not enforce a service-account-specific policy yet. Cognito M2M token support (the mechanism that would allow a distinct machine-identity policy) is deferred to a later task. The pattern here is the same as other service-account-aware endpoints in the backend — a stub-level auth guard that will be tightened when M2M token validation is implemented. This limitation is documented in the endpoint comment to prevent it from being overlooked.