feat: MVP End-to-End CDC Data Extractor (Phases 1-8)

[plissb]

Summary

Complete MVP implementation of the SQL Server CDC Data Extractor — a Windows Service that extracts data from SQL Server via Change Data Capture, uploads to a downstream HTTP API as gzip CSV chunks, managed through a WPF desktop application (configurator wizard + management console) connected via Named Pipes IPC (JSON-RPC/StreamJsonRpc).

Phases Delivered

• Phase 1 — Setup: .NET 10 solution, 6 source + 5 test projects, Clean Architecture
• Phase 2 — Foundational: Domain model (Value Objects, Entities, Events), Contracts, State Store (Dapper)
• Phase 3 — US1 Initial Setup & Snapshot: 9-step wizard, CDC configuration, first SNAPSHOT batch, downstream upload
• Phase 4 — US2 Delta Extraction: Scheduled DELTA batches via Cronos, CDC change capture with _op/_lsn/_seqval/_ts, heartbeat, token refresh
• Phase 5 — US3 Management Console: 7 Manager screens (Dashboard, Runs, RunDetails, Tables, Diagnostics, Settings, Logs), live IPC log streaming
• Phase 6 — US4 CDC Gap Detection: LSN gap detection, RE_BOOTSTRAP flow, automatic re-snapshot recovery
• Phase 7 — US5 Manual Run Trigger: "Run Now" from Dashboard, single-instance lock, BatchTrigger.Manual
• Phase 8 — Polish: Global exception handling, Polly retry for SQL transient faults, error classification (table-level vs batch-level), Named Pipe ACL security, actionable error messages audit

Key Stats

• 109 tasks completed across 8 phases
• 171 tests passing (47 Domain + 50 Application + 28 App + 23 Infrastructure + 23 Service)
• 237 files added, ~22,600 lines of code
• 0 warnings, 0 errors on build

Tech Stack

C# / .NET 10, Dapper, Serilog, Polly, CsvHelper, Cronos, CommunityToolkit.Mvvm, StreamJsonRpc, xUnit + NSubstitute + FluentAssertions

Architecture

Clean Architecture: Domain → Application → Infrastructure; Service and App are entry points. DDD patterns: Value Objects, Entities, Domain Events, Repository pattern.

Test plan

• dotnet build src/CdcExtractor.slnx — 0 errors, 0 warnings
• dotnet test tests/CdcExtractor.slnx — 171 tests pass
• WPF app launches to wizard on first run
• Service starts in console mode (--console)
• AI code reviewers (Claude + Gemini) triggered on PR

🤖 Generated with Claude Code

[github-actions]

ERROR:

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:263040dcf2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Start heartbeat for snapshot batches

RunSnapshotBatchAsync creates a leased batch and immediately begins table extraction, but unlike RunDeltaBatchAsync it never starts _heartbeat. On large initial snapshots that run longer than the downstream inactivity TTL, the lease can expire mid-run and subsequent upload/finish calls will fail with 409 even though extraction itself is healthy; snapshot batches should start/stop heartbeat the same way delta batches do.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Handle lease conflicts as SinkUploadException

This conflict handler is effectively dead code because DownstreamClient.SendAsync converts HTTP 409 responses into SinkUploadException, not HttpRequestException. As a result, heartbeat lease conflicts fall into the generic catch and keep retrying instead of signaling LeaseConflictDetected and stopping promptly, which delays superseded-batch shutdown behavior.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Add transient retry policy to downstream HttpClient

The downstream client registration adds only TokenRefreshHandler; there is no resilience/retry handler for transient HTTP failures. Because DownstreamClient throws immediately on 429/5xx, a brief downstream hiccup fails the current extraction path instead of retrying, which materially reduces run reliability under normal transient outages.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Stream chunks instead of buffering all in memory

Chunking currently builds a List<ChunkResult> containing MemoryStream payloads and returns only after all rows are chunked, so uploads cannot begin until the entire table is buffered. For large snapshot/delta runs this can hold many compressed chunks simultaneously and cause excessive memory use or OOM; chunk production should be streamed so each chunk is uploaded and disposed immediately.

Useful? React with 👍 / 👎.

[github-actions]

ERROR:

[claude]

Code Review — MVP End-to-End CDC Data Extractor

