hamid
602 lines
42 KiB
Markdown
602 lines
42 KiB
Markdown
# Post-development backend plan — fixes & improvements
|
||
|
||
**Audit date:** 2026-07-10 · **Scope:** the completed backend chain (backend-phase-0 → 15) under `server/`
|
||
· **Method:** read-only audit of code + `dev/` docs; every finding cites the file/line it was verified at.
|
||
Companion documents: [frontend-backend-gaps.md](frontend-backend-gaps.md) (the REQ-by-REQ contract
|
||
reconciliation) and [runtime-services.md](runtime-services.md) (the deployment topology).
|
||
|
||
The chain is genuinely complete against its own specs: all 16 backend phases shipped, 358 tests are green,
|
||
and the load-bearing invariants (balanced ledger, DB CHECKs, idempotency uniques, tenancy 404s, forward-only
|
||
status machines) verifiably exist in code. What remains falls into eight coherent "post-phases", ordered so
|
||
that each is a runnable unit of work: deployment blockers first, then a code-level money-correctness fix,
|
||
then the contract batch that unblocks the frontend lane, then the infrastructure and vendor swaps the seam
|
||
architecture was built for.
|
||
|
||
| Bucket | Theme | Blocking what? |
|
||
| --- | --- | --- |
|
||
| post-phase-1 | Security & config hygiene | Any non-local deployment |
|
||
| post-phase-2 | Money-path correctness completion | Ledger ⇄ bank reconciliation |
|
||
| post-phase-3 | Frontend-unblock contract batch | 11 of 12 client domains are still mock-primary |
|
||
| post-phase-4 | Scheduling, locking & multi-instance readiness | Unattended operation; >1 API instance |
|
||
| post-phase-5 | Identity & trust rails go real | Real nurses onboarding (OTP, KYC, docs) |
|
||
| post-phase-6 | Money rails go real | Real payments, payouts, tax |
|
||
| post-phase-7 | Observability, audit & ops hardening | Production diagnosability |
|
||
| post-phase-8 | Scale & later | Search scale, analytics, doc debt |
|
||
|
||
Status legend used below — **Current state** always cites what the code does *today*.
|
||
|
||
---
|
||
|
||
## post-phase-1 — Security & config hygiene (do before anything is deployed)
|
||
|
||
Everything in this bucket is small (S) and none of it changes behavior — but each item is a deployment
|
||
blocker, and two of them are live credential leaks sitting in git today.
|
||
|
||
### 1.1 Rotate and remove the committed SQL Server `sa` connection string
|
||
|
||
- **Why:** `appsettings.json` and `appsettings.Development.json` both commit a real connection string —
|
||
public IP `87.107.152.16`, login `sa`, plaintext password — for the app DB *and* the log DB. Anyone with
|
||
repo access owns the database (all PII ciphertext + the encryption keys sit in the same repo, see 1.2).
|
||
This directly violates root `CLAUDE.md` working agreement #6 ("Never commit secrets").
|
||
- **Current state:** `server/src/API/Baya.Web.Api/appsettings.json:3-4` and
|
||
`appsettings.Development.json:3-4` (byte-identical files); consumed at
|
||
`server/src/Infrastructure/Baya.Infrastructure.Persistence/ServiceConfiguration/ServiceCollectionExtensions.cs:41`
|
||
and by the Serilog sink at
|
||
`server/src/Infrastructure/Baya.Infrastructure.CrossCutting/Logging/LoggingConfiguration.cs:44`.
|
||
- **Change:** rotate the `sa` password on that server (assume compromised); create a least-privilege app
|
||
login; move both connection strings to user-secrets (dev) / environment variables (deploy); commit only a
|
||
placeholder. Consider `git filter-repo` history scrubbing, and add a secret-scanning pre-commit hook.
|
||
- **Files/layers:** `appsettings*.json` only (config).
|
||
- **Effort:** S · **Risk:** low (config move) · **Deps:** none. **Do this first.**
|
||
|
||
### 1.2 Replace placeholder JWE signing/encryption keys and field-encryption keys
|
||
|
||
- **Why:** the JWT/JWE `SecretKey`/`Encryptkey` are starter-template placeholders and the PII
|
||
field-encryption keys are committed `local-dev-…-change-me` strings. If these defaults reach any shared
|
||
environment, every token is forgeable and every encrypted PII column is decryptable. The access-token
|
||
lifetime is also ~7 days (`ExpirationMinutes: 10000`) and `RequireHttpsMetadata = false`.
|
||
- **Current state:** `server/src/API/Baya.Web.Api/appsettings.json:7-8` (keys), `:12` (expiry);
|
||
`Seams:FieldEncryption` at `appsettings.json:15-18`; the JWE decryption key is wired at
|
||
`server/src/Infrastructure/Baya.Infrastructure.Identity/ServiceConfiguration/ServiceCollectionExtension.cs:135`
|
||
and `RequireHttpsMetadata = false` at `:139`. `SymmetricFieldEncryptor` derives a real AES-256-CBC key
|
||
from whatever string is configured
|
||
(`server/src/Infrastructure/Baya.Infrastructure.CrossCutting/Seams/SymmetricFieldEncryptor.cs:25`) — the
|
||
crypto is fine; the key *management* is dev-grade (mocks-registry row `IFieldEncryptor` 🟡 agrees).
|
||
- **Change:** per-environment secrets (env vars / Key Vault / KMS per the registry's "make it real"), a
|
||
sane access-token lifetime (≤ 60 min; refresh flow already exists), `RequireHttpsMetadata = true` outside
|
||
Development, real `Issuer`/`Audience` values (currently `"MyWebsite"`). Note: rotating the field key
|
||
requires a re-encryption migration for existing rows — do it before real PII exists.
|
||
- **Effort:** S (config) + M if key-rotation tooling is wanted · **Risk:** medium (existing dev-DB
|
||
ciphertext becomes unreadable — acceptable pre-launch) · **Deps:** 1.1.
|
||
|
||
### 1.3 Remove or environment-gate the seeded `admin` / `qw123321` user
|
||
|
||
- **Why:** every non-Testing boot creates a well-known admin account with a hardcoded weak password and the
|
||
full admin role — in production too.
|
||
- **Current state:** `server/src/Infrastructure/Baya.Infrastructure.Identity/Identity/SeedDatabaseService/SeedDataBase.cs:48`
|
||
(`CreateAsync(user, "qw123321")`), invoked from `server/src/API/Baya.Web.Api/Program.cs:102`.
|
||
- **Change:** read the bootstrap admin credentials from configuration and only seed when explicitly
|
||
configured (or Development-only); force a password change on first login.
|
||
- **Effort:** S · **Risk:** low · **Deps:** none.
|
||
|
||
### 1.4 Environment-gate the auto-seeded sandbox ZarinPal payment gateway
|
||
|
||
- **Why:** `SeedPaymentGatewaysAsync` idempotently inserts an **active** sandbox ZarinPal gateway row
|
||
(all-zeros merchant id) on every boot — a production DB would silently contain an active sandbox money
|
||
gateway.
|
||
- **Current state:** `server/src/API/Baya.Web.Api/Program.cs:103` →
|
||
`server/src/Infrastructure/Baya.Infrastructure.Persistence/ServiceConfiguration/ServiceCollectionExtensions.cs:106-117`
|
||
(sandbox row, `config_json` encrypted via the DbContext converter).
|
||
- **Change:** seed only in Development/Testing, or seed `is_active = false` and require an admin to
|
||
activate a real gateway (`payment_gateways` is already admin data).
|
||
- **Effort:** S · **Risk:** low · **Deps:** none; pairs with 6.1.
|
||
|
||
### 1.5 Fix the Kestrel HTTP/2-only default
|
||
|
||
- **Why:** `Kestrel:EndpointDefaults:Protocols = "Http2"` (set for the gRPC plugin) makes every endpoint
|
||
HTTP/2-only. Browsers can't speak h2c, so any non-TLS hop — container health probes, an HTTP/1.1 reverse
|
||
proxy leg, plain-HTTP Swagger — breaks. Locally it only works because `https://localhost:5002` negotiates
|
||
via ALPN.
|
||
- **Current state:** `server/src/API/Baya.Web.Api/appsettings.json:29-33` (both files). The gRPC plugin that
|
||
motivated it serves a single duplicate OTP/token service
|
||
(`server/src/API/Plugins/Baya.Web.Plugins.Grpc/Services/UserGrpcServices.cs:18`).
|
||
- **Change:** default `Http1AndHttp2`; give gRPC its own `Http2` endpoint if kept (see 7.5 for the
|
||
keep-or-remove decision).
|
||
- **Effort:** S · **Risk:** low · **Deps:** none.
|
||
|
||
### 1.6 Make rate limiting proxy-aware and align the two payment webhooks
|
||
|
||
- **Why:** all rate-limit partitions key on `RemoteIpAddress` and no `ForwardedHeaders` middleware is
|
||
registered — behind any reverse proxy every client shares one 100 req/min bucket (self-DoS). Separately,
|
||
the two webhook siblings disagree: the BNPL webhook runs the 20/min `sensitive` policy while the card
|
||
webhook has only the global 100/min fallback — one of them is wrong on purpose or both by accident.
|
||
- **Current state:** partition key at
|
||
`server/src/API/Baya.WebFramework/ServiceConfiguration/RateLimitingServiceExtension.cs:69`; global limiter
|
||
`:35`; `Program.cs` has no `UseForwardedHeaders` (checked `server/src/API/Baya.Web.Api/Program.cs`).
|
||
Webhooks: `Controllers/V1/WebhooksBnplController.cs:28` (`sensitive`) vs `Controllers/V1/WebhooksController.cs:25`
|
||
(`[AllowAnonymous]`, global only).
|
||
- **Change:** add `ForwardedHeaders` middleware (trusting only the known proxy), partition on the resolved
|
||
client IP, and pick one deliberate webhook policy (PSP callbacks are bursty — a dedicated `webhook`
|
||
policy keyed per-provider is safer than `sensitive`).
|
||
- **Effort:** S · **Risk:** low–medium (limiter behavior changes) · **Deps:** deployment topology decision.
|
||
|
||
---
|
||
|
||
## post-phase-2 — Money-path correctness completion
|
||
|
||
The ledger invariants verified clean (balanced groups throw at
|
||
`server/src/Core/Baya.Domain/Entities/Payments/LedgerPosting.cs:26,65,178,204`; the four DB CHECKs exist —
|
||
`CK_Bookings_AmountSplit`, `CK_NursePayouts_NetSplit`, `CK_Refunds_LegSplit`, `CK_BnplTransactions_SettleSplit`
|
||
— per the EF configs and `Migrations/ApplicationDbContextModelSnapshot.cs:345,3543,3842,206`). This bucket
|
||
closes the holes *around* those invariants.
|
||
|
||
### 2.1 Wire the unreachable BNPL/manual refund settlement (dead-end money state) — **top code fix**
|
||
|
||
- **Why:** a card refund posts its `refund_payable ↔ escrow_held` clearing immediately. A BNPL-revert or
|
||
manual-bank refund is left in `processing` with the clearing "deferred to reconciliation" — but **no
|
||
reconciliation path exists anywhere**: `Refund.MarkSucceededAsync` has zero call sites, no admin
|
||
endpoint/webhook/job performs `processing → succeeded`, and `LedgerPosting.RefundPayableClearing` has
|
||
exactly one call site (the immediate card path). Every BNPL/manual refund permanently overstates
|
||
`escrow_held` and strands `refund_payable` — the ledger will never reconcile with the bank.
|
||
- **Current state:** `server/src/Core/Baya.Domain/Entities/Refunds/Refund.cs:105` (uncalled);
|
||
`server/src/Core/Baya.Application/Features/Refunds/Commands/CreateRefund/CreateRefundCommand.Handler.cs:139-143`
|
||
(clearing only when already `Succeeded`); `:226-231` (BNPL/manual → `MarkProcessing`, never succeeded);
|
||
the `Processing → Succeeded` edge exists unused in
|
||
`server/src/Core/Baya.Domain/Entities/Refunds/RefundTransitions.cs:16`; admin surface is create+list only
|
||
(`Controllers/V1/AdminRefundsController.cs:33`). The b11 handoff promised at least a manual trigger
|
||
(`dev/shared-working-context/backend/handoff/after-backend-phase-11.md:37`).
|
||
- **Change:** a `ConfirmRefundSettlementCommand` (admin `POST admin_refunds/{id}/confirm_settlement`, plus a
|
||
BNPL-callback branch when the provider confirms customer cash-back) that transitions
|
||
`processing → succeeded`, stamps `settled_at`, and posts `LedgerPosting.RefundPayableClearing` in the same
|
||
commit; a `mark_failed` counterpart. Tests for both channels.
|
||
- **Files/layers:** Application `Features/Refunds/`, API `AdminRefundsController`, Domain (rename
|
||
`MarkSucceededAsync` — it's not async), tests.
|
||
- **Effort:** M · **Risk:** medium (money path — but additive) · **Deps:** none; do before real BNPL (6.2).
|
||
|
||
### 2.2 Add the promised-but-missing FKs on the forward-dep columns
|
||
|
||
- **Why:** b11 created `refunds.ticket_id`, `nurse_clawbacks.original_payout_id` /
|
||
`recovered_in_payout_id`, and `invoices.partner_center_id` as FK-less nullable columns "until the target
|
||
table ships". The targets all shipped (b13 `nurse_payouts`, b15 `tickets`/`partner_centers`) and the
|
||
**values** are wired, but no phase added the constraints — referential integrity rests on application
|
||
discipline, and the config comments are now false.
|
||
- **Current state:** stale comment "no FK yet (tickets does not exist)" at
|
||
`server/src/Infrastructure/Baya.Infrastructure.Persistence/Configuration/RefundsConfig/RefundConfig.cs:41`;
|
||
`NurseClawbackConfig.cs:12` (comment says b13 "wires the FKs" — it didn't; b13's `NursePayoutEngine`
|
||
migration touches no clawback FK); `InvoicesConfig/InvoiceConfig.cs:11` (`partner_center_id` has **no FK
|
||
and no index**); the b15 migration adds only `FK_NurseProfiles_PartnerCenters_PartnerCenterId`
|
||
(`Migrations/20260709232741_MessagingAndPartnerCenters.cs:293`). Value-side wiring confirmed:
|
||
`CreateRefundCommand.Handler.cs:53-61` (auto-ticket), `Refunds/NurseClawback.cs:50` (`MarkRecovered`),
|
||
`IssueInvoiceCommand.Handler.cs:63-64` (issuer/center).
|
||
- **Change:** one additive migration adding the three FK sets (`ON DELETE NO ACTION`) + an index on
|
||
`invoices.partner_center_id`; update the three config comments.
|
||
- **Effort:** S · **Risk:** low (data is young; verify no orphans first) · **Deps:** none.
|
||
|
||
### 2.3 Extend `IAuditable` to the admin-decided money & trust entities
|
||
|
||
- **Why:** the append-only `audit_logs` diff interceptor covers exactly three entities — `PlatformConfig`,
|
||
`PartnerCenter`, `Review`. Admin decisions on refunds (approve/reject), payouts (process/retry/fail), and
|
||
**nurse verification** (the trust-critical `is_verified` flip / suspend) leave no audit-diff row. For a
|
||
trust-first escrow platform these are precisely the actions an auditor asks about. (The ledger itself is
|
||
fine — append-only by construction, `LedgerEntry` is `IEntity`-only at
|
||
`server/src/Core/Baya.Domain/Entities/Payments/LedgerEntry.cs:13`.)
|
||
- **Current state:** the three implementors — `Domain/Entities/Configuration/PlatformConfig.cs:12`,
|
||
`Domain/Entities/PartnerCenters/PartnerCenter.cs:16`, `Domain/Entities/Reviews/Review.cs:15`. `Refund`
|
||
(`Domain/Entities/Refunds/Refund.cs:77`) and `NursePayout` (`Domain/Entities/Payouts/NursePayout.cs:62`)
|
||
are not `IAuditable`; `NurseVerification` isn't either.
|
||
- **Change:** add `IAuditable` to `Refund`, `NurseClawback`, `NursePayout`, `NursePayoutBatch`,
|
||
`NurseVerification` (the interceptor at `Persistence/Interceptors/AuditFieldInterceptor` already handles
|
||
any `IAuditable`); confirm `[AuditRedacted]` covers `iban_snapshot` before enabling.
|
||
- **Effort:** S–M · **Risk:** low (write-volume growth on `audit_logs`; see 7.4 archival) · **Deps:** none.
|
||
|
||
### 2.4 Close the refund channel-execute-before-commit crash window
|
||
|
||
- **Why:** `CreateRefundCommand` executes the external channel call (PSP refund / BNPL revert) **before**
|
||
the first DB commit — a crash between provider success and commit loses the record of an executed refund.
|
||
The idempotency key means a *retry* won't double-refund, but nothing retries automatically and no record
|
||
exists to reconcile against.
|
||
- **Current state:** `Features/Refunds/Commands/CreateRefund/CreateRefundCommand.Handler.cs:106-111`
|
||
(channel executes), commit later in the same handler; idempotency key at `:85`.
|
||
- **Change:** persist the refund row in `pending` state (commit) *before* the channel call, then execute and
|
||
update — the standard two-phase intent/confirm shape the webhook handler already uses
|
||
(claim-key-first at `Features/Payments/Commands/HandlePaymentWebhook/HandlePaymentWebhookCommand.Handler.cs:76-88`).
|
||
- **Effort:** M · **Risk:** medium (touches the refund state machine; full test pass required) · **Deps:**
|
||
do together with 2.1.
|
||
|
||
### 2.5 Test the untested admin money paths
|
||
|
||
- **Why:** `WriteOffClawbackCommand` — an admin action that posts a `bad_debt` ledger group — has **zero
|
||
tests** (grep across `server/src/Tests` finds no `WriteOff` match). The webhook duplicate-race path
|
||
(`DbUpdateException` on a concurrent same-key insert) is only exercised sequentially. Messaging has no
|
||
Foundation-level handler tests (the `is_internal` boundary is covered only end-to-end in
|
||
`Tests/Baya.Test.Api/MessagingApiTests.cs:33`).
|
||
- **Current state:** handler at
|
||
`Features/Refunds/Commands/WriteOffClawback/WriteOffClawbackCommand.Handler.cs:26`; coverage inventory:
|
||
49 Foundation + 33 Api test files; payout retry / webhook replay / clawback fork / rebuild convergence
|
||
**are** tested (`Tests/Baya.Test.Foundation/Payouts/PayoutHandlerTests.cs:211`,
|
||
`Payments/PaymentWebhookTests.cs:64`, `Refunds/RefundHandlerTests.cs:81`, `Search/SearchIndexTests.cs:184`).
|
||
- **Change:** add write-off unit + API tests (balanced group, idempotency, 404/409 paths), a true racing
|
||
webhook-insert test, and Foundation tests for `PostMessage`/`GetTicketThread` internal-flag handling.
|
||
- **Effort:** S–M · **Risk:** none · **Deps:** none.
|
||
|
||
### 2.6 Retire the orphaned `refund_ticket_required` config key
|
||
|
||
- **Why:** b15 superseded the config-gated rule by unconditionally auto-opening a refund ticket, so the
|
||
seeded key has zero production consumers and a now-false description ("Off until b15 ships the tickets
|
||
table") — a small honesty debt that will mislead an operator.
|
||
- **Current state:** seed row at
|
||
`Persistence/Configuration/ConfigurationConfig/PlatformConfigConfig.cs:49`; auto-open at
|
||
`CreateRefundCommand.Handler.cs:53-61`; only remaining reference is a test stub
|
||
(`Tests/Baya.Test.Foundation/Refunds/RefundsTestHost.cs:174`).
|
||
- **Change:** delete the seed row (migration) or repurpose it to gate whether a *customer-visible* ticket is
|
||
required; update the description either way.
|
||
- **Effort:** S · **Risk:** low · **Deps:** none.
|
||
|
||
---
|
||
|
||
## post-phase-3 — Frontend-unblock contract batch
|
||
|
||
Full field-level detail, evidence, and priority ordering live in
|
||
[frontend-backend-gaps.md](frontend-backend-gaps.md). Summary: of the 15 filed REQs, **REQ-001 and
|
||
REQ-015 are effectively done, REQ-010 is a doc fix, and the other 12 are undelivered**; 11 of the client's
|
||
12 service domains still default to mock-primary, most gated on exactly these items. This bucket is one
|
||
backend phase-sized batch of small DTO/endpoint additions:
|
||
|
||
- **3.1 Booking-surface fields (S):** `variantPrice` on `BookingRequestDto`, `variantLabel` (+ optional
|
||
`patientAge`) on `BookingRequestListItemDto` (REQ-013/014).
|
||
- **3.2 Identity/profile fields (S–M):** patient `relation` + `conditions` (REQ-005); customer
|
||
name/preferred-language upsert (REQ-007); avatar upload endpoint + `avatarUrl` (REQ-006 — the only item
|
||
needing multipart + `IObjectStorage`).
|
||
- **3.3 Address fields (S):** accept the client map pin on create/update (REQ-008 — matters for EVV
|
||
accuracy later) + `provinceId` on `CustomerAddressDto` (REQ-009).
|
||
- **3.4 Search & public profile (M — the single highest-leverage item):** enrich `NurseSearchResultDto`
|
||
with `nurseName`/`avatarUrl` (+ optional `distanceKm`) and add the aggregated public
|
||
`GET nurses/{id}/profile` (REQ-012). Unblocks the discovery funnel (C2/C3).
|
||
- **3.5 Verification details (S–M):** nurse-facing `credential_details` command + `isRequired` on
|
||
`VerificationStepDto` (REQ-011 — without it the INO number/specialties are silently dropped).
|
||
- **3.6 Auth polish (S):** `codeLength`/`expiresInSeconds` on `RequestOtpResult` (REQ-002); machine-readable
|
||
`code` (+ `retryAfterSeconds`) on OTP failures (REQ-003 — needs a small `OperationResult`/envelope
|
||
extension, the only cross-cutting piece).
|
||
- **3.7 Zero-code confirmations & doc fixes (S):** answer REQ-001/004/015 in the tracker; fix the stale
|
||
`page_size` occurrences in `dev/contracts/domains/*.md` (REQ-010 — the server verifiably binds camelCase
|
||
`pageSize`); mark every delivered REQ `delivered in …` (all 15 currently read `Status: open`).
|
||
|
||
**Effort:** one M-sized phase overall · **Risk:** low (additive DTO fields; regenerate
|
||
`dev/contracts/openapi/swagger.v1.json` after) · **Deps:** none — can run in parallel with post-phase-1/2.
|
||
|
||
---
|
||
|
||
## post-phase-4 — Scheduling, locking & multi-instance readiness
|
||
|
||
### 4.1 Real job scheduler + register the four deferred crons
|
||
|
||
- **Why:** only two recurring jobs exist (booking-request expiry every 1 min, notification retention every
|
||
24 h). The verification credential-expiry scan, the EVV no-show sweep, the **weekly payout batch**, and
|
||
the Moadian reconciliation poll are admin-manual-only — their cadence config keys are seeded but nothing
|
||
reads them on a schedule. Operationally today: credentials never re-expire, no-shows are never flagged,
|
||
and **nurses are not paid unless an operator clicks**.
|
||
- **Current state:** the two hosted services at
|
||
`Persistence/ServiceConfiguration/ServiceCollectionExtensions.cs:63,67` (intervals hardcoded at
|
||
`Services/Notifications/NotificationRetentionHostedService.cs:19` and
|
||
`Services/Booking/BookingRequestExpiryHostedService.cs:21`). Manual triggers:
|
||
`Controllers/V1/AdminVerificationsController.cs:49` (`scan_expiring`), `AdminEvvController.cs:37`
|
||
(`detect_no_shows`), `AdminPayoutsController.cs:44` (batch generate). Unconsumed cadence keys:
|
||
`verification_expiry_scan_cadence_hours`, `no_show_scan_cadence_hours`, `nurse_payout_interval_days`
|
||
(`PlatformConfigConfig.cs:46,48,36`). Note: **no `IJobScheduler` interface exists** — the registry row is
|
||
aspirational naming; there is nothing to swap behind, only hosted services to re-home. No
|
||
Hangfire/Quartz package is referenced (`server/Directory.Packages.props` — verified absent).
|
||
- **Change:** adopt Hangfire (SQL Server storage — no new infra) or Quartz; move the two existing sweeps
|
||
and add recurring jobs for expiry-scan, no-show, payout-batch generation (+ 2.1's reconciliation and 6.5's
|
||
Moadian poll), each reading its seeded cadence key; keep the admin manual triggers as overrides. Payout
|
||
processing (money-moving) can stay human-approved — schedule *generation*, keep `process` manual until
|
||
trust is earned.
|
||
- **Effort:** M–L · **Risk:** medium (new infra dependency in-app; jobs must stay idempotent — they already
|
||
are by design) · **Deps:** none hard; pairs with 4.2 for multi-instance.
|
||
|
||
### 4.2 Redis for `ICacheService` + `IDistributedLock`
|
||
|
||
- **Why:** both are in-process today. Cache: fine single-instance, silently wrong (stale geo/catalog/config
|
||
reads, generation-token invalidation not shared) the moment a second instance runs. Lock: the money-path
|
||
mutex (`booking:{id}:payment` / `:refund`) is a per-key `SemaphoreSlim` — no cross-instance protection
|
||
(DB uniques remain the correctness backstop, as designed, but the lock is doing nothing across nodes).
|
||
- **Current state:** `CrossCutting/Seams/MemoryCacheService.cs:11` and `InProcessDistributedLock.cs:14-22`,
|
||
registered at `CrossCutting/ServiceConfiguration/ServiceCollectionExtension.cs:27,61`. No Redis package
|
||
referenced.
|
||
- **Change:** add `StackExchange.Redis`; `RedisCacheService` (same key/TTL scheme) +
|
||
`RedisDistributedLock` (SET NX PX + token-checked release, lease ≥ the longest money handler);
|
||
config-selected registration per the registry's make-it-real steps (rows `ICacheService`,
|
||
`IDistributedLock`).
|
||
- **Effort:** M · **Risk:** medium (lock semantics under expiry; keep DB uniques authoritative) ·
|
||
**Deps:** Redis service (see runtime-services.md).
|
||
|
||
### 4.3 Separate migrations from boot (multi-instance + least privilege)
|
||
|
||
- **Why:** every non-Testing boot runs `MigrateAsync` + three seeders — concurrent instance start-ups race
|
||
on DDL (no distributed lock exists at boot), and the app login needs permanent DDL rights.
|
||
- **Current state:** `server/src/API/Baya.Web.Api/Program.cs:99-104`;
|
||
`Persistence/ServiceConfiguration/ServiceCollectionExtensions.cs:92` (`MigrateAsync` unconditional).
|
||
- **Change:** a deploy-time migration step (`dotnet ef database update` in CI, or a `--migrate` one-shot
|
||
mode) and boot-time schema *check* instead of apply; seeders become idempotent deploy steps.
|
||
- **Effort:** S–M · **Risk:** low · **Deps:** CI/CD pipeline exists.
|
||
|
||
---
|
||
|
||
## post-phase-5 — Identity & trust rails go real
|
||
|
||
Ordered by user impact: nobody can log in without 5.1.
|
||
|
||
### 5.1 Real SMS gateway behind `ISmsSender` — **launch-critical**
|
||
|
||
- **Why:** OTP delivery is a log statement; no real user can ever log in. This is the single seam standing
|
||
between the platform and its first real session.
|
||
- **Current state:** `CrossCutting/Seams/LoggingSmsSender.cs:12-16` (logs the code, phone last-4),
|
||
registered at `ServiceCollectionExtension.cs:32`. No `Seams:Sms` options exist yet (verified:
|
||
`SeamOptions.cs` has no Sms group).
|
||
- **Change:** per registry row `ISmsSender`: pick Kavenegar/Ghasedak/SMS.ir, add `Seams:Sms:{ApiKey,
|
||
SenderLine,BaseUrl}` options + package, implement pattern/template OTP send, config-selected
|
||
registration. Keep the per-phone resend window and `otp` rate policy untouched.
|
||
- **Effort:** M · **Risk:** low (isolated seam) · **Deps:** vendor account.
|
||
|
||
### 5.2 Real Shahkar + e-KYC vendors (`IShahkarVerifier`, `IIdentityKycProvider`)
|
||
|
||
- **Why:** nurse verification currently passes any well-formed input (mock passes everything except two
|
||
configured magic values) — the trust engine's automated steps assert nothing real.
|
||
- **Current state:** `CrossCutting/Seams/MockShahkarVerifier.cs:17-37`, `MockIdentityKycProvider.cs:17-25`;
|
||
registrations `:45-46`; config defaults `SeamOptions.cs:157-184`. The pipeline already persists
|
||
`external_response_json` and handles shared-SIM/mismatch as explicit states — the handler side is ready.
|
||
- **Change:** per registry rows: one Finnotech-style bridge client for both استعلامها; keep shared-SIM as a
|
||
handled failure; persist real vendor refs. The phone-change re-trigger already works
|
||
(`ShahkarVerifiedAt` reset on phone change — b2).
|
||
- **Effort:** M–L (vendor onboarding dominates) · **Risk:** medium (real-world failure modes) · **Deps:**
|
||
vendor contract; 5.1 not required but sensible first.
|
||
|
||
### 5.3 Real استعلام شبا (`IBankAccountOwnershipVerifier`)
|
||
|
||
- **Why:** the b13 first-payout gate (`matched_national_id = 1`) is currently satisfied by a mock that
|
||
matches every IBAN except one magic value — real money would flow against unverified account ownership.
|
||
- **Current state:** `CrossCutting/Seams/MockBankAccountOwnershipVerifier.cs:17-26` (nurseNationalId
|
||
deliberately ignored, comment `:21-22`); registration `:36`; defaults `SeamOptions.cs:204`.
|
||
- **Change:** per registry row; verify the payout gate end-to-end (`is_primary=1 AND is_verified=1 AND
|
||
matched_national_id=1` skip-with-reason path already exists in b13).
|
||
- **Effort:** M · **Risk:** medium (money gate) · **Deps:** same vendor family as 5.2 — bundle them.
|
||
|
||
### 5.4 Real geocoder (`IGeocoder`) behind Neshan
|
||
|
||
- **Why:** address coordinates (and therefore the EVV distance check) are deterministic fakes jittered ±5 km
|
||
around 8 hardcoded city centroids — EVV mismatch alerts are currently noise. Accepting the client pin
|
||
(REQ-008, post-phase-3) reduces but doesn't remove the need.
|
||
- **Current state:** `CrossCutting/Seams/MockGeocoder.cs:15-52`; registration `:40`; `Seams:Geocoding` is
|
||
one of only three seam sections present in `appsettings.json:22-26`.
|
||
- **Change:** per registry row (Neshan client, rate-limit/retry, keep the null-coordinate path).
|
||
- **Effort:** M · **Risk:** low · **Deps:** REQ-008 first (pin > geocode for EVV).
|
||
|
||
### 5.5 Real object storage (`IObjectStorage`) — MinIO/S3/ArvanCloud
|
||
|
||
- **Why:** verification documents (and future avatars/invoice PDFs) live on the API host's local disk under
|
||
a temp root — non-durable, non-shared, and `GetUrl` returns a `file://` URI rather than a presigned URL,
|
||
so the b6 "short-lived signed URL" contract is only shape-deep.
|
||
- **Current state:** `CrossCutting/Seams/LocalDiskObjectStorage.cs:12-56` (temp-dir fallback `:20`,
|
||
`file://` URL); `Seams:ObjectStorage:RootPath` empty in `appsettings.json:19-21`.
|
||
- **Change:** per registry row: S3-compatible client, presigned PUT/GET with expiry, bucket + creds from
|
||
config; migrate any existing dev files or reset.
|
||
- **Effort:** M · **Risk:** low · **Deps:** storage service; REQ-006 (avatar) builds on this.
|
||
|
||
### 5.6 Accept manual as the real path for MoH/INO credentials and partner licensing (document, don't build)
|
||
|
||
- **Why:** `ICredentialVerifier` and `ILicenseVerificationService` return "needs manual review" by design —
|
||
there is **no public MoH/INO/eNamad B2B API** today. The admin review flows are the real mechanism; the
|
||
seams exist so a portal API can slot in if one appears.
|
||
- **Current state:** `CrossCutting/Seams/MockCredentialVerifier.cs:18`,
|
||
`MockLicenseVerificationService.cs:26`; registry rows agree ("no public B2B API today").
|
||
- **Change:** none in code. Mark these 🟡 rows as "manual = intended MVP state" in the registry so they stop
|
||
reading as debt.
|
||
- **Effort:** S (docs) · **Risk:** none.
|
||
|
||
---
|
||
|
||
## post-phase-6 — Money rails go real
|
||
|
||
The seam shapes are faithful (idempotency keys, server-side re-verify, upsert-first webhooks are already the
|
||
handler behavior — verified at `HandlePaymentWebhookCommand.Handler.cs:31-88`), so each swap is an adapter,
|
||
not a redesign. **Do 2.1 first** so the BNPL refund path is complete before real money uses it.
|
||
|
||
### 6.1 Real PSP/IPG + webhook signatures + تسهیم (`IPaymentProvider`, `IWebhookVerifier`, `ISettlementSplitProvider`)
|
||
|
||
- **Why:** card capture, callback authenticity, and the settlement split are all deterministic mocks
|
||
(`VerifyAsync` always succeeds echoing the expected amount; any callback is "validly signed" unless it
|
||
contains a magic marker; any balanced split "settles"). The legal تسهیم model (provider splits to
|
||
registered IBANs; platform never holds funds) only exists as an interface.
|
||
- **Current state:** `CrossCutting/Seams/MockPaymentProvider.cs:14-24`, `MockWebhookVerifier.cs:15-21`,
|
||
`MockSettlementSplitProvider.cs:13`; registrations `:58-60`; merchant creds intended to come from the
|
||
encrypted `payment_gateways.config_json` (seed at `Persistence/ServiceCollectionExtensions.cs:109-117`).
|
||
- **Change:** per registry rows 39–41: one acquirer-with-تسهیم (ZarinPal/Sadad/Vandar/Jibit); real
|
||
`InitPaymentAsync`/`VerifyAsync`/`RefundAsync`; per-provider HMAC verification of the raw body; a
|
||
provider registry/factory selected per gateway row; persist full gateway responses.
|
||
- **Effort:** L · **Risk:** high (real money; Shaparak certification lead time) · **Deps:** merchant
|
||
registration; 1.4; 2.1; 4.2 (real lock) strongly recommended.
|
||
|
||
### 6.2 Real BNPL adapters (`IBnplProvider` / `IBnplProviderResolver`)
|
||
|
||
- **Why:** the full BNPL state machine runs against one mock provider; per-contract commission and
|
||
non-instant settlement are simulated by config.
|
||
- **Current state:** `CrossCutting/Seams/MockBnplProvider.cs:17-53`, `MockBnplProviderResolver.cs:14-17`;
|
||
registrations `:72-74`; `Seams:Bnpl:*` defaults `SeamOptions.cs:98-120`. `bnpl_settlement_entries`
|
||
(tranched settlement) is modeled-but-not-built by design
|
||
(`dev/shared-working-context/backend/handoff/after-backend-phase-12.md:40`).
|
||
- **Change:** per registry row 46: SnappPay (OAuth verb set) and/or Digipay adapters, creds from encrypted
|
||
gateway config, Toman↔IRR only via `ICurrencyNormalizer`, per-contract commission read from the settle
|
||
response, resolver registered per `provider_code`.
|
||
- **Effort:** L · **Risk:** high · **Deps:** 2.1 (revert clearing), 6.1 patterns, provider contracts.
|
||
|
||
### 6.3 Real PAYA/SATNA payout rail (`IBankTransferProvider`) + async reconciliation
|
||
|
||
- **Why:** payouts "settle" instantly in the mock, collapsing the real `submitted → paid/failed` async
|
||
reconciliation; no money reaches nurses.
|
||
- **Current state:** `CrossCutting/Seams/MockBankTransferProvider.cs:18-31`; registration `:82`;
|
||
`Seams:BankTransfer` defaults `SeamOptions.cs:61-72`. The status machine, batch idempotency key,
|
||
`nurse_payout_booking_links` UNIQUE (`Persistence/Configuration/PayoutsConfig/NursePayoutBookingLinkConfig.cs:22`),
|
||
and PAYA/SATNA threshold selection already exist.
|
||
- **Change:** per registry row 23: Jibit/Vandar/Sadad payout API, source settlement account config, the
|
||
async callback that flips `submitted → paid/failed`, batch caps/minimums. Keep whole-batch/single-row
|
||
failure + retry semantics (already tested at `Tests/.../Payouts/PayoutHandlerTests.cs:211`).
|
||
- **Effort:** L · **Risk:** high (irreversible transfers — the UNIQUE link + ledger-exists guard are the
|
||
backstops, and they're in place) · **Deps:** 5.3 (real ownership check) before real runs; 4.1 for the
|
||
weekly trigger.
|
||
|
||
### 6.4 Remove `IPaymentCaptureSimulator` (the registry's own end-state)
|
||
|
||
- **Why:** the b9 temporary conversion trigger was supposed to be removed once b10's real capture shipped
|
||
(registry row 37, "make it real" step 3). b10 shipped; the seam and registration remain. Harmless as a
|
||
test trigger, but it's a second way to mint a booking without a payment row — undesirable once real money
|
||
exists.
|
||
- **Current state:** registered at `CrossCutting/ServiceConfiguration/ServiceCollectionExtension.cs:52`;
|
||
`Seams:PaymentCapture` options `SeamOptions.cs:142`.
|
||
- **Change:** move the simulator into the test host (b9's tests are its only legitimate consumer) and drop
|
||
the production registration; or gate the registration to Development/Testing.
|
||
- **Effort:** S · **Risk:** low (b9 Convert tests must keep a path) · **Deps:** none.
|
||
|
||
### 6.5 Real Moadian submission + reconciliation (`IMoadianClient`)
|
||
|
||
- **Why:** e-invoicing to سامانه مودیان is a legal obligation; today every invoice stays
|
||
`moadian_status = pending` forever (mock leaves it pending; **no reconciliation job or endpoint exists** —
|
||
the only `ApplyMoadianResult` caller is issue-time, `Features/Invoices/Commands/IssueInvoice/IssueInvoiceCommand.Handler.cs:72`).
|
||
- **Current state:** `CrossCutting/Seams/MockMoadianClient.cs:15-25`; registration `:65`;
|
||
`Seams:Moadian:ForceRegistered` default false.
|
||
- **Change:** enrollment (memory/economic code + signing cert), real `SubmitAsync`, and the
|
||
`pending → submitted → registered/failed` reconciliation job (register it under 4.1). The invoice
|
||
number sequence and VAT-on-commission math are already correct and tested.
|
||
- **Effort:** M–L (enrollment dominates) · **Risk:** medium · **Deps:** 4.1 (scheduler) for the poll.
|
||
|
||
### 6.6 Decide the partner-center settlement rail (currently: resolver without money)
|
||
|
||
- **Why:** b15 resolves merchant-of-record per booking and stores each center's encrypted
|
||
`settlement_iban` + `commission_rate`, but no money path pays a center or applies its rate — the b15
|
||
report itself lists the settlement rail as a follow-up.
|
||
- **Current state:** resolver + invoice wiring real
|
||
(`Persistence/Repositories/PartnerCenterRepository.cs:118`, `IssueInvoiceCommand.Handler.cs:63`);
|
||
`IBankTransferProvider` consumed only by nurse payouts (`Controllers/V1/AdminPayoutsController.cs:44`);
|
||
follow-up noted at `dev/shared-working-context/reports/backend-phase-15-report.md:83`.
|
||
- **Change:** product decision first (does a merchant-of-record center receive the commission split at
|
||
launch, or is it bookkeeping-only?). If money moves: a center-settlement ledger account + payout command
|
||
reusing the b13 machinery.
|
||
- **Effort:** M–L (if built) · **Risk:** medium · **Deps:** product decision; 6.3.
|
||
|
||
---
|
||
|
||
## post-phase-7 — Observability, audit & ops hardening
|
||
|
||
### 7.1 Add tracing and consolidate the two metric stacks
|
||
|
||
- **Why:** OTel is metrics-only (no `WithTracing`, no OTLP exporter) — cross-service money flows (webhook →
|
||
confirm → ledger) can't be traced in production. Two overlapping Prometheus stacks run simultaneously
|
||
(OTel's `AddPrometheusExporter` + prometheus-net's `UseMetricServer`/`UseHttpMetrics`).
|
||
- **Current state:** `Monitoring/Configurations/OpenTelemetryConfigurations.cs:11-21`;
|
||
`PrometheusMetricsConfigurations.cs:11`; W3C activity format set but unexported
|
||
(`Program.cs:32`); Serilog already enriches with span ids (`LoggingConfiguration.cs:24`).
|
||
- **Change:** add `WithTracing` (AspNetCore + EF instrumentation) exporting OTLP; pick **one** metrics
|
||
stack; wire trace-id into the `ApiResult.requestId` for support correlation.
|
||
- **Effort:** S–M · **Risk:** low · **Deps:** an OTLP-capable collector (optional at MVP; Prometheus alone
|
||
is acceptable — see runtime-services.md).
|
||
|
||
### 7.2 Broaden health checks and split readiness/liveness
|
||
|
||
- **Why:** the single check is app-DB connectivity; the log DB, object-storage root, and (future)
|
||
Redis/PSP get no signal. A deploy can pass health while logging or uploads are broken.
|
||
- **Current state:** `Monitoring/Configurations/HealthCheckConfigurations.cs:17` (SQL Server only),
|
||
`/HealthCheck` endpoint `:28`, dead `currentUrl` variable `:20`.
|
||
- **Change:** add checks for `logDb`, object storage (write probe), Redis when 4.2 lands; tag checks and
|
||
expose `/healthz/live` (process) vs `/healthz/ready` (dependencies); remove the dead line.
|
||
- **Effort:** S · **Risk:** low · **Deps:** tracks new infra as it arrives.
|
||
|
||
### 7.3 Revisit production log levels and the notification-channel plan
|
||
|
||
- **Why:** deployed environments write **only Warning+** to the SQL sink — every Information-level audit
|
||
trail (logins, money operations context) is dropped in production while Development keeps it. Also note:
|
||
OTP codes are currently logged by design (`LoggingSmsSender`) — that must not survive 5.1.
|
||
- **Current state:** `CrossCutting/Logging/LoggingConfiguration.cs:40-53`; Elasticsearch sink referenced
|
||
but commented out (`:58-70`; package still pinned at `server/Directory.Packages.props:51`).
|
||
- **Change:** Information+ to the sink with table retention (or a file/OTLP sink), structured category
|
||
filters; delete the dead Elastic sink block + package (or revive it deliberately); verify no PII is
|
||
logged (the SMS mock's OTP log disappears with 5.1).
|
||
- **Effort:** S · **Risk:** low · **Deps:** none.
|
||
|
||
### 7.4 Audit-log growth & archival
|
||
|
||
- **Why:** `audit_logs` is append-only with no archival or retention (deferred since b1); 2.3 will grow it
|
||
faster. The notification purge job is the only retention job in the system.
|
||
- **Current state:** deferral recorded at `dev/phases/backend/backend-phase-1.md:124`; no purge/archive job
|
||
exists for `ops.AuditLogs` (only `NotificationRetentionHostedService`).
|
||
- **Change:** a retention/archival policy (cold table or export) as a 4.1 job; define legal retention for
|
||
money/verification audit rows first.
|
||
- **Effort:** S–M · **Risk:** low · **Deps:** 4.1.
|
||
|
||
### 7.5 Decide TicketMessage.Body encryption and the gRPC plugin's fate
|
||
|
||
- **Why (tickets):** ticket messages are the refund/dispute paper trail — users will type phone numbers,
|
||
addresses, and clinical details. `TicketMessage.Body` is plaintext with **no documented decision**,
|
||
unlike `BookingRequest.CustomerNotes` which carries an explicit "deliberately plaintext" comment
|
||
(`Domain/Entities/Booking/BookingRequest.cs:45`).
|
||
- **Why (gRPC):** the plugin duplicates the OTP/token flow only, forces the HTTP/2 posture (1.5), and
|
||
enables reflection unconditionally — cost without a consumer (the Next.js client is HTTP/JSON only).
|
||
- **Current state:** `Domain/Entities/Messaging/TicketMessage.cs:20`;
|
||
`Plugins/Baya.Web.Plugins.Grpc/GrpcPluginStartup.cs:14-23`, `Services/UserGrpcServices.cs:18-53`.
|
||
- **Change:** (a) either encrypt `Body` via the existing converter pattern (accepting the search/ops cost —
|
||
admin thread reads already decrypt per-row elsewhere) or add the explicit "deliberately plaintext"
|
||
decision comment + docs; recommend encrypting. (b) remove the gRPC plugin or disable reflection outside
|
||
Development and give it a dedicated HTTP/2 endpoint.
|
||
- **Effort:** S–M · **Risk:** low · **Deps:** 1.5 pairs with (b).
|
||
|
||
### 7.6 Keep the docs honest (registry + tracker + map)
|
||
|
||
- **Why:** the mocks-registry contains stale duplicate rows — the early block still says 🔴 "not built" for
|
||
`IDistributedLock`/`INurseSearch`/`IPaymentProvider`/`ISettlementSplitProvider`/`IWebhookVerifier`/
|
||
`IMoadianClient`/`ILicenseVerificationService` while later rows correct all seven (e.g. rows 15/16 vs
|
||
38/42). `IJobScheduler` is listed as a seam but no such interface exists. All 15 REQs read `Status: open`.
|
||
Stale instructions are worse than none (root CLAUDE.md rule 7).
|
||
- **Current state:** `dev/shared-working-context/reports/mocks-registry.md:15-19,32,36` (stale block) vs
|
||
`:38-50` (corrected rows); `dev/shared-working-context/frontend/requests/for-backend.md` (all open).
|
||
- **Change:** prune the stale registry block, rename the `IJobScheduler` row to "recurring jobs (hosted
|
||
services)", mark delivered/answered REQs, and note `IPaymentCaptureSimulator`'s intended removal (6.4).
|
||
- **Effort:** S · **Risk:** none · **Deps:** post-phase-3 outcomes.
|
||
|
||
---
|
||
|
||
## post-phase-8 — Scale & later (explicitly not MVP)
|
||
|
||
- **8.1 Elasticsearch read backend + outbox feeder** — `SqlNurseSearch` is real and correct
|
||
(`Persistence/Services/Search/SqlNurseSearch.cs:18`); `Search:Backend` fails fast on any non-`sql` value
|
||
(`ServiceCollectionExtensions.cs:74-78`). Build `ElasticNurseSearch` + the outbox/CDC feeder per registry
|
||
rows 38/43 only when SQL search shows strain. **Effort:** L.
|
||
- **8.2 Analytics pipeline** — `IAnalyticsSink` writes `ops.SystemEvents` rows fire-and-forget
|
||
(`Persistence/Services/Analytics/AnalyticsSink.cs:15-35`); pipe to a warehouse/stream when product needs
|
||
it. **Effort:** M.
|
||
- **8.3 Holiday-calendar feed** — the table is manually maintained; a lunar-Hijri drift shifts payout dates
|
||
(`Persistence/Services/Holidays/HolidayCalendarService.cs:25-45`). A yearly ops checklist item is an
|
||
acceptable alternative to a feed. **Effort:** S.
|
||
- **8.4 Push/SMS notification channels** — `InAppNotificationDispatcher` silently drops non-InApp channels
|
||
(`Persistence/Services/Notifications/InAppNotificationDispatcher.cs:17`); add channel fan-out (SMS via
|
||
5.1's sender, FCM push) when the mobile/notification UX demands it. **Effort:** M.
|
||
- **8.5 Deferred product tables** — `organizations`, `organization_nurses`, `fraud_flags`,
|
||
`recurring_booking_schedules` (b15), `bnpl_settlement_entries` (b12), nurse availability slots (b5/b8),
|
||
customer national-ID KYC (b3), geo bulk import (b4) — all verified absent and all pure additive
|
||
migrations when product pulls them (evidence: `dev/phases/backend/backend-phase-15.md:333-336`,
|
||
`after-backend-phase-12.md:40`, `backend-phase-3.md:188`, `backend-phase-4.md:194`; model snapshot clean).
|
||
|
||
---
|
||
|
||
## Suggested sequencing
|
||
|
||
```
|
||
post-phase-1 (security) ──┬──► post-phase-2 (money correctness) ──► post-phase-6 (money rails)
|
||
│
|
||
post-phase-3 (contract batch — parallel, unblocks frontend f9–f15)
|
||
│
|
||
post-phase-4 (scheduler/Redis) ──► needed by 6.3/6.5 triggers
|
||
post-phase-5 (trust rails; 5.1 SMS is launch-critical, schedule early)
|
||
post-phase-7 (observability — start anytime, finish before launch)
|
||
post-phase-8 (later)
|
||
```
|
||
|
||
The two items that should not wait for their bucket: **1.1 (rotate the committed sa credentials — today)**
|
||
and **2.1 (the unreachable refund clearing — before any real BNPL/manual refund exists)**.
|