fix: remove consumer-side Repository client WithCaching to prevent startup crash - #53
Merged
Merged
Conversation
…artup crash Repository 4.2.21 client-side caching policy composition throws ArgumentException: expression must invoke a method declared by IAdminActionsApi at DI build time. portal-sync and portal-repository-func are already crashing in production with this signature. portal-server-events main (PR #52) has the same registration and is subject to the same deterministic failure on restart. Removes only `.WithCaching(c => c.UseLibraryDefaults())` from Program.cs. BaseUrl + Entra ID authentication preserved. Repository server-side caching remains active. Player-context IMemoryCache, LiveStatus writes, moderation, VPN protection, welcome messages, and all event handling behaviour unchanged. Repository 4.2.21 / MX.Api 2.3.76 versions unchanged. Adds Startup/RepositoryApiClientRegistrationTests that build the production DI registration and resolve IRepositoryApiClient, IVersionedAdminActionsApi and every sub-client the processor calls at runtime (.V1 access forces the typed-client graph to materialise) so any regression in the AddRepositoryApiClient composition is caught before it reaches production. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
This hotfix removes consumer-side Repository API client caching configuration from the Processor Azure Functions app to avoid a deterministic startup crash seen in other services on Repository client 4.2.21, while adding a targeted DI-resolution test to ensure typed sub-clients can still be materialized during startup.
Changes:
- Removed
.WithCaching(c => c.UseLibraryDefaults())from the Repository API client DI registration in the Processor App. - Added startup DI resolution tests that force versioned sub-client materialization (e.g.,
AdminActions.V1) to catch future regressions in client composition.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/XtremeIdiots.Portal.Server.Events.Processor.App/Program.cs | Removes consumer-side caching from Repository API client registration to prevent startup crashes. |
| src/XtremeIdiots.Portal.Server.Events.Processor.App.Tests/Startup/RepositoryApiClientRegistrationTests.cs | Adds DI-resolution tests to ensure Repository client + representative sub-clients resolve at startup. |
Suppressed comments (2)
src/XtremeIdiots.Portal.Server.Events.Processor.App.Tests/Startup/RepositoryApiClientRegistrationTests.cs:51
- ServiceProvider is IDisposable; this test creates a new provider without disposing it. Use a using declaration to avoid leaking resources across the test run.
var provider = BuildProductionServiceProvider();
src/XtremeIdiots.Portal.Server.Events.Processor.App.Tests/Startup/RepositoryApiClientRegistrationTests.cs:38
- ServiceProvider is IDisposable; this test creates a new provider without disposing it. Use a using declaration to avoid leaking resources across the test run.
var provider = BuildProductionServiceProvider();
| [Fact] | ||
| public void ProductionRegistration_BuildsAndResolvesRepositoryClient() | ||
| { | ||
| var provider = BuildProductionServiceProvider(); |
frasermolyneux
temporarily deployed
to
Development
August 2, 2026 21:19 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
|
🏗️ Terraform Plan
✅ Validate — Passed ✅ Plan
|
9 tasks
frasermolyneux
added a commit
that referenced
this pull request
Aug 3, 2026
…ing (#54) * feat: roll Repository client to 4.2.22 and re-enable consumer L1 caching Bumps XtremeIdiots.Portal.Repository.Api.Client.V1 from 4.2.21 to 4.2.22 and MX.Api.Client from 2.3.76 to 2.3.77. 4.2.22 consumes MX.Api 2.3.77's reflection-free SharedCacheConfiguration which scopes each cache policy to its matching typed sub-API, fixing the cross-sub-API expression fanning bug that crashed startup with 'The expression must invoke a method declared by ...IAdminActionsApi ...' on 4.2.21 (hotfix removed via PR #53). Re-applies '.WithCaching(c => c.UseLibraryDefaults())' on AddRepositoryApiClient that was removed by PR #53. Adds the new mandatory '.WithCachePartition("portal-server-events")' introduced in MX.Api 2.3.77 to isolate this consumer's cache keys. Cache safety verified: the processor's cached surface (library defaults cover GET-shaped reads on GetConfiguration/GetConfigurations, GetProtectedNames, GetAdminActions and Get/HeadPlayer variants) is disjoint from its write surface (Create*/Update*/Set*/Upsert*/Record*/Ensure*/Claim*/Complete*/Consume*). No read-after-write on the same entity in a single Service Bus message handler: HeadPlayerByGameType 404 -> CreatePlayer -> GetPlayerByGameType is a fresh GET (HEAD 404 is not a cached success payload). Configuration/ProtectedNames/AdminAction reads all happen before their respective writes in the same handler. App-owned IMemoryCache player-context is unaffected and still explicitly invalidated after writes. Updates Startup/RepositoryApiClientRegistrationTests to mirror the production registration exactly (both '.WithCachePartition' and '.WithCaching' present) so the smoke test resolves IRepositoryApiClient plus every typed sub-API .V1 the processor uses under the same DI composition path that failed on 4.2.21. Runs under the default CI test filter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: extract Repository client registration into shared helper PR #54 review feedback: the boot smoke test previously duplicated the production Repository client option chain inline, so Program.cs and the test could drift (especially the cache partition literal and the option order). Introduce internal AddPortalServerEventsRepositoryApiClient extension in the App project as the single source of truth for BaseUrl / EntraId audience / cache partition / L1 caching policy. Program.cs and RepositoryApiClientRegistrationTests both call it, so the test is now a real regression guard: any future change to the production chain is exercised by the DI resolution asserts automatically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Summary
Urgent preventive production hotfix. Removes consumer-side
WithCaching(c => c.UseLibraryDefaults())from the Repository API client registration in the Processor App so it does not hit the same startup crash that took down portal-sync and portal-repository-func.portal-syncandportal-repository-funcare currently crashing at startup withArgumentException: expression must invoke a method declared by IAdminActionsApi...on Repository 4.2.21 client-side caching policy composition.mainhere (PR #52 / commit28ab5f4) has the identical registration and is subject to the same deterministic failure on the next Function App restart.Repository 4.2.21 / MX.Api 2.3.76 versions unchanged. Repository server-side caching remains active. Player-context
IMemoryCache,LiveStatus.V1writes, moderation, VPN protection, welcome messages, and all Service Bus event handling are untouched.Closes #
Type of change
Required reading consulted
AGENTS.md(repo brief).github/copilot-instructions.md(repo orientation).github-copilot/.github/instructions/personal.working-preferences.instructions.md(always-on rules)AGENTS.md(standards.*,patterns.*,platform.*,shared.*)Validation evidence
Build
Tests
Targeted (new startup DI test):
Full unit suite (excluding integration tests, per
AGENTS.md):Format check
Other (lint, terraform plan summary, screenshots)
No Terraform changes in this PR —
terraformdirectory untouched.Risk and rollout
portal-server-eventsProcessor Function App only (dev + prd). Preventive change; the crash has not yet manifested here because the app has not restarted since PR Implement caching rollout for portal-repo 4.2.21 #52 merged, but any restart / scale event / deploy would trigger it.main, prd via the standard release workflow. No new workflow steps.ArgumentException: expression must invoke a method declared by IAdminActionsApiin Application Insights traces during host startup) before promoting to prd.Consumer impact
Reviewer focus areas
RepositoryApiClientRegistrationTestsexercises real DI resolution (calls.V1on each versioned facade to force typed-client materialisation) rather than merely inspecting anEnableCachingflag — yes; that is the exact code path where the 4.2.21 policy-composition crash surfaces.WithCaching/UseLibraryDefaults/CachePolicycall sites remain in the App project — verified by grep.portal-syncandportal-repository-funcis out of scope for this repo and tracked separately.Agent attestation
code-reviewsub-agent; High/Medium findings resolved or justified above in Reviewer focus areasstandards.oidc-and-secrets.instructions.md)