Overall this is a well-structured, thorough MVP. Clean Architecture layers are largely respected, async/await usage is consistent throughout, DDD value objects (Lsn, TableIdentifier, SchemaHash) are correctly immutable with defensive copies, and error handling with actionable messages is a clear strength. The review below focuses on findings that should be addressed before production readiness.

---

🔴 Critical

1. DPAPI CurrentUser scope breaks cross-account token access

File:src/CdcExtractor.Infrastructure/Http/DpapiTokenStore.cs:62

varencryptedBytes=ProtectedData.Protect(plainBytes,null,DataProtectionScope.CurrentUser);

The WPF wizard (interactive user) writes the encrypted token file, and the Windows Service (service account) must read it. With CurrentUser scope, only the encrypting user can decrypt — the service account will always get a CryptographicException on Unprotect. This is a functional blocker in a real deployment.

Options:

• Use DataProtectionScope.LocalMachine (the encrypted file is then bound to the machine, not the user — weaker but appropriate for a service account scenario).
• Store the token in a location accessible to the service and protect it with NTFS ACLs restricted to the service account SID, then use LocalMachine scope.
• Have the wizard write the plain refresh token to a DPAPI-LocalMachine-protected location that only the service account can read via NTFS ACLs.

2. SQL injection via unescaped bracket identifier

File:src/CdcExtractor.Domain/ValueObjects/TableIdentifier.cs:13

publicstringQuotedFullName=> $"[{Schema}].[{Name}]";

This is used directly in a raw SQL command in CdcReader.ReadFullTableAsync:129:

command.CommandText=$"SELECT * FROM {table.QuotedFullName};";

If Schema or Name contains ], the bracket quoting breaks. While schema/name values typically originate from sys.schemas/sys.tables, table names with special characters are legal in SQL Server and injection via catalog values is a real attack surface if CDC is enabled on adversarially named tables.

Fix:

publicstringQuotedFullName=>
$"[{Schema.Replace("]","]]")}].[{Name.Replace("]","]]")}]";

---

🟠 High

3. DapperStateStore uses reflection to set private properties — domain encapsulation broken

File:src/CdcExtractor.Infrastructure/StateStore/DapperStateStore.cs:145

privatestaticvoidSetPrivateProperty<T>(TableStatetarget,stringpropertyName,Tvalue){varproperty=typeof(TableState).GetProperty(propertyName,BindingFlags.Public|BindingFlags.Instance)??thrownewInvalidOperationException(...);property.SetValue(target,value);}

This defeats the purpose of private set properties on the aggregate root. Renaming a property on TableState silently breaks the store at runtime (not compile time). Consider a static factory method or internal constructor on TableState that Infrastructure can call via [InternalsVisibleTo], or introduce a separate reconstitution DTO/factory that the repository owns.

4. TableState.CaptureInstance has a public setter

File:src/CdcExtractor.Domain/Entities/TableState.cs:18

publicstring?CaptureInstance{get;set;}

This is the only mutable property with a public setter on what is otherwise a well-guarded aggregate root. Add a SetCaptureInstance(string value) domain method with a guard, or make it init-only.

5. All chunks held in memory simultaneously

File:src/CdcExtractor.Application/Services/ChunkingService.cs:35, 129

varresults=newList<ChunkResult>();// ...results.Add(chunk);// chunk.Data is a MemoryStream// ...returnresults;// caller then iterates and uploads

All MemoryStream buffers for every chunk are allocated before upload begins. For a large table snapshot (hundreds of MB of compressed CSV), this multiplies peak heap usage. The existing IAsyncEnumerable<DataRow> streaming from CdcReader is immediately undermined by this buffering.

Recommendation: return IAsyncEnumerable<ChunkResult> from both ChunkSnapshotRowsAsync and ChunkCdcRowsAsync so callers can upload and dispose each chunk as it is produced, keeping memory usage proportional to one chunk at a time rather than the whole table.

6. Application layer references HttpRequestException — Clean Architecture violation

File:src/CdcExtractor.Application/Services/ExtractionOrchestrator.cs:376

privatestaticboolIsBatchLevelError(Exceptionex)=>exisHttpRequestException||// <-- infrastructure concern in Application layerIsLeaseConflict(ex)||(exisSinkUploadException{HttpStatusCode:>=500});

HttpRequestException is System.Net.Http, an infrastructure concern. The Application layer should be agnostic of transport. Consider a domain/application-layer marker exception (e.g., DownstreamUnavailableException) that DownstreamClient wraps into, so the orchestrator can catch that without taking a dependency on System.Net.Http.

