Persistence and messaging data-integrity plan (October 2026)¶
This plan covers the persistence and messaging defects from the architecture review (architecture-review-2026-10.md: V5, V13, Appendix B B6–B8, Appendix C item 3, Appendix D B4–B8) and issues #3984 and #3985. It also covers the durability of domain events once #3973 (await the dispatch, owned by the backend bug plan) has landed. It is planning only; nothing starts until Chris approves and answers the decisions in §10.
Every item was re-checked against main@85e6facf7 (3 Oct 2026) by reading the code. Counts come
from grep over production sources (tests, bin/ and obj/ excluded), so they are approximate.
1. Scope¶
Persistence¶
| ID | Problem | Impact | Evidence (main@85e6facf7) | Status | Issue |
|---|---|---|---|---|---|
| P1 | SaveAsync/Save replace on {_id, Audit.Version} with IsUpsert = true; bulk models do the same |
A document hard-deleted after it was loaded is silently re-inserted | MongoExtensions.cs:262-299 (:282,:289), :328-354; MongoUnitOfWorkBase.cs:498-516 (PrepareSaveMany), :631-633 (batched) |
VERIFIED. Counts CORRECTED: about 117 production .Save(/.SaveAsync( calls (not 106) against 25 TrySaveExistingAsync (not 38). The live hard-delete paths are narrower than the review implied: public project and search deletion answer DeletionLifecycleUnavailable (ProjectController.cs:419, SearchController.cs:122,134). What remains: guarded account deletion (AccountController.cs:378), import rollback (ProjectManagementService.cs:592,603 → :1542) and the seeder. The exposure grows when the durable deletion kernel (#2934) enables deletion |
#3985 |
| P2 | OnSaving bumps Audit.Version in memory before the write and never restores it |
A failed or missed save leaves the instance at N+1. A retry on that instance filters on N+1 and can overwrite another writer's N+1 | MongoExtensions.cs:270-271,313-314; Audit.cs:28-34 |
VERIFIED, partly mitigated: the unit of work now evicts the cache and the isolated scope on every attempt (MongoUnitOfWorkBase.cs:301-308, :434-440, :664-669). Any holder of the reference, including the caller, keeps N+1 |
#3985 |
| P3 | Direct Study writes don't bump Audit.Version |
A concurrent whole-document save silently undoes them: annotations of a deleted question come back, or CustomId/PdfRelativePath/inclusion elements are lost |
StudyRepository.cs:1306-1319 ($pull), :1321-1350 (ApplySimpleUpdates), :1618-1650 (three inclusion passes) |
VERIFIED. Line numbers CORRECTED: the inclusion passes moved from :1626-1646 to :1618-1650. Other members do bump (:494,:1980,:2013,:2821). InvestigatorRepository has 1 direct write with no bump (not yet read) |
#3985 |
| P4 | The shared, mutable 2-second RepositoryCache hands one instance to every caller |
Unsaved mutations are visible to, and publishable by, concurrent requests | RepositoryCache.cs:43; .claude/rules/repository-cache.md |
VERIFIED. Count CORRECTED: 4 explicit BeginIsolatedReads sites (ProjectManagementService.cs:1384, StageWorkloadSharesController.cs:65, ProjectController.cs:374,885) plus 13 calls to the (Await)UpdateAndSaveAsync helpers, which open the scope themselves. There are about 141 cached Projects/Studies/Investigators/SystematicSearches.Get* calls in controllers, consumers and Core services |
— |
| P5 | Mutate the cached aggregate, then return 409 without saving | Partial changes and their pending domain events stay on the cached instance, and the next request to save that Project within 2 s publishes and dispatches them | ProjectController.cs:515-532 (ApproveJoinRequests: mutate in loop, Conflict at :526) |
VERIFIED for ApproveJoinRequests. DeclineJoinRequests :539, ResendInvitationsBulk :647, RevokeInvitationsBulk :674 and UpdateInvitationGroupsBulk :695 were located but not each re-read (NOT VERIFIED individually) |
new |
| P6 | ResetCache swaps the MemoryCache and the lock map |
A load that started before an invalidation writes stale data into the new cache for 2 s; a reader on the disposed old cache throws ObjectDisposedException, which gets wrapped |
RepositoryCache.cs:120-166 (_cache.Set reads the field after the await), :330-337 |
VERIFIED (mechanism); not reproduced | new |
| P7 | Every save flushes the whole type's cache | Lower hit rate. Each flush widens the P6 window | MongoRepositoryBase.cs:479-482; 120 production InvalidateCache() calls |
VERIFIED | new |
| P8 | Batched and multi saves drop domain events | Cancellation after committed batches, or an exception, skips DispatchEvents, and GetDomainEvents has already cleared them |
MongoUnitOfWorkBase.cs:624-627 (return), :671; SaveMany :414-443, SaveManyAsync :455-494; Entity.cs:63-74 |
VERIFIED | new |
| P9 | The "unit of work" is a process singleton registered by naming convention | No per-request identity map or transaction boundary | MongoLamarRegistry.cs:82-102 (AddSingleton) |
VERIFIED | — (structural) |
Messaging and domain events¶
| ID | Problem | Impact | Evidence | Status | Issue |
|---|---|---|---|---|---|
| M1 | No bus-wide retry or redelivery | A transient Mongo blip or a lost optimistic race sends the message straight to _error |
MassTransitHelpers.cs:30-60 (ConfigureEndpoints, no AddConfigureEndpointsCallback) |
VERIFIED. About 12 of about 35 consumers configure retry: 4 session consumers (ReviewSessionConsumerRetry.cs), 4 bulk-PDF consumers, 3 job consumers (SetRetry) and the saga (SearchImportJobStateMachine.cs:298-302) |
#3984 |
| M2 | Saga CreatePartitioner(1); the Parsing state has no timeout |
Every search import in the process is serialised through one partition. A lost parse result leaves the job in Parsing forever |
SearchImportJobStateMachine.cs:308-318, :154-166 (the only schedule is UploadTimeout, :41) |
VERIFIED | #3984 |
| M3 | Consumers that aren't idempotent | Redelivery (once M1 adds it) could double-apply effects | Read so far: UpdateStudyScreeningStatsConsumer (a redelivery after completion fails its precondition at :65-68 and lands in M5). Import terminal-failure claims are idempotent per .claude/rules/materialized-stats.md |
PARTLY VERIFIED; the other consumers are NOT VERIFIED (PR-9 produces the inventory) | #3984 |
| M4 | The inclusion recalculation can latch forever | The project stays CalculatingInclusionInfo, and UpdateAgreementMode answers UpdateAlreadyPending forever |
No retry in the definition (UpdateStudyScreeningStatsConsumer.cs:150-160); the precondition reads the cached Project (:65); the comments at :83-85,:96-101,:133-136 assume "the redelivery resumes", but no redelivery is configured |
VERIFIED | #3984 |
| M5 | Recording a project error always throws | The real failure is masked and the error save fails | UpdateStudyScreeningStatsConsumer.cs:139-146 adds an anonymous object to List<Object> Errors (Project.cs:122). Driver 3.10's default ObjectSerializer rejects types outside its allow-list |
VERIFIED (code); serializer behaviour not re-run. PR-2's first test is the repro | #3984 |
| M6 | Message contracts embed domain types; no round-trip tests | Silent field loss on the wire, as already seen with the agreement threshold (V7) | 8 of 27 SyRF.ProjectManagement.Messages files reference ProjectAggregate/Core; the .csproj references ProjectManagement.Core (:9). No STJ round-trip test for PM contracts found |
VERIFIED | #3984 |
| E1 | Domain events are neither awaited nor durable | Event handler failures are lost; a crash between the save and the dispatch loses the event | MongoUnitOfWorkBase.cs:731-741; EventManager.cs:21-31. 14 IHandles<> handlers: 10 mail or notification, 2 bus sends (ProjectAgreementThresholdUpdatedHandler, LivingSearchHandler), 1 save (UserHasRegisteredHandler) |
VERIFIED. Awaiting is #3973 (backend bug plan); durability is this plan | #3984, #3973 |
Precedents and constraints confirmed in code:
- In-house outboxes already exist: ActivityClaimRevocationOutbox (lease, attempts, TTL), ProjectStatisticsNotificationOutbox with its ProjectStatisticsOutboxDispatcher, Identity's RecoveryEmailOutbox, and BulkPdfHoldRetirementOutboxIdentity.
- MongoDbReplicaSetTestFixture (src/libs/testing/SyRF.Testing.Common/Fixtures/) and the replica-set E2E Mongo (e2e/docker-compose.yml:35, --replSet rs0) are already available.
- FEAT-024 relies on today's failure shape. StudyUpsertConflictRetry.IsLostUpsertRace (:123-128) and MongoConcurrencyConflict.IsConcurrencyConflict treat E11000 as "lost race".
- ReservationChangeConflictException is the precedent for a typed replacement of that E11000.
- The fold command-budget tests pin the exact command count of each save.
- CORRECTED: the comment in MongoConcurrencyConflict.cs blames MassTransit.MongoDb for the MongoDB.Driver.Core 2.x type collision. MassTransit.MongoDb 8.4.0 actually depends on MongoDB.Driver ≥ 3.2.1 (nuspec). PM's local project.assets.json shows Driver.Core 2.28 arriving through MongoDB.Analyzer 1.5.0. That file may be stale, so PR-10's spike re-checks it.
2. MVP boundary, out of scope, flag decision¶
MVP (releases R0–R2): no silent loss of writes or events on the paths we know about. - A deleted aggregate is never resurrected. - A failed save never leaves a version that can overwrite another writer's. - Direct writes participate in optimistic concurrency. - A transient failure or a lost race is retried and then redelivered rather than dropped. - The inclusion recalculation and search imports cannot hang forever. - Every contract survives the bus serializer. - A domain event raised by a committed save is delivered at least once.
Hardening (R3): cache correctness (P4–P7) and the cache-leak detector.
Out of scope, and where it is tracked:
| Item | Where |
|---|---|
| Awaiting domain events and surfacing handler failures | #3973, backend bug plan (prerequisite for PR-10) |
ValueObject equality and AgreementMeasure serialization (V6/V7) |
#3980 (PR-3 records them as expected failures) |
Scoped identity map replacing RepositoryCache and the singleton unit of work (P9) |
Synthesis Phase 3 "Persistence"; follows the DI decision (Lamar exit). New issue on approval |
PM.Contracts split, one writer per collection, job state out of Project |
Synthesis Phase 3 "Module boundaries" |
| MassTransit after v8 | #3986 (ADR being written in parallel; PR-10 is designed to survive any outcome) |
| Import progress conflict (Appendix D B7) | CORRECTED, so out of this plan: progress callbacks are now drained and counted (ProjectManagementService.cs:614-621), not run under a blocking wait |
| Identity-service persistence | Identity uses its own stores and outbox; owned by the authentication migration session |
| Durable deletion lifecycle | #2934 (must not enable deletion before PR-5 is on; see §8) |
Flag decision. Backend, hot-path, changed observable semantics, so the default is flagged. These are startup configuration switches, set per environment through cluster-gitops for both API and PM, read once at start-up, defaulting to the legacy behaviour:
| Switch (PROPOSAL names) | PRs | Default | Why |
|---|---|---|---|
Persistence:Strict:NonUpsertSaves |
PR-5 | off | Changes the failure type and outcome of every save of an existing aggregate |
Persistence:Strict:DirectWriteVersionBump |
PR-6 | off | Raises the conflict rate for in-flight saves during project-wide passes |
Messaging:GlobalRetry:Enabled |
PR-7 | off | Changes delivery semantics for about 20 endpoints |
DomainEvents:Outbox:Enabled |
PR-10 | off | New write path (a transaction) on event-raising saves |
Persistence:Cache:PerKeyEviction |
PR-11 | off | Changes cache behaviour on the hottest read path |
Persistence:Cache:LeakAudit |
PR-12 | off, staging only | Costs one BSON serialisation per put and per expiry |
Not flagged (with the reason): - PR-1: telemetry and tests only; zero extra commands. - PR-2: restores intended behaviour on a path that always throws today. - PR-3: tests only. - PR-4: failure path only; it restores the invariant the cache eviction already assumes. - PR-8: parsing timeout and partitions; the saga has no feature surface. The timeout value is configuration. - PR-9: tests, plus fixes the inventory finds, each argued in its own PR.
Production rollout of any switch is a separate Chris-approved step.
3. Common acceptance criteria (every PR)¶
| # | Criterion | Verification |
|---|---|---|
| C1 | The regression test is written first, fails on main (or characterises the defect), then passes |
Red and green runs quoted in the PR body |
| C2 | Focused tests only, run niced on this host: nice dotnet test <one project> --filter <class>; never the full solution |
Commands and output in the PR body; CI Test Summary green |
| C3 | FEAT-024 invariants untouched: the fold command-budget tests (FoldSaveCommandBudgetTests, FoldSaveEligibilityCommandBudgetTests, ProjectStatisticsFoldCommandBudgetTests) pass unchanged; no statistics write moves |
Test output; the FEAT-024 session reviews any PR that touches StudyRepository.cs or the classifiers |
| C4 | ADR-020 lock guards untouched: StudyWriteLockArchitectureTests and StudyBulkUpdateLockHonouringTests pass |
Test output |
| C5 | With every switch off, behaviour is byte-identical (same commands, same exceptions) | A Testcontainers test runs each changed path with the switch off and asserts the command list and the exception type |
| C6 | Docs ride in the PR: .claude/rules/repository-cache.md, docs/architecture/mongodb-reference.md, and a new ADR for PR-5 and PR-10; the PR body states the flag decision |
PR review |
| C7 | Reviews settled on the head (pr-review-settled.sh exit 0), CI green, and the bot summary verdict read |
Settled-gate output |
| C8 | Staging rollout: the switch is turned on for API and PM through cluster-gitops, the named metric is watched for 7 days (PROPOSAL), and the result is recorded on the issue. No production change | Staging rollout check |
4. Releases and pull requests¶
Numeric values marked PROPOSAL need Chris's confirmation.
R0: prove it and stop the cheap bleeding (three PRs, in parallel)¶
PR-1: concurrency harness and resurrection detector (P1, P2, P3, P5, P8; effort M)
Files:
- new src/libs/mongo/SyRF.Mongo.Common.Tests/Concurrency/* on MongoDbReplicaSetTestFixture;
- MongoExtensions.cs (SaveAsync, Save): detector only;
- MongoUnitOfWorkBase.cs (bulk paths): detector only;
- a new PersistenceMetrics meter, named in both hosts' AddMeter.
Approach:
1. Characterisation tests, one per defect, each asserting today's (wrong) outcome under [Trait("Defect", …)]. Later PRs flip them.
2. Detector: when ReplaceOneResult.UpsertedId is non-null and the version before the save was greater than 0, the save has just re-inserted a deleted document. Log a warning with type and id, and increment syrf.persistence.resurrections{aggregate}.
3. The same check for bulk results (BulkWriteResult.Upserts).
| # | Acceptance criterion | Verification |
|---|---|---|
| 1.1 | A Study is loaded, deleted, then saved → the test observes the re-inserted document (characterises P1) | Testcontainers |
| 1.2 | That save → the resurrection counter goes up by 1 and one warning names the type and id | Testcontainers plus a MeterListener |
| 1.3 | An ordinary existing save or a new-aggregate save → no counter increment and no extra command | Testcontainers command-listener assertion |
| 1.4 | Characterisation tests exist for P2 (retry at N+1 overwrites), P3 ($pull undone by a whole save), P5 (409 leaves mutation and events on the cached instance) and P8 (cancel after batch 1 → batch-1 events never dispatched) |
Testcontainers |
| 1.5 | Staging, 7 days after deploy → the resurrection count is recorded on #3985 (expected 0; non-zero raises PR-5's priority) | Staging rollout check |
PR-2: inclusion recalculation cannot latch; project errors serialise (M4, M5; effort M)
Files:
- UpdateStudyScreeningStatsConsumer.cs;
- Project.cs (Errors becomes List<ProjectError>, a record with a tolerant class map that reads legacy BSON elements);
- ProjectRepository.cs (class map).
Approach:
1. Read the precondition uncached (GetUncachedAsync).
2. A redelivery that finds the job complete for the same threshold acknowledges as a no-op.
3. Keep the FEAT-024 fence semantics exactly: the token is derived as today, and the fence is never released on failure.
4. Add consumer retry and scheduled redelivery for MongoConcurrencyConflict.IsRetryableSaveFailure only.
5. Write the error record through an uncached load and TrySaveExistingAsync.
Coordination: the FEAT-024 session reviews it (fence code); #3941 touches Project.cs.
| # | Acceptance criterion | Verification |
|---|---|---|
| 2.1 | The consumer fails after the fence → the error is persisted on the Project and the original exception is rethrown (no BsonSerializationException) |
Testcontainers (red on main) |
| 2.2 | The precondition was true in Mongo but the cache held a stale false → the job runs |
Testcontainers |
| 2.3 | The same command is delivered twice after success → the second delivery writes nothing and does not fault | MassTransit test harness plus Mongo |
| 2.4 | A transient save failure on completion → redelivered, and the project leaves CalculatingInclusionInfo |
Harness with fault injection |
| 2.5 | Existing pmProject documents with legacy Errors entries → still deserialise |
Unit test with captured BSON shapes; a dry-run count on syrftest (§7) sizes them |
| 2.6 | The FEAT-024 inclusion-fence tests pass unchanged | Test output |
PR-3: contract round-trip tests (M6; effort S)
Files: new SyRF.ProjectManagement.Messages.Tests (or a folder in an existing test project), covering every ICommand/IEvent in PM, API and Quartz message assemblies.
Approach:
- Reflect over the contract assemblies.
- Build each message with non-default values for every property, recursively.
- Serialize with MassTransit's SystemTextJsonMessageSerializer options, then assert deep equality.
- Known failures (the AgreementMeasure protected setters) are listed as expected failures linked to #3980, so the test fails when #3980 lands until the entry is removed.
| # | Acceptance criterion | Verification |
|---|---|---|
| 3.1 | Every contract type → round-trips with all properties equal, except the listed known failures | Contract test |
| 3.2 | A new contract with a non-public setter → the test fails and names the property | Contract test (one red case shown in the PR) |
| 3.3 | The 8 contracts that reference domain types → listed in the test output as "domain-coupled", feeding the Phase 3 PM.Contracts issue |
Test report |
R1: persistence correctness (three PRs; PR-4 → PR-5 sequential, PR-6 in parallel)¶
PR-4: restore the audit on a failed or missed write (P2; effort S)
Files: Audit.cs (an internal Snapshot/Restore), MongoExtensions.cs (SaveAsync, Save, TrySaveExistingAsync), MongoUnitOfWorkBase.cs (bulk paths).
Approach:
- Snapshot the audit before OnSaving.
- Restore it when the call throws, or when MatchedCount == 0 without an upsert.
- An unknown commit result also restores. A retry with filter N then either matches (the write never happened) or conflicts (it did), and both are safe.
| # | Acceptance criterion | Verification |
|---|---|---|
| 4.1 | A save loses a race → the instance's Version, LastModified and LastModifiedBy equal their pre-save values |
Testcontainers |
| 4.2 | The P2 characterisation (retry at N+1 overwrites another host's N+1) → now conflicts instead | Testcontainers (flipped from 1.4) |
| 4.3 | Successful saves → unchanged commands and version | Command-listener assertion |
PR-5: non-upsert saves for existing aggregates (P1; effort L; flag Persistence:Strict:NonUpsertSaves)
Files:
- MongoExtensions.cs, MongoUnitOfWorkBase.cs;
- new AggregateConcurrencyException in SharedKernel. It carries no driver type, because the endpoint assemblies cannot name driver types;
- MongoConcurrencyConflict.cs, StudyUpsertConflictRetry.cs;
- the other 13 files that catch MongoWriteException/MongoBulkWriteException in the API, PM and Core: each is audited, and only those that classify saves of SaveAsync are changed.
Approach:
1. With the switch on, Version > 0 uses ReplaceOne(IsUpsert = false), and bulk models likewise.
2. A miss runs the existing AggregateWriteGuards.ThrowIfGuardedAsync (the guard read the E11000 path already does). It then throws AggregateConcurrencyException, the same command count as today's E11000 path.
3. Version == 0 keeps today's upsert (D2).
4. Both classifiers recognise the new exception as a lost race, mirroring the ReservationChangeConflictException precedent.
5. A reload that returns null is the caller's existing not-found path.
6. ADR written in the PR.
| # | Acceptance criterion | Verification |
|---|---|---|
| 5.1 | Switch on; a loaded aggregate is deleted, then saved → no document is inserted and AggregateConcurrencyException is thrown |
Testcontainers (flips 1.1) |
| 5.2 | Switch on; a stale-version save → AggregateConcurrencyException, and MongoConcurrencyConflict.IsRetryableSaveFailure and StudyUpsertConflictRetry.IsLostUpsertRace return true |
Unit plus Testcontainers |
| 5.3 | Switch on; a locked study → still StudyLockedByBulkUpdateException, never the concurrency exception |
Testcontainers (ADR-020 honouring tests) |
| 5.4 | Switch on; the fold, screening and annotation saves → command lists identical to the pinned budgets | FEAT-024 budget tests run with the switch on and off |
| 5.5 | Switch on; a new aggregate (Version == 0) → inserted as today |
Testcontainers |
| 5.6 | Switch on; a hub screening save that loses a race with the fold flag on → reloaded and retried within today's budget | Existing StudyUpsertConflictRetry tests, parameterised on the switch |
| 5.7 | Switch on in staging for 7 days → resurrections stay 0, and the concurrency-exception rate is within 2× of the previous E11000 rate (PROPOSAL) | Staging rollout check |
| 5.8 | Reviewer screening and annotation journey with the switch on → passes | E2E (hermetic e2e/ stack, targeted specs) |
PR-6: direct writes bump the version (P3; effort M; flag Persistence:Strict:DirectWriteVersionBump)
Files:
- StudyRepository.cs (:1306, :1321, :1618-1650);
- InvestigatorRepository.cs and ProjectRepository.cs direct writes, after reading each;
- new DirectWriteVersionArchitectureTests, modelled on StudyWriteLockArchitectureTests, with an allow-list that carries reasons. Fold bookkeeping already bumps the version.
Approach:
- Add $inc Audit.Version and the audit $set (the existing WithAuditCeremony helpers) to each update.
- Pipeline updates use WithAuditCeremonyPipeline.
Coordination: StudyRepository.cs sits in the FEAT-024 rule's paths. Its IsFoldOnlyChange free-retry rule stays correct, because a direct-write bump is charged. #2934 also touches this file.
| # | Acceptance criterion | Verification |
|---|---|---|
| 6.1 | Switch on; RemoveAnnotationsFromStudiesInProjectForQuestionAsync runs while a stale whole-Study save is in flight → the stale save conflicts and the annotations stay removed |
Testcontainers (flips 1.4 P3) |
| 6.2 | Switch on; ApplySimpleUpdates and the three inclusion passes → every modified Study's version goes up by exactly 1 per pass that modified it |
Testcontainers |
| 6.3 | A new direct Study, Project or Investigator write without a version bump → the build fails | Architecture test |
| 6.4 | Switch on; an inclusion recalculation during active screening (two contexts) → no screening decision is lost; the reviewer sees at most one retry | E2E (hermetic stack) |
R2: messaging resilience and durable events (four PRs)¶
PR-7: global retry and redelivery (M1; effort M; flag Messaging:GlobalRetry:Enabled)
Files: MassTransitHelpers.cs, a new ISyrfOwnsRetryPolicy marker on the definitions that already configure retry, and a cluster-gitops alert on _error queue depth (a separate cluster-gitops PR).
Approach:
- Use AddConfigureEndpointsCallback.
- Skip endpoints whose definition carries the marker: session, bulk PDF, jobs, the saga, and FEAT-024 consumers, whose typed refusals must never become faults.
- Apply UseMessageRetry with intervals of 200 ms, 1 s and 5 s (PROPOSAL), then UseScheduledRedelivery at 30 s, 2 min and 10 min (PROPOSAL), both for IsRetryableSaveFailure plus AggregateConcurrencyException only.
- Business exceptions still fail fast.
| # | Acceptance criterion | Verification |
|---|---|---|
| 7.1 | Switch on; an unmarked consumer throws a transient Mongo exception twice → it succeeds on the third attempt | MassTransit harness |
| 7.2 | Switch on; a consumer throws a business exception → exactly one attempt, then _error |
Harness |
| 7.3 | A marked endpoint → its own policy only (no stacked retry) | Harness inspecting the configured pipe (GetProbeResult) |
| 7.4 | A message lands in any _error queue → an alert fires in Grafana within 15 min (PROPOSAL) |
Staging rollout check |
| 7.5 | Switch on in the hermetic stack → the full smoke spec set passes | E2E (run:e2e-smoke) |
PR-8: saga timeout and partitions (M2; effort S)
Files: SearchImportJobStateMachine.cs and its handler tests. Coordinate with #2858, which edits the same file.
Approach:
- Schedule(ParsingTimeout) on entering Parsing, set to 2 h (PROPOSAL, configurable; above the parse job's 5-minute timeout with 4 retries).
- On expiry: FailSearchJobActivity, then Error. A late completion in Error is already ignored (:190-196).
- Partition count 8 (PROPOSAL), keyed by SearchId, so ordering per search is preserved.
- The saga state gains one additive field, the timeout token.
| # | Acceptance criterion | Verification |
|---|---|---|
| 8.1 | No parse result arrives within the timeout → the job ends in Error and the Project's import job shows the failure |
Saga harness with a test scheduler |
| 8.2 | The completion arrives before the timeout → the timeout is unscheduled; Completed |
Saga harness |
| 8.3 | Two searches import concurrently → both progress without waiting on each other, and per-search order is kept | Saga harness |
| 8.4 | Import a RIS file in the hermetic stack → it completes as today | E2E |
PR-9: consumer idempotency inventory (M3; effort M)
Files: a test-only RedeliveryContractTests per host, plus a table appended to docs/architecture/messaging.md (or the nearest existing messaging doc).
Approach: for each of the about 35 consumers, record its effect, idempotency key and mechanism. Deliver the same message (same MessageId) twice and assert one effect. Non-idempotent consumers are fixed in follow-up PRs, one per consumer family, each flagged only if the fix changes semantics.
| # | Acceptance criterion | Verification |
|---|---|---|
| 9.1 | Every consumer → appears in the table, marked idempotent / not / not applicable with its reason | Docs plus a test asserting table and registry agree |
| 9.2 | Each consumer marked idempotent → its double-delivery test passes | Harness plus Testcontainers |
| 9.3 | Each consumer marked not idempotent → it has a follow-up issue, and PR-7's switch is not turned on in production until that issue is closed or the endpoint is marked | Governance (issue links in the table) |
PR-10: durable domain-event outbox (E1, P8; effort L; flag DomainEvents:Outbox:Enabled; needs #3973 and PR-5)
Files:
- SharedKernel: IDomainEventOutbox (stage events in a session) and IDomainEventSink (deliver one envelope);
- Mongo.Common: MongoDomainEventOutbox (collection pmDomainEventOutbox) and DomainEventOutboxDispatcher (a hosted service);
- MongoUnitOfWorkBase.cs save paths;
- BSON class maps for the event types.
Approach:
1. Write. With the switch on, a save whose aggregate graph has pending events writes the aggregate and the event envelopes in one transaction. It joins the caller's transaction if one is active, as the FEAT-024 deferred save does; otherwise it opens one and commits with the unknown-result retry. A save with no events takes today's path unchanged.
2. Envelope. EventId, type discriminator, payload (BSON), OccurredAtUtc, aggregate type and id, and HostRole (API or PM), because the two hosts register different handlers.
3. Dispatch. Claim with a lease, invoke each handler through IDomainEventSink, record per-handler completion so a retry re-runs only the failed handlers, back off, abandon after 10 attempts (PROPOSAL) with an alert, and expire delivered rows by TTL. This copies the ActivityClaimRevocationOutbox pattern.
4. Batched saves. Envelopes are staged per batch inside that batch's write, so cancellation after a committed batch no longer loses its events (P8).
5. Bus-agnostic by construction. Only IDomainEventSink knows about MassTransit: the in-process EventManager sink today, and the two bus-sending handlers through the bus. #3986 is decided (ADR-021: v9 rejected 2026-10-03; Wolverine with a single switch-over chosen 2026-10-04), so the sink sends through MassTransit 8.5.11 until the switch and then through Wolverine. The store, the envelope and the write path stay as they are.
6. Spike first (1 day, inside the PR): confirm the driver-3 session sharing and the transaction cost on Atlas. Record the measured command count per event-raising save.
| # | Acceptance criterion | Verification |
|---|---|---|
| 10.1 | Switch on; the process is killed after the commit and before dispatch → after restart, the event's handlers run exactly once per handler | Testcontainers (dispatcher restarted in-test) |
| 10.2 | Switch on; the transaction aborts → neither the aggregate nor the envelope is persisted | Testcontainers replica set |
| 10.3 | Switch on; a handler throws → retried with backoff; the other handlers of that event are not re-run | Testcontainers |
| 10.4 | Switch on; a save with no pending events → the command list is identical to switch-off, including every FEAT-024 budget | Command-listener plus budget tests |
| 10.5 | Switch on; BatchedSaveManyAsync is cancelled after batch 1 → batch 1's events are delivered (flips 1.4 P8) |
Testcontainers |
| 10.6 | Switch on; a threshold change → the recalculation command is sent even when the bus is down at save time (the latch case of Appendix D B3) | Testcontainers plus harness with the bus stopped |
| 10.7 | Every domain event type → round-trips through its BSON envelope | Contract test (extends PR-3) |
| 10.8 | Invite a member and approve a join request in the hermetic stack → the mails arrive in Mailpit once | E2E |
| 10.9 | Staging, 7 days → zero abandoned envelopes and a p95 dispatch lag ≤ 5 s (PROPOSAL) | Staging rollout check |
R3: cache hardening (post-MVP; two PRs, in parallel)¶
PR-11: cache invalidation without the race (P6, P7; effort M; flag Persistence:Cache:PerKeyEviction)
Files: RepositoryCache.cs, MongoRepositoryBase.cs, MongoUnitOfWorkBase.cs (the invalidator calls).
Approach:
- Single-aggregate saves evict their key; bulk and direct writes keep the whole-type reset.
- A generation counter is captured before a load, and the load caches its result only if no invalidation of that key or type happened since.
- Never swap or dispose the live MemoryCache. Clear it instead, and prune the lock map.
| # | Acceptance criterion | Verification |
|---|---|---|
| 11.1 | A load starts, a save of that key completes, then the load finishes → the stale result is returned to its caller but not cached | Unit test with a controllable factory |
| 11.2 | A concurrent reset during reads → no ObjectDisposedException |
Stress unit test (1,000 iterations, PROPOSAL) |
| 11.3 | Switch on; saving Project A → a cached Project B is still a cache hit | Unit |
| 11.4 | The _locks map → bounded after 10,000 distinct keys |
Unit |
PR-12: stop the 409 leaks and measure the rest (P4, P5; effort M; flag Persistence:Cache:LeakAudit for the detector)
Files: ProjectController.cs (5 bulk endpoints plus UpdateStagePermissions), NotificationHub.cs, StudyController.cs (each re-read first), and RepositoryCache.cs (the detector).
Approach:
- Validate before mutating, or open BeginIsolatedReads before the load, per endpoint.
- Detector: on put, record a BSON hash; on expiry (not explicit eviction), re-hash. A mismatch means a cached instance was mutated and never saved. Log the type, the key and the stack captured at put time. Staging only.
| # | Acceptance criterion | Verification |
|---|---|---|
| 12.1 | Approving two join requests where one is invalid → 409, and a concurrent read sees neither approval; no join-approved mail is sent | Testcontainers (flips 1.4 P5) plus an API integration test |
| 12.2 | The same for each of the other located endpoints | API integration tests |
| 12.3 | Detector on, in the hermetic stack → the full smoke run reports the leaking call sites; each becomes a follow-up issue | E2E run plus issue list |
| 12.4 | Detector off → no hashing and no allocation on put | Unit (allocation assertion) |
5. Order and critical path¶
flowchart LR
PR1[PR-1 harness + detector] --> PR4[PR-4 audit restore] --> PR5[PR-5 non-upsert] --> PR10[PR-10 outbox]
T3973[#3973 await events] --> PR10
PR2[PR-2 inclusion latch]
PR3[PR-3 contract tests] --> PR10
PR1 --> PR6[PR-6 direct-write bump]
PR7[PR-7 global retry] --> PR9[PR-9 idempotency]
PR8[PR-8 saga]
PR5 --> PR11[PR-11 cache]
PR1 --> PR12[PR-12 409 leaks]- Start immediately, in parallel (disjoint files): PR-1 (Mongo.Common), PR-2 (consumer and
Project.cs), PR-3 (new test project), PR-7 (MassTransitHelpers.cs) and PR-8 (saga). - Sequential on
MongoExtensions.cs/MongoUnitOfWorkBase.cs: PR-1 → PR-4 → PR-5 → PR-10, plus #3973, which editsDispatchEventsin the same file. Critical path: #3973 and PR-1 → PR-4 → PR-5 → PR-10. - PR-6 follows PR-1 and runs alongside PR-4/PR-5 (
StudyRepository.csand the repositories only). - PR-9's not-idempotent list gates turning on PR-7's switch in production, not its merge.
- PR-11 waits for PR-5 (same files). PR-12 needs only PR-1.
- One worker per worktree. This host is the CI runner, so workers run single test classes niced.
6. Issues¶
-
3985 → PR-1, PR-4, PR-5, PR-6.¶
-
3984 → PR-2, PR-3, PR-7, PR-8, PR-9, PR-10 (split into six on approval).¶
- New issues on approval: PR-11 and PR-12, and the Phase 3 "scoped identity map" issue.
- Each issue copies its acceptance-criteria table from this plan.
7. Production data: separate, Chris-approved steps¶
Nothing in this plan writes production data. Two read-only dry runs on syrftest are proposed, each
run only after Chris approves it. Each uses countDocuments with maxTimeMS, at a quiet hour, and
GUIDs as CSUUID(...):
| Dry run | Query (sketch) | Informs |
|---|---|---|
| D-a: resurrection residue | pmStudy whose ProjectId has no pmProject; pmStudy whose SystematicSearchId has no pmSystematicSearch (a bounded $lookup per project, sampled first) |
Whether a cleanup of orphans is needed (P1) |
D-b: Project.Errors shapes |
pmProject.countDocuments({"Errors.0": {$exists: true}}), then a $project of Errors element types on at most 50 documents |
PR-2's tolerant class map (2.5) |
Any cleanup they reveal becomes its own plan: - dry-run counts, plus a list of ids exported off-cluster; - the delete is reversible, by restoring from that export; - a staging rehearsal first; - Chris's explicit approval before it runs.
8. Risks¶
| Risk | Mitigation |
|---|---|
| PR-5 changes the failure type that FEAT-024 retries depend on | The classifiers change in the same PR; the budget and conflict-retry tests are parameterised on the switch (5.4, 5.6); the FEAT-024 session reviews it |
| PR-6 raises conflict rates for reviewers during project-wide passes | Flagged; E2E 6.4; the staging metric is compared with the E11000 baseline |
| PR-10 adds a transaction to event-raising saves (invitations, join requests, threshold changes) | Only saves with pending events pay; the spike measures it; flagged per host |
| Outbox events are delivered at least once, so a mail could be sent twice | Per-handler completion records; mail handlers keyed on EventId (10.3, 10.8) |
| The global retry stacks on endpoints with their own policy, or retries FEAT-024 typed refusals | The marker opts them out; 7.3 inspects the pipe |
| Durable deletion (#2934) enabled before PR-5 is on would widen P1 a lot | Recorded as a dependency on #2934; D1 asks Chris to make it a gate |
| The MassTransit decision (#3986) lands mid-programme | Only the sink touches the bus; the store and the write path are bus-agnostic |
| Version-0 upsert kept (D2) leaves a tiny window for never-saved aggregates | Version-0 documents are new by construction; the PR-1 detector keeps counting |
9. Coordination with other active work¶
Open PRs touching these files (gh pr list --state open, 128 PRs checked on 3 Oct):
| PR | Overlap | Action |
|---|---|---|
| #2934 durable deletion kernel | StudyRepository.cs, IStudyRepository.cs, Project.cs |
Whichever lands second rebases. Deletion must not be enabled before PR-5's switch is on (D1) |
| #2858 legacy upload metadata contract | SearchImportJobStateMachine.cs and its tests |
PR-8 rebases on it, or the other way round |
| #3941 notifications inbox capture | Project.cs, ProjectController.cs |
PR-2 and PR-12 rebase; the notifications stack (#3932, #3938–#3947, #3965) is in flight |
| #3939 progressive shared review batches | IStudyRepository.cs |
Trivial rebase |
| #2572, #2224, #2469 (dormant) | Project.cs, ProjectController.cs |
None now |
Owning streams:
- FEAT-024 ("materialised project statistics" session): reviews PR-2, PR-5, PR-6 and PR-10, which touch StudyRepository.cs, the conflict classifiers, the inclusion fence and the deferred-dispatch seam.
- Backend bug plan: #3973 (prerequisite) and #3980.
- MassTransit ADR (#3986): PR-10's sink.
- Authentication migration session: none of these PRs touch Identity. Account deletion uses InvestigatorRepository saves (P1) but no identity code.
10. Decisions needed from Chris¶
- D1. Gate durable deletion on PR-5. Recommended: yes. #2934's enablement waits until
Persistence:Strict:NonUpsertSaveshas run clean in staging. - D2. New aggregates (
Version == 0): keep the upsert (recommended: smallest change, and the resurrection window needs a never-saved document to be deleted), or switch toInsertOnewith an E11000 fallback? - D3. Outbox mechanism: an in-house transactional outbox collection behind
IDomainEventOutbox(recommended), or wait for #3986 and use the MassTransit Mongo outbox? The in-house store matches four existing outboxes and survives any bus decision. - D4. PR-4 unflagged? Recommended: yes. It acts only on the failure path, and the cache eviction already assumes the invariant it restores.
- D5. PR-7 production order: turn global retry on in production only after PR-9's inventory is closed (recommended), or right after staging proof?
- D6. Cache direction: do R3 (PR-11/PR-12) now, or skip to the Phase 3 scoped identity map? Recommended: R3 now. It is cheap, it produces the leak list the identity map needs, and the identity map waits on the DI decision.
- D7. Read-only dry runs D-a and D-b on
syrftest(§7): approve, or defer until PR-1's staging detector has data? - D8. PROPOSAL values:
- 7-day staging soak (C8);
- conflict rate within 2× of baseline (5.7);
- retry intervals 200 ms / 1 s / 5 s and redelivery 30 s / 2 min / 10 min (PR-7);
- error-queue alert within 15 min (7.4);
- parsing timeout 2 h and 8 partitions (PR-8);
- outbox abandon after 10 attempts and p95 lag ≤ 5 s (PR-10);
- stress loop of 1,000 iterations (11.2).