Architecture review: synthesis and roadmap (Oct 2026)¶
This is the merged outcome of the October 2026 whole-repo review. Raw findings, file:line evidence and the reviewer appendices live in architecture-review-2026-10.md. That file's "Verification results" table (V1–V19) is the evidence base for everything marked verified here.
1. Verdict¶
SyRF is a capable product carried by one maintainer and a large agent workforce: about 1,250 PRs in two months. The codebase has three properties that set the agenda:
- Breadth beyond what one maintainer can keep in their head. About 500K lines of C# and 160K of TS.
- Seven deployables share one database and one domain library: a distributed monolith.
- Six state-management idioms are in use in the web app.
- 19K lines of workflow YAML.
- Correctness depends on conventions that nothing enforces.
- Lamar's "last registration wins" ordering decides which implementations are used.
- A shared, mutable 2-second aggregate cache.
- Fire-and-forget domain events.
- Hand-maintained dependency maps that disagree with each other.
- Lint and strict typecheck rules that CI never runs.
- Safety nets that look present but fail open.
- The only required check passes when skipped.
- Promotion proceeds after a test failure.
- Liveness probes include external dependencies.
- Feature-flag overrides are silently reverted.
The new code (bulk PDF, identity, statistics, pdf-agent) is careful and defensive. Most defects sit in older domain and API code and in the cross-cutting infrastructure. The highest-leverage work is to make the existing rules enforceable and to remove surface area, not to add features.
2. Act now: security and integrity (in flight)¶
| # | Issue | Status |
|---|---|---|
| V4 | Swagger UI serves the Auth0 client secret (live on production) | Fix PR #3966 parked as a draft. Owner decision 2026-10-03: Auth0 is left as it is; the Swagger change is owned by the OpenIddict migration, staging first — handover issue #3992. |
| V1 | Invitation token ignored: any user can take a project's lone pending invitation | Merged #3967 (2026-10-03); closed #2729. Owner decision 2026-10-03: keep the email-association flow; no token-bearer acceptance. |
| V2 | Change password without the current password | Merged #3968 (2026-10-03) |
| V12 | Project admin can take ownership | #3969 paused (WIP commit) — duplicates #3964 from the integrated-review plan; owner to pick one |
| V3 | One client can exhaust the global password budget and lock out everyone | Merged #3970 (2026-10-03) |
| — | No production promotion without explicit approval (owner rule 2026-10-03) | cluster-gitops#1555 CODEOWNERS + code-owner review required + token approvals off; 500+ stale production PRs closed; rolling draft PR: #3993; open caveat: camarades-argo-cd ruleset bypass |
| V10/V11 | Promotion after test failure; required check fails open | Merged #3971 (2026-10-03). Owner decision 2026-10-03: test-dotnet-integration blocks production promotion only; staging unchanged. |
| V17 | Email-verification codes can be brute-forced (enables invitation hijack) | Issue: attempt limits plus the rate limiter (below) |
| V16 | Anonymous email relay; no rate limiting anywhere | Issue: built-in AddRateLimiter on anonymous and credential endpoints |
| — | 170 open Dependabot alerts (2 critical, 53 high) | Issue: triage |
3. Architecture weaknesses, ranked¶
- Implicit, order-dependent composition (verified, V8).
- Lamar convention scans plus last-wins registration already produce a live bug: runtime flags are reset on every resolution.
- Hosts must satisfy the dependencies of dark features they never call.
- Direction: explicit per-feature
AddXxx()modules, a startup test asserting key singleton identities, then migration to built-in DI (keyed services) over time. - Persistence model unsound under concurrency (V5, V13, plus reviewer B5–B8). The pieces:
- a singleton "unit of work";
- a shared mutable aggregate cache (only 3 of about 99 load-mutate-save paths are isolated);
- the version bumped in memory before the write;
- upsert saves that resurrect deleted documents;
- direct
$setwrites that don't bump the version; - fire-and-forget domain events.
Direction: await events, then a transactional outbox; a non-upsert save for existing aggregates; then a scoped identity map in place of RepositoryCache.
3. Distributed monolith with unclear ownership.
- The API and PM both write pmProject, pmStudy and the statistics collections.
- Message contracts reference the domain.
- PM correctness depends on a single replica with Recreate.
- Project is a 1,725-line hot document holding the state of 5 job kinds.
Direction: declare one writer per collection, move job state out of Project, and create PM.Contracts (primitive records). Accept the modular monolith explicitly and simplify the service story to match.
4. Messaging resilience.
- No global retry or outbox; only about 12 of 31 consumers retry.
- Saga partitioning is set to 1.
- Strategic: open-source MassTransit v8 support ends December 2026.
5. ProjectStatistics scale. About 56% of PM Core, 36 collections and about 25 flags, still dark. Decide: activate or freeze. Either way, move it to its own assembly and retire its flags together.
6. Unenforced quality gates.
- Web: production builds are non-strict, and no lint runs in CI.
- .NET: 14K build warnings with TreatWarningsAsErrors=false, and the culture analyzers are muted.
- No central package management, and no global.json.
7. Delivery complexity.
- Four drifting dependency maps (the Lambda never rebuilds on library changes).
- A 4.5K-line pr-preview.yml.
- Promotion copies staging config wholesale.
- The Lambda release step never works.
8. Operational fragility.
- Liveness and readiness are inverted (V9).
- Change-stream health never recovers, and one subscriber exception kills the shared stream.
- Identity sits on every API request (uncached introspection).
9. Frontend consistency.
- Six state idioms.
- God files: stage-review.component.ts (2.2K), project-detail.effects.ts (1.9K), the AF2 store (3K).
- 7 import cycles.
- Wrong flattening operators in effects, and the SignalR reconnect losing its subscriptions (verified).
10. Legacy surface still shipped.
- The Auth0 paths (API and web).
- The v0/v1 schema branches.
- Dead aggregates and contracts.
- Several observability vendors in parallel (Elastic APM, Sentry, OpenTelemetry, LogRocket).
4. Roadmap¶
Phases are ordered by value against risk. Each item becomes an issue (see §6).
Phase 0: stop the bleeding (now; small PRs)¶
- The six fix PRs above.
- Rotate the Swagger secret.
- Rate limiting plus attempt limits for email codes, signup, contact forms and recovery mail.
- Dependabot critical and high triage.
- Await domain events and log handler failures.
- Fix the health-probe split (live = process only; ready = dependencies).
- Fix the Lamar scan (explicit registrations), with a singleton-identity test.
- SignalR: re-subscribe groups after an automatic reconnect.
- Web effects:
addSearch$dispatch;- data export
mergeMap; - the project-navigation race.
- Small API bugs:
Forbid(string);- the private keywords leak;
- the PageSize bound;
RetrieveInvitation500;- the DataExport unsubscribe;
- API/Quartz exit codes.
Phase 1: make the rules enforceable (weeks)¶
- CI:
- Web:
ng lintplus a stricttsc --noEmitin CI. - Central Package Management,
global.jsonand NuGet audit as errors. - Turn on the culture analyzers.
- One service registry generated from the csproj graph, feeding change detection, Dockerfiles and path filters. This fixes the stale Lambda.
- A release gate for integration tests. Make the integration lane reliable first, then require it.
- Messaging:
- Global MassTransit retry, redelivery and outbox.
- A STJ round-trip test for every contract.
- Records for value objects and messages (fixes V6/V7).
- Persistence:
- Non-upsert saves for existing aggregates.
$incofAudit.Versionon every direct write, enforced by an architecture test.
Phase 2: reduce surface area (1–2 months)¶
- Retire:
- Auth0, after the cutover plus 28 days;
- the legacy authorization mode;
- the v0/v1 schema (one migration);
- dead code and packages;
- the duplicate AutoMapper scanners.
- Mapping: move AutoMapper to Mapperly, which also removes the licensing exposure, and move I/O out of value resolvers.
- Observability: consolidate on OpenTelemetry plus one error tracker, and decide whether to keep LogRocket (GDPR).
- Web state:
- Converge on NgRx global entities plus SignalStore for feature-local state.
- Retire RxState and the empty ComponentStores.
- Break
core→ feature imports. - Workflows: split
pr-preview.yml, use composite actions, and move inline bash into tested scripts.
Phase 3: structural (quarter)¶
- Module boundaries:
PM.Contracts;- one writer per collection;
- job state in its own collections;
- ProjectStatistics in its own assembly (after the activate/freeze decision).
- Persistence: a scoped identity map replacing
RepositoryCache. - DI: move off Lamar to built-in DI.
- MassTransit: carry out the decision taken in §5.
5. Decisions needed (ADR candidates)¶
- MassTransit after v8: decided. v9 licensing was ruled out (2026-10-03), and Wolverine was chosen with a single coordinated switch-over after extensive isolated testing (2026-10-04). See ADR-021: bridge on 8.5.11, production switch targeted for Q3 2027.
- ProjectStatistics: activate to production on a date, or freeze and isolate.
- Service boundaries: formalise a modular monolith with single-writer ownership, or invest in real service autonomy.
- Gating: require the integration lane once it is stable; decide whether a human review is required (self-approve today).
- Mapping and DI direction: Mapperly and built-in DI as the target.
6. Issue breakdown¶
Created 2026-10-03 from §4:
- #3972 security: rate-limit anonymous and credential endpoints; cap email-verification attempts
- #3973 fix(pm): await domain event handlers and surface failures
- #3974 fix(infra): split liveness from readiness; recover change-stream health
- #3975 fix(api): runtime feature-flag provider re-created per resolve, resetting overrides
- #3976 fix(web): re-subscribe SignalR groups after automatic reconnect
- #3977 fix(web): search-import actions never dispatched, second export dropped, project navigation race
- #3978 fix(api): small confirmed API defects from architecture review
- #3979 fix(pm): study library session filter hard-codes 2 reviewers
- #3980 fix(pm): ValueObject equality ignores base fields; agreement threshold lost in serialization
- #3981 ci(web): run ng lint and strict typecheck in CI
- #3982 build(.NET): central package management, global.json, NuGet audit
- #3983 ci: generate the service dependency registry from the csproj graph
- #3984 messaging: global MassTransit retry/redelivery, outbox, contract round-trip tests
- #3985 persistence: non-upsert saves for existing aggregates; bump version on direct writes
- #3986 ADR: MassTransit after v8 (open-source support ends Dec 2026)
- #3987 ADR: ProjectStatistics activate or freeze, and isolate into its own assembly
- #3988 Epic: reduce surface area (Auth0, legacy authz, v0/v1 schema, dead code, AutoMapper, observability)
- #3989 Epic: web state-management convergence and import cycles
- #3990 Epic: simplify CI workflows
- Existing: #806 (S3 signature rate limit), #2177 (Dependabot alerts epic: 170 open alerts at review time)
7. Follow-up plans¶
Each plan is a Draft for Chris to approve. Each has a scope table, acceptance criteria per PR, a feature-flag decision, coordination notes and its own decisions list. Nothing starts until Chris answers those decisions. Every production rollout is a separate Chris-approved step.
| Plan | Covers | First decisions for Chris |
|---|---|---|
| Frontend bug-fix plan | #3976, #3977, web findings B1–B17 | D2–D5 |
| State-management convergence plan | #3989, SignalStore, resources, realtime and routing | D1–D8 |
| Backend bug-fix plan | #3972–#3975, #3978–#3980, V14–V17, Identity specs ID-1…ID-7 for the migration session | D1 limiter placement, D2 rollout modes, D6 events via the outbox, D7 fix ValueObject |
| Persistence and messaging plan | #3984, #3985, durable domain events | D1 gate #2934 on non-upsert saves, D3 in-house outbox, D7 read-only syrftest dry runs |
| ADR-021: messaging after MassTransit v8 | #3986 (v9 ruled out; Wolverine single switch-over chosen) | D0 approve the 8.5.11 bridge, D1 host switch on main, D2 risk window, D4 production maintenance window |
| Build and CI plan | #3982, #3983, #3990, delivery risks from appendix F | D2 clear production's copied deploymentId, D3 purge stale cluster-gitops branches, D10 CA2007 |
| ProjectStatistics decision brief | #3987 (the owner session reviews it first) | D1 activate the proven slice on a date, D3 gate dates |
| Surface-area reduction plan | #3988 | D2 observability target, D3 LogRocket, D5 AutoMapper licence |
| System.Text.Json migration plan | API-wide Newtonsoft → STJ (REST, both hubs, PM host); prerequisite of realtime hub v2 (Chris, 2026-10-04) | D1 startup switches, D2 custom converters, D3 parity bar, D5 JSON Patch formatter |
| Realtime server plan | Change streams → SignalR: replace Rx.NET with in-box BackgroundService + Channel readers, a subscription registry and a dispatcher; hub contract v2 |
D1 replace Rx, D2 hybrid trigger, D4 no backplane, D5 retire the full-stats push |
| Realtime client plan | Browser-side SignalR rewrite on signals and the state plan; supersedes most of state-plan §5b | D1 single client flag, D3 no cross-tab sharing, D4 stateful reconnect, D6 contract typing |