7. HeartbeatWorker — unsynchronized access to _batchId/_leaseToken across threads

File:src/CdcExtractor.Service/Workers/HeartbeatWorker.cs:17-19, 96

privatestring?_batchId;privatestring?_leaseToken;// ...await_downstreamClient.HeartbeatAsync(_batchId!,_leaseToken!,ct)

_batchId and _leaseToken are written by StartHeartbeat/StopHeartbeat (called from the scheduler thread) and read concurrently by RunHeartbeatLoopAsync (running on a thread pool thread). No volatile, Interlocked, or lock is present. If StopHeartbeat sets these to null while the heartbeat loop is mid-read, the null-forgiving ! dereference throws NullReferenceException. Marking the fields volatile is a minimal fix.

---

🟡 Medium

8. SqlServerConfig.Password stored in plaintext

File:src/CdcExtractor.Contracts/Config/SqlServerConfig.cs:11

With AuthType = "SqlLogin", the cleartext password lands in appsettings.json on disk. At minimum, log a warning when SqlLogin is detected suggesting environment variables or a secrets manager. Consider accepting a DPAPI-encrypted form of the password and decrypting in SqlConnectionFactory.FromConfig.

9. Named pipe ACL grants access to all interactive users

File:src/CdcExtractor.Service/Ipc/IpcServer.cs:104

varinteractiveUsers=newSecurityIdentifier(WellKnownSidType.InteractiveSid,null);

InteractiveSid covers every user with an active desktop logon on the machine. On a terminal server or shared machine, any logged-in user can send arbitrary JSON-RPC commands (including triggerRun). A more restrictive ACL would allow only the service account SID plus a specific operator group SID from configuration.

10. Capture instance name not validated against the 100-char SQL Server limit

File:src/CdcExtractor.Infrastructure/SqlServer/CdcManager.cs:105

varcaptureInstance=$"{table.Schema}_{table.Name}";

sys.sp_cdc_enable_table enforces a 100-character maximum on @capture_instance. Add a guard with an actionable error message.

11. CsvChunkWriter appears to be dead code

Files:src/CdcExtractor.Infrastructure/Csv/CsvChunkWriter.cs, CdcRowMapper.cs

CsvChunkWriter (CsvHelper-based) is not registered in DI and is not called by ChunkingService, which writes CSV manually. Tests exist for it but it is unreachable in production. Either wire it up to replace the manual CSV writer in ChunkingService, or remove it to avoid confusion about which implementation is canonical.

12. ExtractorServiceRpc singleton shares one subscriber ID across all IPC clients

File:src/CdcExtractor.Service/Ipc/ExtractorServiceRpc.cs:22

privatereadonlystring_subscriberId=Guid.NewGuid().ToString("N");

ExtractorServiceRpc is registered as a singleton (AddSingleton), so all pipe connections share the same instance and the same _subscriberId. A second WPF window's subscribe call overwrites the first; unsubscribing either window silences logs for all clients. ExtractorServiceRpc should be scoped per connection, not a singleton.

13. CdcChangeRow._ts records extraction time, not CDC change commit time

File:src/CdcExtractor.Infrastructure/SqlServer/CdcReader.cs:180

returnnewCdcChangeRow(operation,lsn,seqVal,DateTimeOffset.UtcNow,values);

_ts reflects extraction wall-clock time, not the actual change commit time. sys.fn_cdc_map_lsn_to_time(@start_lsn) can provide the approximate commit time. At minimum, document that _ts reflects extraction time.

---

🔵 Low / Nits

14. Log file path is hardcoded

File:src/CdcExtractor.Service/Program.cs:109 — should be driven from configuration or Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData).

15. Missing negative-path tests for Lsn.Parse

File:tests/CdcExtractor.Domain.Tests/ValueObjects/LsnTests.cs — add cases for wrong-length hex, non-hex characters, and null/empty input.

16. GetValue() on SequentialAccess reader materialises large column values

File:src/CdcExtractor.Infrastructure/SqlServer/CdcReader.cs:173 — not urgent, but typed streaming reads (GetChars/GetBytes) would reduce allocations for very wide tables.

---

Test Coverage Notes

• 171 unit tests with NSubstitute mocks are a solid foundation.
• No integration tests against a real SQL Server instance. At least one SQL Server integration test (Docker container or LocalDB) would catch real-world issues: LSN increment edge cases, sp_cdc_enable_table failures, and the MERGE upsert logic.
• DpapiTokenStore, IpcServer ACL, and SqlConnectionFactory.FromConfig (SqlLogin credential path) have no test coverage.
• SchedulerWorkerTests should include a test that verifies the semaphore correctly skips a second trigger when one is in flight.

---

Summary Table

#	Severity	File	Issue
1	🔴 Critical	DpapiTokenStore.cs:62	DPAPI CurrentUser prevents service from decrypting WPF-written tokens
2	🔴 Critical	TableIdentifier.cs:13	QuotedFullName doesn't escape ] — SQL injection risk in snapshot query
3	🟠 High	DapperStateStore.cs:145	Reflection-based property setting breaks compile-time safety
4	🟠 High	TableState.cs:18	CaptureInstance public setter breaks aggregate root encapsulation
5	🟠 High	ChunkingService.cs:35,129	All chunk MemoryStreams buffered simultaneously — memory spike on large tables
6	🟠 High	ExtractionOrchestrator.cs:376	Application layer depends on HttpRequestException (Clean Architecture violation)
7	🟠 High	HeartbeatWorker.cs:17,96	Unsynchronized read/write of _batchId/_leaseToken across threads
8	🟡 Medium	SqlServerConfig.cs:11	SQL Login password stored in plaintext config
9	🟡 Medium	IpcServer.cs:104	Pipe ACL allows all interactive users — overly broad
10	🟡 Medium	CdcManager.cs:105	Capture instance name not validated for 100-char SQL Server limit
11	🟡 Medium	CsvChunkWriter.cs	Dead code — not wired to DI or used in production paths
12	🟡 Medium	ExtractorServiceRpc.cs:22	Singleton service shares one log subscriber ID across all IPC clients
13	🟡 Medium	CdcReader.cs:180	_ts records extraction time, not CDC change commit time
14	🔵 Low	Program.cs:109	Log file path hardcoded
15	🔵 Low	LsnTests.cs	Missing negative-path tests for Lsn.Parse
16	🔵 Low	CdcReader.cs:173	GetValue() on SequentialAccess reader materialises large column values

The two critical items (#1, #2) and the chunk memory issue (#5) should be resolved before production. The Clean Architecture violation (#6) and thread-safety issue (#7) should be addressed before the next feature phase.

🤖 Reviewed with Claude Code

[github-actions]

ERROR:

[github-actions]

Gemini Code Review

Привет! Отличная работа над этим PR. Это очень большой объем работы, охватывающий множество фаз MVP, и архитектура выглядит очень продуманной. Я впечатлен тщательным подходом к DDD, Clean Architecture, TDD и вниманием к деталям, таким как CancellationToken и IAsyncEnumerable.

Вот мои выводы по коду, основанные на заданных критериях:

Gemini Code Review

🔴 Критично (Critical)

1. Целевая платформа .NET и версия языка C#:
   • Проблема: В файле Directory.Build.props указаны <TargetFramework>net10.0</TargetFramework> и <LangVersion>13</LangVersion>. На момент текущих стабильных релизов .NET 8.0 является последней LTS-версией, использующей C# 12. .NET 10.0 и C# 13 являются будущими, невыпущенными версиями. Это приведет к ошибкам сборки или непредсказуемому поведению при использовании текущих SDK.
   • Конфликт: Файлы CLAUDE.md и constitution.md правильно указывают .NET 8 (LTS).
   • Рекомендация: Необходимо устранить это несоответствие. Измените <TargetFramework> на net8.0 и <LangVersion> на 12 в Directory.Build.props, чтобы соответствовать стабильной LTS-версии.

🟡 Рекомендация (Recommendation)

1. ConfigureAwait(false) в сервисах проекта App:
   • Проблема: В то время как в большинстве слоев ConfigureAwait(false) используется последовательно, некоторые вызовы await Task.Delay в ViewModels (например, BootstrapRunViewModel, ConnectSqlViewModel, DownstreamAuthViewModel, ReviewApplyViewModel) и await в ConfigService и IpcClient в проекте CdcExtractor.App не используют ConfigureAwait(false).
   • Обоснование: Хотя это приложение WPF (UI-контекст), и ConfigureAwait(true) (по умолчанию) обычно желателен для UI-потока, ConfigService и IpcClient являются сервисными компонентами. Если они когда-либо будут вызываться из не-UI контекста или если их await будет блокировать UI-поток, это может привести к проблемам. Для согласованности и лучшей практики в общих/сервисных компонентах рекомендуется использовать ConfigureAwait(false).
2. Polly для обработки временных HTTP-ошибок:
   • Проблема: Контракт downstream-api-client.md явно указывает на использование Polly для обработки временных HTTP-ошибок (429, 500, 503). Однако текущая реализация DownstreamClient.SendAsync просто выбрасывает SinkUploadException для этих статусов, а ExtractionOrchestrator затем обрабатывает их. Логика повторных попыток не инкапсулирована в DownstreamClient, как подразумевается контрактом.
   • Рекомендация: Примените политику повторных попыток Polly непосредственно в DownstreamClient (например, используя IHttpClientFactory с AddTransientHttpErrorPolicy), чтобы соответствовать спецификации контракта и улучшить инкапсуляцию логики устойчивости.
3. Секреты в логах (Редактирование):
   • Проблема: Принцип IV конституции проекта ("Observability & Logging") гласит: "Строки подключения, токены, пароли НЕ ДОЛЖНЫ появляться в логах. Маскируйте или редактируйте." Хотя используется структурированное логирование Serilog, явная логика редактирования для всех потенциально конфиденциальных данных (например, если сообщение об исключении случайно содержит пароль) не продемонстрирована в конфигурации Serilog.
   • Рекомендация: Рассмотрите возможность добавления явных правил редактирования в конфигурацию Serilog (например, с помощью Serilog.Enrichers.WithProperty или пользовательских преобразователей), чтобы гарантировать, что конфиденциальные данные никогда не попадают в логи, даже в сообщениях об исключениях.
4. XML-документация:
   • Проблема: Хотя многие публичные члены имеют XML-документацию, полный аудит для обеспечения того, чтобы все публичные члены во всех проектах имели исчерпывающую XML-документацию, был бы полезен для поддерживаемости и обнаруживаемости кода.
   • Рекомендация: Проведите аудит и добавьте недостающую XML-документацию для всех публичных классов, методов и свойств.
5. Язык обзора Gemini:
   • Проблема: Рабочий процесс gemini-review.yml использует промпт на русском языке.
   • Рекомендация: Если основным языком проекта является английский, промпт следует изменить на английский для согласованности и лучшего понимания командой. Если русский является намеченным языком для этого конкретного AI-ревьюера, это должно быть явно указано в CLAUDE.md проекта или аналогичной документации.

🟢 Хорошо (Good)

1. Чистая архитектура и DDD: Отличное соблюдение принципов Чистой архитектуры (проект Domain не имеет зависимостей, Infrastructure реализует интерфейсы Domain) и паттернов DDD (Value Objects, Entities, Aggregates, Repositories).
2. Соответствие TDD: Сильная приверженность разработке через тестирование, с обширными наборами тестов и явным порядком задач в tasks.md.
3. Использование CancellationToken: Последовательное и правильное распространение и использование CancellationToken в асинхронных операциях, обеспечивающее корректное завершение работы.
4. Потоковая обработка и эффективность памяти: Эффективное использование IAsyncEnumerable и разбиения на чанки для обработки больших наборов данных, предотвращающее чрезмерное потребление памяти.
5. Polly для временных SQL-ошибок: Хорошо реализованный конвейер устойчивости Polly для обработки временных ошибок SQL Server.
6. Лучшие практики Windows Service: Правильное использование BackgroundService, глобальных обработчиков исключений, Path.Combine и интеграции с Event Log.
7. Безопасность Named Pipe ACL: Надежная конфигурация безопасности для именованных каналов, ограничивающая доступ авторизованным учетным записям.
8. Параметризованные SQL-запросы и квотирование идентификаторов: Последовательное использование параметризованных запросов и правильное квотирование SQL-идентификаторов, снижающее риски SQL-инъекций.
9. Структурированное логирование и информативные сообщения об ошибках: Комплексное структурированное логирование с Serilog и сильный акцент на информативные сообщения об ошибках с диагностическим контекстом и подсказками по устранению.
10. Рабочий процесс разработки с использованием AI (speckit): Введение команд speckit и обновление constitution.md демонстрируют сложный подход к разработке с использованием AI, что является значительным плюсом.

В целом, это очень качественный и хорошо структурированный код. Устранение критической проблемы с целевой платформой и рассмотрение рекомендаций еще больше улучшит проект.

Удачи!