diff --git a/CURRENT_ROADMAP.md b/CURRENT_ROADMAP.md index e013fd02..3630986c 100644 --- a/CURRENT_ROADMAP.md +++ b/CURRENT_ROADMAP.md @@ -2,14 +2,14 @@ **최종 갱신:** 2026-08-08 (VS 번호 재배정 — 아래 "알려진 문서 정합성 문제" 1번 참조. 2026-08-07 갱신 내용은 실제 코드/테스트를 직접 확인한 결과였고 이번 갱신은 그 위에 번호 충돌만 정정한 것입니다.) -**상태 요약:** VS-27(감사 추적), VS-10(매도 결정), VS-28(거래 실행), VS-29(포트폴리오 대사) 백엔드 구현 + 테스트 완료. VS-26(승인 워크플로우, 구 VS-03)은 DEBT-017(중복 구현) 미해결로 **BLOCKED**. Phase 1 Shadow Run(Gate 5a, 252+ 거래일 검증)은 **아직 시작되지 않음** (과거 "RUNNING" 기록은 허위였음이 이미 문서로 정정됨). 상세 항목별 상태는 `docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv` 참조. +**상태 요약:** VS-27(감사 추적), VS-10(매도 결정), VS-28(거래 실행), VS-29(포트폴리오 대사) 백엔드 구현 + 테스트 완료. VS-26(승인 워크플로우, 구 VS-03)은 DEBT-017(중복 구현) 아키텍트 결정이 2026-08-08에 내려지고 실행되었으나(죽은 구현 삭제, 유일 구현에 통합 테스트 신규 작성), 그 테스트를 실 PostgreSQL로 검증하지 못해 여전히 **BLOCKED**. Phase 1 Shadow Run(Gate 5a, 252+ 거래일 검증)은 **아직 시작되지 않음** (과거 "RUNNING" 기록은 허위였음이 이미 문서로 정정됨). 상세 항목별 상태는 `docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv` 참조. --- ## ⚠️ 알려진 문서 정합성 문제 (DECISION_REQUIRED) 1. **VS 번호 체계 충돌 — 2026-08-08 부분 해결:** `WBS_MASTER.csv`(원 계획)와 `WBS_PROGRESS_TRACKER.csv`(실행 트래커) 사이의 VS-03/VS-04/VS-12/VS-14 충돌은 트래커 쪽 4개 슬라이스(승인워크플로우/감사추적/거래실행/포트폴리오대사)를 VS-26/27/28/29로 재번호 부여하여 해결했습니다. 근거: `docs/DECISIONS/ADR-WBS-001-slice-renumbering.md`. - - **이 과정에서 새로 발견한, 아직 미해결인 문제 (TECH_DEBT_REGISTER.md DEBT-017):** VS-26(구 VS-03) 승인워크플로우는 **동일 기능의 중복 구현이 두 벌** 존재합니다 — `src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/`(20/20 테스트 통과하지만 전 엔드포인트가 `[DontRegister]`로 비활성화되어 실제로는 호출 불가능한 죽은 코드)와 `Features/ApprovalWorkflow/`(`Program.cs`에 실제 등록되어 살아있지만 전용 테스트가 없음). 즉 **테스트된 코드는 죽어있고, 살아있는 코드는 테스트되지 않았습니다.** 아키텍트 결정(둘 중 하나를 정식으로 선택하고 나머지 삭제) 전까지 이 슬라이스에 프런트엔드를 붙이지 마세요. WBS_PROGRESS_TRACKER.csv의 AEG-VS-26-01 행 상태를 `BLOCKED`로 정정했습니다. + - **2026-08-08 후속 갱신 (TECH_DEBT_REGISTER.md DEBT-017 해결):** 죽은 구현(`src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/`, `[DontRegister]`)과 그 전용 테스트 파일을 삭제했습니다. 살아있는 `Features/ApprovalWorkflow/`가 이제 유일한 구현이며, 동일 시나리오(생성/승인/활성화 역할 검증, maker≠checker 분리, 증거 첨부, `DateOnly` 라운드트립)를 검증하는 Handler+Sql+실DB 통합 테스트를 새로 작성했습니다. 포팅 과정에서 살아있는 구현의 `Sql.cs`에도 죽은 코드에 있던 것과 동일한 Dapper `DateOnly` 바인딩 버그가 있음을 발견해 동일한 방식으로 수정했습니다. 다만 **이 세션에서도 실 PostgreSQL에 연결할 수 없어(127.0.0.1:5432 연결 거부, SSH 터널 미개통) 새 통합 테스트 8건은 하나도 실행 검증되지 않았습니다** — 순수 Policy 테스트 10건만 통과 확인. 그래서 AEG-VS-26-01은 여전히 `BLOCKED`입니다. 이번 정리 과정에서 두 가지 잔여 결함도 발견했습니다(이번 세션이 만든 결함 아님, 기존부터 있었음): `GET /approvals/{id}` 엔드포인트가 없어 승인 후 증거(evidence)를 HTTP로 조회할 방법이 없고, Draft→Proposed 전환을 호출하는 Handler/Endpoint가 어디에도 없어 실제로는 승인 API가 끝까지 도달 불가능한 상태입니다 — TECH_DEBT_REGISTER.md DEBT-025/DEBT-026으로 신규 등록했습니다. - **또 다른 발견 — 미추적 작업:** `src/KArtSell.Host/Features/MarketData/VS03_*.cs`, `Features/Portfolio/VS04_*.cs`/`VS05_*.cs`/`VS08_*.cs`는 실제 구현되고 테스트도 있는(commits `2bc2b1e`, `32b49a4`, `14c5e4f`, `2eee44d`) **세 번째** VS-03/04/05/08 사용례(Market Data Ingestion Dashboard, Portfolio Rebalance, Risk Metrics, Dashboard)인데, `WBS_PROGRESS_TRACKER.csv`에 전혀 기록되어 있지 않습니다. 다음 세션에서 이 작업을 검증(빌드/테스트 재현, 프런트엔드 존재 여부 확인)하고 트래커에 추가해야 합니다. 2. **DbUp 마이그레이션 테스트 DB 권한 문제:** `kartsell` DB 사용자가 `kartsell_migration_test` 데이터베이스의 소유자가 아니어서 `DbUpMigrationTests`(12건)가 로컬에서 실패합니다. 코드 문제가 아니라 DBA 조치(소유권 부여)가 필요합니다. 실행할 SQL 초안: `scripts/dba/grant-migration-test-db-ownership.sql`. 3. **frontend 빌드 산출물 재해시:** `dotnet build`를 실행할 때마다 `pnpm build`가 재실행되어 `wwwroot/assets/*` 해시 파일명이 바뀌고 git에 불필요한 변경이 쌓이는 구조적 문제가 있습니다 (아직 미해결). @@ -18,11 +18,11 @@ ## ✅ 완료 (Backend 구현 + 테스트, 2026-08-07 기준 검증됨) -### VS-26 (구 VS-03): 모델 승인 워크플로우 (Maker-Checker Governance) — 🔴 BLOCKED -- **위치:** `src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/` (죽은 코드, `[DontRegister]`) + `Features/ApprovalWorkflow/` (실제 등록됨, 테스트 없음) -- **테스트:** 20/20 PASS는 죽은 코드 쪽 결과입니다 — 실제로 호출되는 구현은 테스트되지 않았습니다. 자세한 내용은 TECH_DEBT_REGISTER.md DEBT-017 참조. -- **미완료:** 프런트엔드 UI 없음. DEBT-017 해결 전까지 착수 금지. -- **이전 세션에서 발견/수정한 결함 (여전히 유효, 죽은 코드 쪽 한정):** `ApprovalSql`이 Dapper로 `DateOnly` 파라미터를 바인딩하지 못해 승인 제안서 생성이 실 DB 환경에서 100% 실패하던 버그 — 병합 이후 실 DB로 한 번도 검증되지 않아 발견되지 않고 있었음 +### VS-26 (구 VS-03): 모델 승인 워크플로우 (Maker-Checker Governance) — 🟡 BLOCKED (DB 미검증) +- **위치:** `Features/ApprovalWorkflow/` — 2026-08-08부로 유일한 구현 (중복 구현 삭제 완료, DEBT-017 참조) +- **테스트:** 순수 `Policy` 단위 테스트 10/10 PASS(DB 불필요). Handler+Sql+실DB 통합 테스트 8건 신규 작성했으나 **이 세션에서 실 PostgreSQL에 연결하지 못해(127.0.0.1:5432 connection refused) 단 하나도 실행 검증되지 않음.** `dotnet build -c Release`는 0 경고/0 오류로 성공. +- **미완료:** 프런트엔드 UI 없음. 실DB 대상 테스트 실행 전까지 COMPLETED로 전환 금지. +- **이번 세션(2026-08-08)에서 발견/수정한 결함:** 살아있는 `Features/ApprovalWorkflow/Sql.cs`의 `InsertProposalAsync`에 죽은 `ApprovalSql`이 갖고 있던 것과 동일한 Dapper `DateOnly` 바인딩 버그가 있었음(수정 완료, DB로 미검증). 잔여 결함(수정하지 않고 README에만 기록): `GET /approvals/{id}` 엔드포인트 없음(증거 조회 불가), Draft→Proposed 전환이 어디에도 연결되어 있지 않음(승인 API가 실사용 시 끝까지 도달 불가능). ### VS-27 (구 VS-04): 불변 감사 추적 (Audit Trail / GDPR) - **위치:** `src/KArtSell.Modules.ModelOperations/Compliance/` diff --git a/TECH_DEBT_REGISTER.md b/TECH_DEBT_REGISTER.md index 68c7eafd..1ad7f75f 100644 --- a/TECH_DEBT_REGISTER.md +++ b/TECH_DEBT_REGISTER.md @@ -49,14 +49,16 @@ | DEBT-007 | Newtonsoft.Json override | Medium (2) | Medium (2) | Completed | Fixed in 88ea5ed: CA1848/CA1859 actual implementation. LoggerMessage + HashSet/Dictionary. | @claude | - | | DEBT-008 | Namespace consistency | Medium (2) | Low (1) | Accepted | All projects use RootNamespace=KArtSell.Aegis; AssemblyName retained per-project for DLL clarity. Trade-off accepted: DLL clarity > namespace alignment. No action. | @claude | PR 4d | | DEBT-016 | VS-02 mislabeled domain | Medium (2) | Low (1) | Backlog | Existing code `VS02_SyncSecurityMasterEndpoint.cs`, `VS02_SecurityMasterJobs.cs`, `VS02_SecurityMasterPolicy.cs` implement RBAC rule synchronization (access control), not financial security master data (listing/delisting/product structure). Dead code: endpoints disabled (DISABLED comment), schema `security_master.rules` table never migrated, never deployed. Correct domain documented in `docs/CURRENT/SLICE_SPECS/VS-02-SLICE_SPEC.md` (financial PIT). Removal decision deferred pending architect review (PR recommended). | @claude | docs/CURRENT/SLICE_SPECS/VS-02-SLICE_SPEC.md | -| DEBT-017 | Duplicate VS-03 Approval Workflow implementation | High (3) | Medium (2) | Backlog | Two independent, functionally-identical VS-03 maker-checker slices exist: `ApprovalWorkflow/` (Workstream H, own `ApprovalProposal`/`IClock`/`IOutbox` types) and `Features/ApprovalWorkflow/` (Workstream G, matches documented `Features//` convention). Both mapped the same routes (`/approvals`, `/approvals/{id}`, `/approvals/{id}/approve`), which crashed Host startup with a duplicate-route/missing-DI error the first time the app was actually booted (2026-08-07 — apparently never booted successfully before). Old set annotated `[DontRegister]` (FastEndpoints) 2026-08-07 to unblock boot; code and its test file (`ApprovalWorkflowTests.cs`) kept for now. Needs an architect decision: delete the old slice entirely (and its test) or intentionally keep both for a reason not yet documented. | @claude | Session 2026-08-07 (Phase 3 J/K/L hardening) | +| DEBT-017 | Duplicate VS-26 (formerly VS-03) Approval Workflow implementation | High (3) | Medium (2) | Completed (DB verification pending) | **Decision (2026-08-08):** `Features/ApprovalWorkflow/` (Workstream G) kept as canonical — it is the implementation actually wired into `Program.cs`/`FastEndpoints`. `src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/` (Workstream H, `[DontRegister]`'d dead code) and its dedicated test file (`tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs`, the old 20/20-passing suite that exercised only the dead code) were **deleted**. `ApprovalWorkflowPolicyTests.cs` already tested the kept implementation's pure `Policy` class and was extended (5→10 cases) rather than replaced. New Handler+Sql+real-Postgres integration tests were written at the same path the old dead-code tests occupied (`tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs`), covering create (Maker-role-gated), approve (Maker≠Checker separation of duties, Checker-role-gated, evidence attachment), activate (SRE-role-gated), list filtering, and an explicit `DateOnly EffectiveAt` round-trip. **Bug found and fixed while porting:** the kept implementation's `Sql.cs InsertProposalAsync` had the *exact same* Dapper-cannot-bind-`DateOnly` bug that was found and fixed in the deleted implementation's `ApprovalSql.cs` (commit `2ccf74c`) — i.e. the "tested" dead code had already been fixed for this, but the "live" code had not; it would have failed 100% of proposal-creation calls against a real database. Fixed identically (`::date` cast + `"yyyy-MM-dd"` string parameter). **Not fixed (out of scope, flagged as residual gaps in the slice's README):** no `GET /approvals/{id}` endpoint (evidence becomes unreachable via HTTP after approval), and no wired Draft→Proposed transition anywhere in the running app (`ApprovalWorkflowPolicy.CanProposeForReview` exists but no Handler/Endpoint calls it), and `approval_proposals` rows are mutated in place via `UPDATE` rather than appended as new PIT revisions (the table's schema only has `id` as `PRIMARY KEY`, so the deleted implementation's append-only INSERT approach would itself have violated that constraint on the second write — this is pre-existing, schema-level, and not a regression from this cleanup). **Verification status: `dotnet build -c Release` is clean (0 errors/warnings). `dotnet test --filter "FullyQualifiedName~ApprovalWorkflow" -c Release` was run 2026-08-08: 10/10 pure-`Policy` tests passed; all 8 new DB-backed integration tests failed with `Npgsql.NpgsqlException: Failed to connect to 127.0.0.1:5432` (connection refused) because no PostgreSQL was reachable in that session (no SSH tunnel to 178.104.200.7 open). None of the 8 have been confirmed to pass against a real database.** Do not mark this row fully verified until that run happens; see `docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv` row `AEG-VS-26-01`, kept `BLOCKED` for the same reason. | @claude | commit a2e742c (original dup.), this session's commit (resolution), `docs/DECISIONS/ADR-WBS-001-slice-renumbering.md` | | DEBT-018 | Outbox write not co-transactional with entity write | Medium (2) | Medium (2) | Backlog | `TradeExecution/TradeHandlers.cs` (`TradeOutboxPublisher`) and `PortfolioReconciliation/ReconcileTradeHandler.cs` open a second, separate connection/transaction to write the outbox message after the trade/holding write already committed on its own connection. A crash between the two leaves the entity updated but no outbox event emitted (silent, non-atomic). Proper fix: thread a shared `NpgsqlTransaction` through `TradeSql`/`ReconciliationSql` mutation methods so entity insert + outbox insert commit together, matching `DapperModelOperationRequestRepository`'s pattern. | @claude | Session 2026-08-07 (Phase 3 J/K/L hardening) | | DEBT-019 | Multiple duplicate cross-cutting abstractions (`IClock`, `IOutboxWriter`, `IKrxDataService`) | Medium (2) | Low (1) | Completed (partial) | Found and collapsed 3 separate cases where a slice reinvented an abstraction that already existed in `KArtSell.BuildingBlocks`: a second `IKrxDataService` (deleted, `ShadowRun.Services`), a second `IOutboxWriter`/`WriteAsync` in `ReconcileTradeHandler.cs` (removed, switched to `BuildingBlocks.Reliability.IOutboxWriter`), and a second `IClock`/`SystemClock` in `ApprovalWorkflow/ApprovalPolicy.cs` (removed, switched to `BuildingBlocks.Time.IClock`). Root cause: successive sessions implementing a slice without searching `BuildingBlocks` first. Recommend a pre-implementation checklist step ("does this abstraction already exist in BuildingBlocks?") for future slices. | @claude | Session 2026-08-07 (Phase 3 J/K/L hardening) | | DEBT-020 | `model_operations.models` and `compliance` schema never created by any migration | High (3) | Low (1) | Completed | `0036`/`0038` reference `model_operations.models(id)` via FK and `OpenDartDailyBatchJob.cs` queries it directly, but no migration ever ran `CREATE TABLE model_operations.models`; `0037` wrote to `compliance.*` tables without `CREATE SCHEMA compliance`. Any fresh database — including the actual deploy target (178.104.200.7), confirmed via a live failed SCP/DbMigrator deploy on 2026-08-07 — failed at migration `0036`/`0037`. Fixed via new `0035_model_operations_models.sql` (minimal: id/ticker/published_at/correlation_id/revision only — full Model Card schema is separate future work) and `CREATE SCHEMA IF NOT EXISTS compliance;` added to `0037`. Full chain 0000→0040 now verified fresh-install + idempotent re-run clean. | @claude | Session 2026-08-07 (deploy failure triage) | | DEBT-021 | Dapper never configured for snake_case↔PascalCase column mapping | High (3) | Low (1) | Completed | `Dapper.DefaultTypeMap.MatchNamesWithUnderscores` was never set anywhere in the codebase, so every `QueryAsync`/`QuerySingleOrDefaultAsync` result-mapping onto a snake_case DB column (e.g. `event_type` → `EventType`) silently returned null/default for that property instead of throwing — masking the bug in every Sql class across every module. Confirmed via `ApprovalWorkflowTests.InsertAndRetrieveProposal_RoundTrips` and `AuditTrailTests.InsertAuditEvent_CreatesImmutableRecord` both getting real rows back with null fields. Fixed centrally via a `[ModuleInitializer]` in `KArtSell.BuildingBlocks/Data/DapperBootstrap.cs` (runs once per process regardless of entry point — Host/DbMigrator/tests). | @claude | Session 2026-08-07 (deploy failure triage) | | DEBT-022 | jsonb/inet columns written as plain text without an explicit cast | Medium (2) | Low (1) | Completed (partial) | Dapper does not know to cast a `string` parameter to `jsonb`/`inet` for Npgsql; `AuditSql.InsertAuditEventAsync` (`details`, `ip_address`), `AuditSql.RedactAuditEventDetailsAsync` (duplicate `SET details =` assignment, separately fixed), `TradeSql.InsertTradeAsync`/`UpdateTradeStatusAsync` (`kis_response`), and `SellDecisionSql.InsertDecisionAsync` (`oos_performance`) all failed with `42804: column "x" is of type jsonb but expression is of type text` the first time they were run against a real schema. Fixed with explicit `::jsonb`/`::inet` casts at each call site (mechanical, no behavior change). `AuditSql`'s jsonb read-back (`Dictionary` from a jsonb column) also needed a raw-DTO + `JsonSerializer.Deserialize` mapping since Dapper has no built-in jsonb→Dictionary conversion either. **Not yet checked**: `PortfolioReconciliation`/`ApprovalWorkflow` Sql classes for the same pattern beyond what surfaced in this session's test runs — a full audit of jsonb/inet columns across all Sql classes is still open. | @claude | Session 2026-08-07 (deploy failure triage) | -| DEBT-023 | `ApprovalSql.InsertProposalAsync` fails on `DateOnly` parameter | Medium (2) | Low (1) | Backlog | `ApprovalWorkflowTests.InsertAndRetrieveProposal_RoundTrips` fails with `System.NotSupportedException: The member effectiveAt of type System.DateOnly cannot be used as a parameter value` — Dapper's `LookupDbType` doesn't recognize `DateOnly` without an explicit type map (`SqlMapper.AddTypeMap`/custom `TypeHandler`). Likely affects every other `DateOnly`-typed Dapper parameter in the codebase, not just this one; needs a similar centralized fix to DEBT-021 rather than a per-call-site patch. Discovered but not fixed in this session (scope cut to unblock the live deploy). | @claude | Session 2026-08-07 (deploy failure triage) | +| DEBT-023 | `ApprovalSql.InsertProposalAsync` fails on `DateOnly` parameter | Medium (2) | Low (1) | Completed | Stale entry, corrected 2026-08-08: this described `ApprovalSql.cs` under `src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/` — that per-call-site fix (`::date` cast + `"yyyy-MM-dd"` string parameter, not a centralized type handler) landed in commit `2ccf74c` but this row was never updated to reflect it. That whole file was then deleted as dead code while resolving DEBT-017 (2026-08-08); its surviving sibling, `Features/ApprovalWorkflow/Sql.cs`, was found to have the *same* unfixed bug independently and received the identical fix in that session — see DEBT-017. No centralized `DateOnly` type handler was added; this remains a per-call-site fix pattern, so any *other* `DateOnly`-typed Dapper INSERT elsewhere in the codebase should still be checked individually rather than assumed safe. | @claude | commit 2ccf74c; DEBT-017 (this session) | | DEBT-024 | New integration tests don't insert FK parent rows / one pure-logic test flakes under full-suite run | Low (1) | Low (1) | Backlog | `TradeExecutionTests` constructs `Trade` with a random `sellDecisionId` that was never inserted into `sell_decisions`, so every insert now correctly fails its FK constraint (`trades_sell_decision_id_fkey`) once the schema was actually complete (see DEBT-020) — test-only gap, not a production code defect; needs the tests updated to insert a parent `models`+`sell_decisions` row first. Separately, `SellPriorityRankerTests.CalculateScore_HardImpairment_ReturnsLowestScore` (pure logic, no DB) passed in isolation but returned 1000 instead of the expected 950 (age-boost not applied) when run as part of the full suite — not yet root-caused; may be test-order/parallelization state leakage rather than a `SellPriorityRanker` bug. Also, `DbUpMigrationTests.*` (pre-existing, unrelated to this session) fail locally with `42501: must be owner of database kartsell_migration_test` — a local Postgres role permission gap, not a code issue. | @claude | Session 2026-08-07 (deploy failure triage) | +| DEBT-025 | `Features/ApprovalWorkflow` has no `GET /approvals/{id}` endpoint | Medium (2) | Low (1) | Backlog | Discovered while resolving DEBT-017 (2026-08-08). The deleted duplicate implementation had a single-proposal fetch endpoint that included the attached `Evidence` list in its response; the kept, canonical implementation (`Features/ApprovalWorkflow/Endpoints.cs`) only has `POST /approvals`, `GET /approvals` (list, no evidence in the DTO), and `POST /approvals/{id}/approve`. Net effect: evidence attached during approval (PBO/DSR/OOS artifact links, the whole point of this slice per CLAUDE.md's "Activation gating") is currently unreachable via HTTP — only readable by querying `model_operations.approval_evidence` directly. Needs a `GetApprovalByIdEndpoint` + `ApprovalDetailResponse` (with `Evidence`) added to the slice; not done in the DEBT-017 session because it is new functionality, not a duplication cleanup, and out of that session's scope. | @claude | DEBT-017 (2026-08-08), `src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/README.md` | +| DEBT-026 | `Features/ApprovalWorkflow` has no wired Draft→Proposed transition | High (3) | Low (1) | Backlog | Discovered while resolving DEBT-017 (2026-08-08). `ApprovalWorkflowPolicy.CanProposeForReview` exists but no `Handler` or `Endpoint` in the kept implementation calls it, so nothing in the running application ever moves a proposal from `Draft` to `Proposed`. `ApproveApprovalHandler` requires `Proposed` and `ActivateModelHandler` requires `Approved`, so as shipped, a proposal created via `POST /approvals` cannot reach `Approved`/`Active` through the HTTP API at all — the maker-checker gate is not actually completable end-to-end today. Higher impact than DEBT-025 because it blocks the slice's core purpose, not just an ancillary read. Needs a `ProposeForReviewHandler` + `POST /approvals/{id}/propose` (or equivalent) endpoint. Not done in the DEBT-017 session (new functionality, out of that session's duplication-cleanup scope). | @claude | DEBT-017 (2026-08-08), `src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/README.md` | --- diff --git a/docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv b/docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv index 239ec8c3..6cfb57e9 100644 --- a/docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv +++ b/docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv @@ -27,7 +27,7 @@ AEG-VS-00-07,S0,VS-00,회귀·관제·Runbook·Rollback 증거,COMPLETED,2026-08 AEG-X-009,S1,Cross,Source catalog 고도화,COMPLETED,2026-08-07,"docs/CURRENT/CATALOGS/source-catalog.md; docs/CURRENT/AEG-X-009_AUTOMATION_PROPOSAL.md; contracts/data/source-approval.v1.proposed.json; docs/DECISIONS/ADR-DATA-001.md; db/migrations/0033_source_approval_contract.sql; db/migrations/0034_dataset_manifest_freeze_contract.sql; db/migrations/0033_market_data_import_logs.sql; src/KArtSell.Modules.ModelOperations/Infrastructure/DapperApprovedModelContextReader.cs",Data Governance,"✅ Workstream D/E/F COMPLETED: source-catalog.md v2.0 (KRX/OpenDart/KIS consolidated), VS-02_DATA_GOVERNANCE_POLICY.md, VS-03/04 SLICE_SPECs. All 4 unknowns resolved. ✅ Workstream G (commit 136665c, 2026-08-07) also now COMPLETE: live KRX OpenAPI / OpenDart / KIS service integrations (P1-P3), daily scheduling + error classification + SLA tracking + LKG fallback (P4-P6), market_data schema with append-only import logs, correlation_id-based idempotent replay. ⚠️ Note: 0033 is used by two different, unrelated migrations across branches (source_approval_contract.sql vs market_data_import_logs.sql) — confirm actual applied migration number in the target DB's kartsell_schema_versions journal before assuming both landed as authored." AEG-VS-01-01,S1,VS-01,정책·범위·실패상태 계약 확정,COMPLETED,2026-08-07,docs/CURRENT/SLICE_SPECS/VS-01-SLICE_SPEC.md,PM/Architect,"✅ SLICE_SPEC produced: VS-01-SLICE_SPEC.md (identity/MFA/RBAC/maker-checker contract). Prerequisite AEG-X-001 + AEG-VS-00-02 already COMPLETED. Ready for security team review and schema implementation." AEG-VS-02-01,S1,VS-02,정책·범위·실패상태 계약 확정,COMPLETED,2026-08-07,"docs/CURRENT/SLICE_SPECS/VS-02-SLICE_SPEC.md; docs/CURRENT/VS-02_DATA_GOVERNANCE_POLICY.md",PM/Architect,"✅ COMPLETE: VS-02-SLICE_SPEC.md + governance policy. All 4 unknowns resolved (data source, import SLA, audit policy, schema versioning). Financial security master implementation ready for Phase 2." -AEG-VS-26-01,S2,VS-26,모델 승인 워크플로우 구현 (Maker-Checker Governance),BLOCKED,-,"docs/CURRENT/SLICE_SPECS/VS-26-SLICE_SPEC.md; src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ (ApprovalSql.cs, ApprovalPolicy.cs, [DontRegister]'d); src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/ (Handlers.cs, Sql.cs, Policy.cs — the one actually wired in Program.cs); db/migrations/0036_approval_workflow.sql; tests/KArtSell.Integration.Tests/ApprovalWorkflow/*; TECH_DEBT_REGISTER.md DEBT-017; commit a2e742c (Workstream H, PR #23, merged to main)",PM/BE Lead,"🔴 CORRECTED 2026-08-08 — this row previously said 'Backend implementation + tests complete, 20/20 tests PASS' and that claim is misleading: TECH_DEBT_REGISTER.md DEBT-017 records that TWO independent ApprovalWorkflow implementations exist with colliding routes. The one this row's evidence links to and the one with 20/20 passing tests (`ApprovalWorkflow/`, Workstream H) has all 4 endpoints annotated `[DontRegister]` and is NOT reachable via HTTP. The implementation actually registered in `Program.cs` and reachable at runtime is `Features/ApprovalWorkflow/` (Workstream G) — grep found no dedicated test file exercising it. Net effect: the maker-checker approval gate that is actually live has not been shown to work, and the one that has been shown to work is dead code. Do not build frontend UI against either implementation, and do not re-mark this COMPLETED, until an architect decision (DEBT-017) picks the canonical implementation, deletes the other, and a test suite runs against whichever is kept. Renumbered from VS-03 to VS-26 on 2026-08-08 per docs/DECISIONS/ADR-WBS-001-slice-renumbering.md — VS-03 in WBS_MASTER.csv ('IngestMarketDataPIT') is a separate, unrelated, still-unimplemented slice and keeps its original number unchanged. Also note: src/KArtSell.Host/Features/MarketData/VS03_*.cs is a THIRD, unrelated, already-implemented-and-tested body of work also labeled 'VS-03' (Market Data Ingestion Dashboard, commits 2bc2b1e/32b49a4) that is entirely absent from this tracker — see CURRENT_ROADMAP.md for a note to investigate and add it." +AEG-VS-26-01,S2,VS-26,모델 승인 워크플로우 구현 (Maker-Checker Governance),BLOCKED,-,"docs/CURRENT/SLICE_SPECS/VS-26-SLICE_SPEC.md; src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/ (Handlers.cs, Sql.cs, Policy.cs, Endpoints.cs — sole implementation, wired in Program.cs); db/migrations/0036_approval_workflow.sql; tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs (new, 8 cases); tests/KArtSell.Integration.Tests/ApprovalWorkflowPolicyTests.cs (extended, 10 cases); TECH_DEBT_REGISTER.md DEBT-017/DEBT-023; commit a2e742c (original duplication, superseded)",PM/BE Lead,"🟡 UPDATED 2026-08-08 — DEBT-017 architect decision made and executed: the duplicate `src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/` (dead code, `[DontRegister]`'d, previously credited with the misleading '20/20 tests PASS') was DELETED along with its dedicated test file. `Features/ApprovalWorkflow/` (the implementation actually reachable over HTTP) is now the sole implementation and gained new Handler+Sql+real-Postgres integration tests covering create/approve/activate role gating, maker≠checker separation of duties, evidence attachment, and an explicit `DateOnly EffectiveAt` round-trip. While porting: found `Features/ApprovalWorkflow/Sql.cs InsertProposalAsync` had the same Dapper `DateOnly`-binding bug already fixed in the deleted implementation (commit 2ccf74c) — fixed identically here (`::date` cast + string parameter). Two residual (pre-existing, not introduced by this cleanup) gaps documented in the slice README rather than fixed: no `GET /approvals/{id}` endpoint, and no wired Draft→Proposed transition anywhere in the running app (approve/activate are therefore currently unreachable end-to-end via HTTP with real data). **Still BLOCKED, not COMPLETED, because no PostgreSQL was reachable in this session (127.0.0.1:5432 connection refused, no SSH tunnel open): `dotnet build -c Release` is clean, but `dotnet test --filter FullyQualifiedName~ApprovalWorkflow -c Release` shows only the 10 pure-Policy (no-DB) tests passing — all 8 new DB-backed integration tests fail with a connection error, unverified either way against a live database.** Do not mark COMPLETED until that run happens against a reachable Postgres and actually passes. Renumbered from VS-03 to VS-26 on 2026-08-08 per docs/DECISIONS/ADR-WBS-001-slice-renumbering.md — VS-03 in WBS_MASTER.csv ('IngestMarketDataPIT') remains a separate, unrelated, still-unimplemented slice. Also still open: `src/KArtSell.Host/Features/MarketData/VS03_*.cs` is a THIRD, unrelated, already-implemented-and-tested body of work also labeled 'VS-03' that is entirely absent from this tracker — see CURRENT_ROADMAP.md." AEG-VS-27-01,S2,VS-27,불변 감사 추적 구현 (Audit Trail / GDPR),COMPLETED,2026-08-07,"docs/CURRENT/SLICE_SPECS/VS-27-SLICE_SPEC.md; src/KArtSell.Modules.ModelOperations/Compliance/ (AuditSql.cs, GdprRetention.cs); tests/KArtSell.Integration.Tests/Compliance/AuditTrailTests.cs; commit 97444c9 (Workstream I, PR #24, merged to main)",PM/BE Lead,"✅ Backend implementation + tests complete: append-only audit_events table, GDPR retention tracking + redaction (actor_email→'', customer_id→''), PII fields (ip_address). 5/5 tests PASS run in isolation (2026-08-07). Also fixed same day (fix/dapper-underscore-mapping-and-build branch): ip_address (inet) and kis_response-style jsonb columns threw InvalidCastException when read through Dapper into a typed class; GdprRetention.RetentionEndsAt was declared DateTime against a DATE column, same failure mode; and a process-wide Dapper snake_case-mapping race condition (KArtSell.BuildingBlocks' [ModuleInitializer] only fires once that assembly loads — AuditSql doesn't reliably touch it) intermittently nulled out every column read from this table depending on unrelated test/host startup order. None of this had ever been exercised against a live database before. ⚠️ No frontend UI yet — in progress 2026-08-08. Renumbered from VS-04 to VS-27 on 2026-08-08 per docs/DECISIONS/ADR-WBS-001-slice-renumbering.md — VS-04 in WBS_MASTER.csv ('ApplyCorporateActions') was an unrelated, still-unimplemented slice and keeps its original number unchanged." AEG-VS-05-01,S3,VS-05,정책·범위·실패상태 계약 확정,BLOCKED,-,docs/CURRENT/AEG-VS-05-01_BLOCKER.md,PM/Architect,"2026-08-08: blocked before implementation. WBS defines IngestFundamentalsPIT (REQ-FND-001/DAT-05/MIG-FND-001/002/J04/UI-FND-01/T-FND-001), but existing VS-05 architecture/data contracts define unrelated Risk Metrics. Declared VS-02 dependency is not concretely evidenced beyond AEG-VS-02-01. See blocker record for required PM/Architect decision and Gate G1 evidence. No build/test/migration claimed." AEG-VS-06-01,S3,VS-06,정책·범위·실패상태 계약 확정,BLOCKED,-,docs/CURRENT/AEG-VS-06-01_BLOCKER.md,PM/Architect,"2026-08-08: blocked before implementation. WBS defines MaintainFeeTaxFxSchedule (REQ-COST-001/COST-01/02/MIG-COST-001/J04C/UI-COST-01/T-COST-001), but existing VS-06 architecture/data contracts define unrelated Stress Testing (STRESS-* / migration 0035). Declared VS-02 dependency is not concrete enough for Gate G1. See blocker record for required PM/Architect and Compliance/Owner decisions. No build/test/migration claimed." diff --git a/src/KArtSell.Host/KArtSell.Host.csproj b/src/KArtSell.Host/KArtSell.Host.csproj index 2e8d1492..e5b986a8 100644 --- a/src/KArtSell.Host/KArtSell.Host.csproj +++ b/src/KArtSell.Host/KArtSell.Host.csproj @@ -4,16 +4,18 @@ + + + + - - - - diff --git a/src/KArtSell.Host/Program.cs b/src/KArtSell.Host/Program.cs index bef836fa..61720362 100644 --- a/src/KArtSell.Host/Program.cs +++ b/src/KArtSell.Host/Program.cs @@ -208,7 +208,7 @@ builder.Services.AddScoped(); builder.Services.AddScoped(); -// Approval Workflow (VS-03, maker-checker) +// Approval Workflow (VS-26, formerly VS-03; maker-checker — see docs/DECISIONS/ADR-WBS-001-slice-renumbering.md) builder.Services.AddScoped(sp => new KArtSell.Modules.ModelOperations.Features.ApprovalWorkflow.ApprovalWorkflowSql(connectionString)); builder.Services.AddScoped(); builder.Services.AddScoped(); diff --git a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalEndpoints.cs b/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalEndpoints.cs deleted file mode 100644 index d2fe4e59..00000000 --- a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalEndpoints.cs +++ /dev/null @@ -1,177 +0,0 @@ -namespace KArtSell.Modules.ModelOperations.ApprovalWorkflow; - -using System; -using System.Collections.Generic; -using System.Threading.Tasks; -using FastEndpoints; -using KArtSell.BuildingBlocks.Time; - -/// -/// Superseded by Features.ApprovalWorkflow.CreateApprovalEndpoint (same route). Kept for -/// ApprovalWorkflowTests.cs coverage of ApprovalSql/ApprovalPolicy; excluded from route -/// registration to avoid a duplicate-route conflict at Host startup. See TECH_DEBT_REGISTER.md. -/// -[DontRegister] -public class CreateApprovalEndpoint : Endpoint -{ - private readonly CreateApprovalProposalHandler _handler; - - public CreateApprovalEndpoint(CreateApprovalProposalHandler handler) - { - _handler = handler; - } - - public override void Configure() - { - Post("/approvals"); - AllowAnonymous(); - } - - public override async Task HandleAsync(CreateApprovalProposalRequest req, CancellationToken ct) - { - var userEmail = User?.FindFirst("email")?.Value ?? "system@kartsell.local"; - var userRole = User?.FindFirst("role")?.Value; - - var response = await _handler.Handle(req, userEmail, userRole); - await Send.CreatedAtAsync(new { id = response.Id }, response, cancellation: ct); - } -} - -/// Superseded by Features.ApprovalWorkflow (same route). See CreateApprovalEndpoint remarks. -[DontRegister] -public class ListApprovalsEndpoint : Endpoint> -{ - private readonly ApprovalSql _sql; - private readonly IClock _clock; - - public ListApprovalsEndpoint(ApprovalSql sql, IClock clock) - { - _sql = sql; - _clock = clock; - } - - public override void Configure() - { - Get("/approvals"); - AllowAnonymous(); - } - - public override async Task HandleAsync(EmptyRequest req, CancellationToken ct) - { - var status = Query("status"); - var cutoff = _clock.UtcNow; - - List proposals; - - if (!string.IsNullOrEmpty(status)) - { - proposals = await _sql.GetProposalsByStatusAsync(status, cutoff); - } - else - { - proposals = await _sql.GetProposalsByStatusAsync("Proposed", cutoff); - } - - var responses = proposals.ConvertAll(p => new ApprovalProposalResponse - { - Id = p.Id, - ModelId = p.ModelId, - Status = p.Status.ToString(), - CreatedBy = p.CreatedBy, - CreatedAt = p.CreatedAt, - Justification = p.Justification, - EffectiveAt = p.EffectiveAt, - ApprovedBy = p.ApprovedBy, - ApprovedAt = p.ApprovedAt, - ApprovalNotes = p.ApprovalNotes - }); - - await Send.OkAsync(responses, ct); - } -} - -/// Superseded by Features.ApprovalWorkflow (same route). See CreateApprovalEndpoint remarks. -[DontRegister] -public class GetApprovalEndpoint : Endpoint -{ - private readonly ApprovalSql _sql; - private readonly IClock _clock; - - public GetApprovalEndpoint(ApprovalSql sql, IClock clock) - { - _sql = sql; - _clock = clock; - } - - public override void Configure() - { - Get("/approvals/{id}"); - AllowAnonymous(); - } - - public override async Task HandleAsync(EmptyRequest req, CancellationToken ct) - { - var id = Route("id"); - var cutoff = _clock.UtcNow; - - var proposal = await _sql.GetProposalByIdAsync(id, cutoff); - if (proposal == null) - { - await Send.NotFoundAsync(ct); - return; - } - - var response = new ApprovalProposalResponse - { - Id = proposal.Id, - ModelId = proposal.ModelId, - Status = proposal.Status.ToString(), - CreatedBy = proposal.CreatedBy, - CreatedAt = proposal.CreatedAt, - Justification = proposal.Justification, - EffectiveAt = proposal.EffectiveAt, - ApprovedBy = proposal.ApprovedBy, - ApprovedAt = proposal.ApprovedAt, - ApprovalNotes = proposal.ApprovalNotes, - Evidence = proposal.Evidence.ConvertAll(e => new ApprovalEvidenceResponse - { - Id = e.Id, - EvidenceType = e.EvidenceType, - EvidenceUrl = e.EvidenceUrl, - ReviewerComment = e.ReviewerComment - }) - }; - - await Send.OkAsync(response, ct); - } -} - -/// Superseded by Features.ApprovalWorkflow (same route). See CreateApprovalEndpoint remarks. -[DontRegister] -public class ApproveApprovalEndpoint : Endpoint -{ - private readonly ApproveApprovalHandler _handler; - private readonly IClock _clock; - - public ApproveApprovalEndpoint(ApproveApprovalHandler handler, IClock clock) - { - _handler = handler; - _clock = clock; - } - - public override void Configure() - { - Post("/approvals/{id}/approve"); - AllowAnonymous(); - } - - public override async Task HandleAsync(ApproveApprovalRequest req, CancellationToken ct) - { - var id = Route("id"); - var checkerEmail = User?.FindFirst("email")?.Value ?? "system@kartsell.local"; - var cutoff = _clock.UtcNow; - - var response = await _handler.Handle(id, req, checkerEmail, cutoff); - await Send.OkAsync(response, ct); - } -} diff --git a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalHandlers.cs b/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalHandlers.cs deleted file mode 100644 index 97415497..00000000 --- a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalHandlers.cs +++ /dev/null @@ -1,205 +0,0 @@ -namespace KArtSell.Modules.ModelOperations.ApprovalWorkflow; - -using System; -using System.Collections.Generic; -using System.Threading.Tasks; -using KArtSell.BuildingBlocks; - -public class CreateApprovalProposalHandler -{ - private readonly ApprovalSql _sql; - private readonly ApprovalPolicy _policy; - private readonly IOutbox _outbox; - - public CreateApprovalProposalHandler(ApprovalSql sql, ApprovalPolicy policy, IOutbox outbox) - { - _sql = sql; - _policy = policy; - _outbox = outbox; - } - - public async Task Handle( - CreateApprovalProposalRequest request, - string userEmail, - string? userRole) - { - if (!_policy.CanCreateProposal(userEmail, userRole)) - throw new UnauthorizedAccessException("Only Makers can create approval proposals"); - - var proposal = _policy.CreateProposal( - request.ModelId, - userEmail, - request.Justification, - request.EffectiveAt); - - await _sql.InsertProposalAsync( - proposal.Id, - proposal.ModelId, - proposal.Status.ToString(), - proposal.CreatedBy, - proposal.Justification, - proposal.EffectiveAt, - proposal.PublishedAt, - proposal.CorrelationId); - - // Log event - var evt = _policy.CreateProposalEvent(proposal, "CREATED", userEmail); - await _sql.InsertEventAsync(evt.Id, evt.ApprovalProposalId, evt.EventType, evt.ActorEmail, evt.Details, evt.CorrelationId); - - // Emit Outbox event - await _outbox.PublishAsync("ApprovalProposalCreated", proposal.CorrelationId, new { proposal.Id, proposal.ModelId }); - - return MapToResponse(proposal); - } - - private ApprovalProposalResponse MapToResponse(ApprovalProposal proposal) - { - return new ApprovalProposalResponse - { - Id = proposal.Id, - ModelId = proposal.ModelId, - Status = proposal.Status.ToString(), - CreatedBy = proposal.CreatedBy, - CreatedAt = proposal.CreatedAt, - Justification = proposal.Justification, - EffectiveAt = proposal.EffectiveAt, - ApprovedBy = proposal.ApprovedBy, - ApprovedAt = proposal.ApprovedAt, - ApprovalNotes = proposal.ApprovalNotes, - Evidence = proposal.Evidence.ConvertAll(e => new ApprovalEvidenceResponse - { - Id = e.Id, - EvidenceType = e.EvidenceType, - EvidenceUrl = e.EvidenceUrl, - ReviewerComment = e.ReviewerComment - }) - }; - } -} - -public class ApproveApprovalHandler -{ - private readonly ApprovalSql _sql; - private readonly ApprovalPolicy _policy; - private readonly IOutbox _outbox; - - public ApproveApprovalHandler(ApprovalSql sql, ApprovalPolicy policy, IOutbox outbox) - { - _sql = sql; - _policy = policy; - _outbox = outbox; - } - - public async Task Handle( - Guid proposalId, - ApproveApprovalRequest request, - string checkerEmail, - DateTimeOffset cutoff) - { - var proposal = await _sql.GetProposalByIdAsync(proposalId, cutoff) - ?? throw new KeyNotFoundException("Approval proposal not found"); - - if (!_policy.CanApproveApproval(proposal, checkerEmail, proposal.CreatedBy)) - throw new UnauthorizedAccessException("Cannot approve: separation of duties violation or wrong status"); - - proposal = _policy.ApproveApproval(proposal, checkerEmail, request.ApprovalNotes, request.Evidence); - - // Update proposal - await _sql.UpdateProposalStatusAsync( - proposal.Id, - proposal.Status.ToString(), - checkerEmail, - request.ApprovalNotes, - proposal.PublishedAt); - - // Add evidence - foreach (var evidence in request.Evidence) - { - await _sql.InsertEvidenceAsync( - Guid.NewGuid(), - proposal.Id, - evidence.Type, - evidence.Url, - evidence.Comment, - proposal.CorrelationId); - } - - // Log event - var evt = _policy.CreateProposalEvent(proposal, "APPROVED", checkerEmail); - await _sql.InsertEventAsync(evt.Id, evt.ApprovalProposalId, evt.EventType, evt.ActorEmail, evt.Details, evt.CorrelationId); - - // Emit Outbox event - await _outbox.PublishAsync("ApprovalProposalApproved", proposal.CorrelationId, new { proposal.Id, checkerEmail }); - - return MapToResponse(proposal); - } - - private ApprovalProposalResponse MapToResponse(ApprovalProposal proposal) - { - return new ApprovalProposalResponse - { - Id = proposal.Id, - ModelId = proposal.ModelId, - Status = proposal.Status.ToString(), - CreatedBy = proposal.CreatedBy, - CreatedAt = proposal.CreatedAt, - Justification = proposal.Justification, - EffectiveAt = proposal.EffectiveAt, - ApprovedBy = proposal.ApprovedBy, - ApprovedAt = proposal.ApprovedAt, - ApprovalNotes = proposal.ApprovalNotes, - Evidence = proposal.Evidence.ConvertAll(e => new ApprovalEvidenceResponse - { - Id = e.Id, - EvidenceType = e.EvidenceType, - EvidenceUrl = e.EvidenceUrl, - ReviewerComment = e.ReviewerComment - }) - }; - } -} - -public class ActivateApprovalHandler -{ - private readonly ApprovalSql _sql; - private readonly ApprovalPolicy _policy; - private readonly IOutbox _outbox; - - public ActivateApprovalHandler(ApprovalSql sql, ApprovalPolicy policy, IOutbox outbox) - { - _sql = sql; - _policy = policy; - _outbox = outbox; - } - - public async Task Handle(Guid proposalId, string sreEmail, string? userRole, DateTimeOffset cutoff) - { - if (!_policy.CanActivateApproval(new ApprovalProposal { CreatedBy = string.Empty, Justification = string.Empty }, userRole ?? string.Empty)) - throw new UnauthorizedAccessException("Only SRE can activate approvals"); - - var proposal = await _sql.GetProposalByIdAsync(proposalId, cutoff) - ?? throw new KeyNotFoundException("Approval proposal not found"); - - proposal = _policy.ActivateApproval(proposal, sreEmail); - - // Update proposal status to ACTIVE - await _sql.UpdateProposalStatusAsync( - proposal.Id, - proposal.Status.ToString(), - sreEmail, - null, - proposal.PublishedAt); - - // Log event - var evt = _policy.CreateProposalEvent(proposal, "ACTIVATED", sreEmail); - await _sql.InsertEventAsync(evt.Id, evt.ApprovalProposalId, evt.EventType, evt.ActorEmail, evt.Details, evt.CorrelationId); - - // Emit Outbox event for model activation - await _outbox.PublishAsync("ApprovalProposalActivated", proposal.CorrelationId, new { proposal.Id, proposal.ModelId }); - } -} - -public interface IOutbox -{ - Task PublishAsync(string eventType, Guid correlationId, object data); -} diff --git a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalPolicy.cs b/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalPolicy.cs deleted file mode 100644 index 7353ab52..00000000 --- a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalPolicy.cs +++ /dev/null @@ -1,162 +0,0 @@ -namespace KArtSell.Modules.ModelOperations.ApprovalWorkflow; - -using System; -using System.Collections.Generic; -using System.Linq; -using KArtSell.BuildingBlocks.Time; - -public class ApprovalPolicy -{ - private readonly IClock _clock; - - public ApprovalPolicy(IClock clock) - { - _clock = clock; - } - - public bool CanCreateProposal(string userEmail, string? userRole) - { - return userRole is "Maker" or "Admin"; - } - - public bool CanProposeApproval(ApprovalProposal proposal, string userEmail) - { - if (proposal.Status != ApprovalStatus.Draft) - return false; - - return proposal.CreatedBy == userEmail; - } - - public bool CanApproveApproval(ApprovalProposal proposal, string checkerEmail, string makerEmail) - { - if (proposal.Status != ApprovalStatus.Proposed) - return false; - - if (checkerEmail == makerEmail) - return false; // Separation of duties: Maker cannot approve own proposal - - return true; - } - - public bool CanActivateApproval(ApprovalProposal proposal, string userRole) - { - if (proposal.Status != ApprovalStatus.Approved) - return false; - - return userRole is "SRE" or "Admin"; - } - - public ApprovalProposal CreateProposal( - Guid modelId, - string createdBy, - string justification, - DateOnly effectiveAt) - { - return new ApprovalProposal - { - Id = Guid.NewGuid(), - ModelId = modelId, - Status = ApprovalStatus.Draft, - CreatedBy = createdBy, - CreatedAt = _clock.UtcNow, - Justification = justification, - EffectiveAt = effectiveAt, - PublishedAt = _clock.UtcNow, - Revision = 1, - CorrelationId = Guid.NewGuid() - }; - } - - public ApprovalProposal ProposeApproval(ApprovalProposal proposal, string makerEmail) - { - if (!CanProposeApproval(proposal, makerEmail)) - throw new InvalidOperationException("Only the creator can propose their own approval"); - - proposal.Status = ApprovalStatus.Proposed; - proposal.ProposedAt = _clock.UtcNow; - proposal.Revision++; - proposal.PublishedAt = _clock.UtcNow; - - return proposal; - } - - public ApprovalProposal ApproveApproval( - ApprovalProposal proposal, - string checkerEmail, - string approvalNotes, - List evidence) - { - if (!CanApproveApproval(proposal, checkerEmail, proposal.CreatedBy)) - throw new InvalidOperationException("Checker cannot approve their own proposals"); - - proposal.Status = ApprovalStatus.Approved; - proposal.ApprovedBy = checkerEmail; - proposal.ApprovedAt = _clock.UtcNow; - proposal.ApprovalNotes = approvalNotes; - proposal.Revision++; - proposal.PublishedAt = _clock.UtcNow; - - // Add evidence - foreach (var evt in evidence) - { - proposal.Evidence.Add(new ApprovalEvidence - { - Id = Guid.NewGuid(), - ApprovalProposalId = proposal.Id, - EvidenceType = evt.Type, - EvidenceUrl = evt.Url, - ReviewerComment = evt.Comment, - PublishedAt = _clock.UtcNow, - CorrelationId = proposal.CorrelationId - }); - } - - return proposal; - } - - public ApprovalProposal ActivateApproval(ApprovalProposal proposal, string sreEmail) - { - if (!CanActivateApproval(proposal, "SRE")) - throw new InvalidOperationException("Only SRE can activate approved proposals"); - - proposal.Status = ApprovalStatus.Active; - proposal.ActivatedBy = sreEmail; - proposal.ActivatedAt = _clock.UtcNow; - proposal.Revision++; - proposal.PublishedAt = _clock.UtcNow; - - return proposal; - } - - public ApprovalProposal RejectApproval(ApprovalProposal proposal, string checkerEmail, string rejectionReason) - { - if (proposal.Status != ApprovalStatus.Proposed) - throw new InvalidOperationException("Only proposed approvals can be rejected"); - - proposal.Status = ApprovalStatus.Rejected; - proposal.ApprovalNotes = $"Rejected: {rejectionReason}"; - proposal.Revision++; - proposal.PublishedAt = _clock.UtcNow; - - return proposal; - } - - public ApprovalEvent CreateProposalEvent( - ApprovalProposal proposal, - string eventType, - string actorEmail, - Dictionary? details = null) - { - return new ApprovalEvent - { - Id = Guid.NewGuid(), - ApprovalProposalId = proposal.Id, - EventType = eventType, - ActorEmail = actorEmail, - EventAt = _clock.UtcNow, - Details = details, - PublishedAt = _clock.UtcNow, - CorrelationId = proposal.CorrelationId - }; - } -} diff --git a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalProposal.cs b/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalProposal.cs deleted file mode 100644 index e7fc49d9..00000000 --- a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalProposal.cs +++ /dev/null @@ -1,109 +0,0 @@ -namespace KArtSell.Modules.ModelOperations.ApprovalWorkflow; - -using System; -using System.Collections.Generic; - -public class ApprovalProposal -{ - public Guid Id { get; set; } - public Guid ModelId { get; set; } - public ApprovalStatus Status { get; set; } - public string CreatedBy { get; set; } = null!; - public DateTimeOffset CreatedAt { get; set; } - public string Justification { get; set; } = null!; - public DateOnly EffectiveAt { get; set; } - - public DateTimeOffset? ProposedAt { get; set; } - public string? ApprovedBy { get; set; } - public DateTimeOffset? ApprovedAt { get; set; } - public string? ApprovalNotes { get; set; } - - public string? ActivatedBy { get; set; } - public DateTimeOffset? ActivatedAt { get; set; } - - public DateTimeOffset PublishedAt { get; set; } - public int Revision { get; set; } - public Guid CorrelationId { get; set; } - - public List Evidence { get; set; } = []; - public List Events { get; set; } = []; - - public bool CanBeProposed => Status == ApprovalStatus.Draft && CreatedBy is not null; - public bool CanBeApproved => Status == ApprovalStatus.Proposed; - public bool CanBeActivated => Status == ApprovalStatus.Approved; -} - -public enum ApprovalStatus -{ - Draft, - Proposed, - Approved, - Active, - Rejected -} - -public class ApprovalEvidence -{ - public Guid Id { get; set; } - public Guid ApprovalProposalId { get; set; } - public string EvidenceType { get; set; } = null!; - public string EvidenceUrl { get; set; } = null!; - public string? ReviewerComment { get; set; } - public DateTimeOffset PublishedAt { get; set; } - public Guid CorrelationId { get; set; } -} - -public class ApprovalEvent -{ - public Guid Id { get; set; } - public Guid ApprovalProposalId { get; set; } - public string EventType { get; set; } = null!; - public string ActorEmail { get; set; } = null!; - public DateTimeOffset EventAt { get; set; } - public Dictionary? Details { get; set; } - public DateTimeOffset PublishedAt { get; set; } - public Guid CorrelationId { get; set; } -} - -public class CreateApprovalProposalRequest -{ - public Guid ModelId { get; set; } - public DateOnly EffectiveAt { get; set; } - public string Justification { get; set; } = null!; -} - -public class ApproveApprovalRequest -{ - public string ApprovalNotes { get; set; } = null!; - public List Evidence { get; set; } = []; -} - -public class EvidenceItem -{ - public string Type { get; set; } = null!; - public string Url { get; set; } = null!; - public string? Comment { get; set; } -} - -public class ApprovalProposalResponse -{ - public Guid Id { get; set; } - public Guid ModelId { get; set; } - public string Status { get; set; } = null!; - public string CreatedBy { get; set; } = null!; - public DateTimeOffset CreatedAt { get; set; } - public string Justification { get; set; } = null!; - public DateOnly EffectiveAt { get; set; } - public string? ApprovedBy { get; set; } - public DateTimeOffset? ApprovedAt { get; set; } - public string? ApprovalNotes { get; set; } - public List Evidence { get; set; } = []; -} - -public class ApprovalEvidenceResponse -{ - public Guid Id { get; set; } - public string EvidenceType { get; set; } = null!; - public string EvidenceUrl { get; set; } = null!; - public string? ReviewerComment { get; set; } -} diff --git a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalSql.cs b/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalSql.cs deleted file mode 100644 index 897d2008..00000000 --- a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ApprovalSql.cs +++ /dev/null @@ -1,216 +0,0 @@ -namespace KArtSell.Modules.ModelOperations.ApprovalWorkflow; - -using System; -using System.Collections.Generic; -using System.Data; -using System.Linq; -using System.Text.Json; -using System.Threading.Tasks; -using Dapper; -using KArtSell.BuildingBlocks.Time; -using Npgsql; - -public class ApprovalSql -{ - private readonly string _connectionString; - private readonly IClock _clock; - - public ApprovalSql(string connectionString, IClock clock) - { - _connectionString = connectionString; - _clock = clock; - } - - public async Task GetProposalByIdAsync(Guid id, DateTimeOffset cutoff) - { - using var conn = new NpgsqlConnection(_connectionString); - const string sql = """ - SELECT - id, model_id, status, created_by, created_at, justification, effective_at, - proposed_at, approved_by, approved_at, approval_notes, activated_by, activated_at, - published_at, revision, correlation_id - FROM model_operations.approval_proposals - WHERE id = @id - AND published_at <= @cutoff - ORDER BY published_at DESC - LIMIT 1 - """; - - var proposal = await conn.QueryFirstOrDefaultAsync(sql, new { id, cutoff }); - if (proposal == null) return null; - - return MapFromRaw(proposal); - } - - public async Task> GetProposalsByStatusAsync(string status, DateTimeOffset cutoff, int pageSize = 100) - { - using var conn = new NpgsqlConnection(_connectionString); - const string sql = """ - SELECT - id, model_id, status, created_by, created_at, justification, effective_at, - proposed_at, approved_by, approved_at, approval_notes, activated_by, activated_at, - published_at, revision, correlation_id - FROM model_operations.approval_proposals - WHERE status = @status - AND published_at <= @cutoff - ORDER BY created_at DESC - LIMIT @pageSize - """; - - var proposals = await conn.QueryAsync(sql, new { status, cutoff, pageSize }); - return proposals.Select(MapFromRaw).ToList(); - } - - public async Task InsertProposalAsync( - Guid id, Guid modelId, string status, string createdBy, string justification, - DateOnly effectiveAt, DateTimeOffset publishedAt, Guid correlationId) - { - using var conn = new NpgsqlConnection(_connectionString); - const string sql = """ - INSERT INTO model_operations.approval_proposals - (id, model_id, status, created_by, created_at, justification, effective_at, published_at, revision, correlation_id) - VALUES (@id, @modelId, @status, @createdBy, @createdAt, @justification, @effectiveAt, @publishedAt, 1, @correlationId) - """; - - await conn.ExecuteAsync(sql, new - { - id, - modelId, - status, - createdBy, - createdAt = _clock.UtcNow, - justification, - effectiveAt, - publishedAt, - correlationId - }); - } - - public async Task UpdateProposalStatusAsync(Guid id, string newStatus, string approvedBy, string? approvalNotes, DateTimeOffset publishedAt) - { - using var conn = new NpgsqlConnection(_connectionString); - const string sql = """ - INSERT INTO model_operations.approval_proposals - (id, model_id, status, created_by, created_at, justification, effective_at, - approved_by, approved_at, approval_notes, published_at, revision, correlation_id) - SELECT id, model_id, @newStatus, created_by, created_at, justification, effective_at, - @approvedBy, @approvedAt, @approvalNotes, @publishedAt, revision + 1, correlation_id - FROM model_operations.approval_proposals - WHERE id = @id - ORDER BY published_at DESC LIMIT 1 - """; - - await conn.ExecuteAsync(sql, new - { - id, - newStatus, - approvedBy, - approvedAt = _clock.UtcNow, - approvalNotes, - publishedAt - }); - } - - public async Task InsertEvidenceAsync(Guid id, Guid proposalId, string evidenceType, string evidenceUrl, string? comment, Guid correlationId) - { - using var conn = new NpgsqlConnection(_connectionString); - const string sql = """ - INSERT INTO model_operations.approval_evidence - (id, approval_proposal_id, evidence_type, evidence_url, reviewer_comment, published_at, correlation_id) - VALUES (@id, @proposalId, @evidenceType, @evidenceUrl, @comment, @publishedAt, @correlationId) - """; - - await conn.ExecuteAsync(sql, new - { - id, - proposalId, - evidenceType, - evidenceUrl, - comment, - publishedAt = _clock.UtcNow, - correlationId - }); - } - - public async Task InsertEventAsync(Guid id, Guid proposalId, string eventType, string actorEmail, Dictionary? details, Guid correlationId) - { - using var conn = new NpgsqlConnection(_connectionString); - const string sql = """ - INSERT INTO model_operations.approval_events - (id, approval_proposal_id, event_type, actor_email, event_at, details, published_at, correlation_id) - VALUES (@id, @proposalId, @eventType, @actorEmail, @eventAt, @details::jsonb, @publishedAt, @correlationId) - """; - - var detailsJson = details != null ? JsonSerializer.Serialize(details) : null; - - await conn.ExecuteAsync(sql, new - { - id, - proposalId, - eventType, - actorEmail, - eventAt = _clock.UtcNow, - details = detailsJson, - publishedAt = _clock.UtcNow, - correlationId - }); - } - - public async Task> GetEvidenceByProposalAsync(Guid proposalId, DateTimeOffset cutoff) - { - using var conn = new NpgsqlConnection(_connectionString); - const string sql = """ - SELECT id, approval_proposal_id, evidence_type, evidence_url, reviewer_comment, published_at, correlation_id - FROM model_operations.approval_evidence - WHERE approval_proposal_id = @proposalId - AND published_at <= @cutoff - ORDER BY published_at DESC - """; - - var results = await conn.QueryAsync(sql, new { proposalId, cutoff }); - return results.ToList(); - } - - private ApprovalProposal MapFromRaw(ApprovalProposalRaw raw) - { - return new ApprovalProposal - { - Id = raw.Id, - ModelId = raw.ModelId, - Status = Enum.Parse(raw.Status), - CreatedBy = raw.CreatedBy, - CreatedAt = raw.CreatedAt, - Justification = raw.Justification, - EffectiveAt = raw.EffectiveAt, - ProposedAt = raw.ProposedAt, - ApprovedBy = raw.ApprovedBy, - ApprovedAt = raw.ApprovedAt, - ApprovalNotes = raw.ApprovalNotes, - ActivatedBy = raw.ActivatedBy, - ActivatedAt = raw.ActivatedAt, - PublishedAt = raw.PublishedAt, - Revision = raw.Revision, - CorrelationId = raw.CorrelationId - }; - } - - private sealed class ApprovalProposalRaw - { - public Guid Id { get; set; } - public Guid ModelId { get; set; } - public string Status { get; set; } = null!; - public string CreatedBy { get; set; } = null!; - public DateTimeOffset CreatedAt { get; set; } - public string Justification { get; set; } = null!; - public DateOnly EffectiveAt { get; set; } - public DateTimeOffset? ProposedAt { get; set; } - public string? ApprovedBy { get; set; } - public DateTimeOffset? ApprovedAt { get; set; } - public string? ApprovalNotes { get; set; } - public string? ActivatedBy { get; set; } - public DateTimeOffset? ActivatedAt { get; set; } - public DateTimeOffset PublishedAt { get; set; } - public int Revision { get; set; } - public Guid CorrelationId { get; set; } - } -} diff --git a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/README.md b/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/README.md deleted file mode 100644 index 675fad2f..00000000 --- a/src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/README.md +++ /dev/null @@ -1,208 +0,0 @@ -# VS-26: Model Approval Workflow - -## Overview - -This vertical slice implements a maker-checker approval workflow for model activation. It enforces separation of duties, state machine transitions, and evidence linkage for regulatory compliance. - -**Status:** ✅ Ready for implementation -**Specification:** `docs/CURRENT/SLICE_SPECS/VS-26-SLICE_SPEC.md` - ---- - -## User Story - -As a platform lead/compliance officer, I want to enforce maker-checker approval workflow for model activation so that only reviewed, authorized models reach production (governance compliance). - ---- - -## Key Features - -### 1. Approval State Machine - -``` -DRAFT (Maker creates) - ↓ -PROPOSED (Maker submits to Checker) - ├→ APPROVED (Checker signs off with evidence) - │ ↓ - │ ACTIVE (SRE activates) - │ - └→ REJECTED (Checker rejects, revise to DRAFT) -``` - -### 2. Maker-Checker Separation of Duties - -- **Maker:** Can create and propose approval proposals (own proposals only) -- **Checker:** Can approve any proposal (must be different from Maker) -- **SRE:** Can activate approved proposals -- **System:** Logs all actions with actor identity and correlation_id - -### 3. Evidence Linkage - -- Store PBO/DSR/OOS artifact URLs during approval -- Checker annotates evidence interpretation -- Traceability: approval_id → evidence_links → S3 artifacts - -### 4. Immutable Audit Trail - -- All state transitions logged in `approval_events` table -- Correlation_id links related events -- PIT tracking via `published_at` + `revision` - ---- - -## Database Schema - -### approval_proposals -```sql -id, model_id, status, created_by, created_at, justification, effective_at, -proposed_at, approved_by, approved_at, approval_notes, activated_by, activated_at, -published_at, revision, correlation_id -``` - -### approval_evidence -```sql -id, approval_proposal_id, evidence_type, evidence_url, reviewer_comment, -published_at, correlation_id -``` - -### approval_events -```sql -id, approval_proposal_id, event_type, actor_email, event_at, details, -published_at, correlation_id -``` - ---- - -## API Endpoints - -### POST /approvals (Create Proposal) -**Role:** Maker -**Request:** -```json -{ - "modelId": "uuid", - "effectiveAt": "2026-09-15", - "justification": "Model passed OOS testing; PBO score 0.95" -} -``` -**Response (201):** -```json -{ - "id": "approval-uuid", - "modelId": "uuid", - "status": "Draft", - "createdBy": "maker@company.com", - "createdAt": "2026-08-07T10:00:00Z" -} -``` - -### GET /approvals (List Proposals) -**Query Params:** `status=Proposed&modelId=uuid` -**Response (200):** -```json -{ - "items": [ - { - "id": "approval-uuid", - "modelId": "uuid", - "status": "Proposed", - "createdBy": "maker@company.com", - "approvalNotes": null - } - ] -} -``` - -### GET /approvals/{id} (Get Single) -**Response (200):** -```json -{ - "id": "approval-uuid", - "modelId": "uuid", - "status": "Proposed", - "evidence": [ - { - "id": "evidence-uuid", - "evidenceType": "PBO_SCORE", - "evidenceUrl": "s3://evidence/pbo-0.95.json", - "reviewerComment": "Verified" - } - ] -} -``` - -### POST /approvals/{id}/approve (Checker Approval) -**Role:** Checker -**Request:** -```json -{ - "approvalNotes": "PBO verified, OOS metrics acceptable", - "evidence": [ - {"type": "PBO_SCORE", "url": "s3://evidence/pbo-0.95.json", "comment": "Verified"}, - {"type": "OOS_RETURN", "url": "s3://evidence/oos-returns.csv", "comment": "Acceptable"} - ] -} -``` -**Response (200):** -```json -{ - "id": "approval-uuid", - "status": "Approved", - "approvedBy": "checker@company.com", - "approvedAt": "2026-08-07T11:00:00Z" -} -``` - ---- - -## RBAC Enforcement - -| Role | Can Create | Can Approve | Can Activate | -|------|-----------|-----------|------------| -| Maker | ✅ (own) | ❌ | ❌ | -| Checker | ❌ | ✅ (others) | ❌ | -| SRE | ❌ | ❌ | ✅ | -| Admin | ✅ | ✅ | ✅ | - -**Separation of Duties:** Maker ≠ Checker (same user cannot approve own proposal) - ---- - -## Compliance & Governance - -- ✅ **Separation of Duties:** Enforced at Endpoint level -- ✅ **Evidence Linkage:** All evidence URLs traceable to artifacts -- ✅ **Immutable Audit Trail:** INSERT-only events table -- ✅ **Correlation Tracking:** CorrelationId links related events across slices -- ✅ **PIT Queries:** All reads include `WHERE published_at <= cutoff` - ---- - -## Related Specifications - -- **VS-00:** PIT envelope (published_at, correlation_id, revision) -- **VS-02:** Financial security master (governance foundation) -- **VS-27:** Audit trail (logs all approval events) -- **VS-10:** Sell decision (uses approved models) - ---- - -## Next Steps - -1. ✅ Schema migration (0036_approval_workflow.sql) -2. ✅ Domain entities (ApprovalProposal, ApprovalEvidence, ApprovalEvent) -3. ✅ Dapper queries (Sql.cs) -4. ✅ Business logic (ApprovalPolicy with state machine) -5. ✅ HTTP handlers (ApprovalHandlers.cs) -6. ✅ FastEndpoints (ApprovalEndpoints.cs) -7. ✅ Unit/Integration tests -8. ⏳ Merge to main (awaiting PR review) -9. ⏳ Integration with VS-27 (audit trail subscribers) -10. ⏳ Phase 2 implementation (after Phase 1 data available) - ---- - -**Co-Authored-By:** Claude Haiku 4.5 -**AGENTS.md v16.0:** 13/13 ✅ -**Compliance:** Spec-before-code, no new tech debt diff --git a/src/KArtSell.Modules.ModelOperations/Compliance/AuditSql.cs b/src/KArtSell.Modules.ModelOperations/Compliance/AuditSql.cs index bebdad94..f7d69e1c 100644 --- a/src/KArtSell.Modules.ModelOperations/Compliance/AuditSql.cs +++ b/src/KArtSell.Modules.ModelOperations/Compliance/AuditSql.cs @@ -135,7 +135,7 @@ public class AuditSql // Get paginated results var sql = $""" SELECT id, event_type, entity_type, entity_id, actor_email, actor_role, event_at, - result, error_message, details, evidence_links, ip_address, user_agent, + result, error_message, details, evidence_links, ip_address::text as ip_address, user_agent, published_at, correlation_id, revision FROM compliance.audit_events WHERE {whereClause} @@ -162,7 +162,7 @@ public class AuditSql { const string sql = """ SELECT id, event_type, entity_type, entity_id, actor_email, actor_role, event_at, - result, error_message, details, evidence_links, ip_address, user_agent, + result, error_message, details, evidence_links, ip_address::text as ip_address, user_agent, published_at, correlation_id, revision FROM compliance.audit_events WHERE id = @EventId diff --git a/src/KArtSell.Modules.ModelOperations/Compliance/GdprRetention.cs b/src/KArtSell.Modules.ModelOperations/Compliance/GdprRetention.cs index e2d7b884..0adccb22 100644 --- a/src/KArtSell.Modules.ModelOperations/Compliance/GdprRetention.cs +++ b/src/KArtSell.Modules.ModelOperations/Compliance/GdprRetention.cs @@ -10,7 +10,7 @@ public class GdprRetention public Guid EventId { get; set; } public Guid? CustomerId { get; set; } public string[]? DataCategories { get; set; } // PII, EMAIL, TRADING_HISTORY, PORTFOLIO_DATA, etc. - public DateTime RetentionEndsAt { get; set; } + public DateOnly RetentionEndsAt { get; set; } public required string PurgeStatus { get; set; } // PENDING, PURGED, EXCEPTION public DateTime? PurgedAt { get; set; } public string? ExceptionReason { get; set; } diff --git a/src/KArtSell.Modules.ModelOperations/DapperMappingBootstrap.cs b/src/KArtSell.Modules.ModelOperations/DapperMappingBootstrap.cs new file mode 100644 index 00000000..55943901 --- /dev/null +++ b/src/KArtSell.Modules.ModelOperations/DapperMappingBootstrap.cs @@ -0,0 +1,25 @@ +using System.Runtime.CompilerServices; + +namespace KArtSell.Modules.ModelOperations; + +/// +/// KArtSell.BuildingBlocks.Data.DapperBootstrap sets Dapper's snake_case-to-PascalCase column +/// mapping via its own [ModuleInitializer], but that only fires once that assembly is actually +/// loaded into the process. Several Sql classes in this module (e.g. AuditSql, TradeSql) only +/// have a `using` for a BuildingBlocks namespace without ever touching a type from it at +/// runtime, so under test isolation - or any host that queries this module before touching +/// BuildingBlocks - the load (and the mapping) can be skipped, silently nulling out every +/// snake_case column. Every Sql class in this assembly is defined here, so a module initializer +/// in this assembly is guaranteed to run before any of them are used, regardless of what else +/// has loaded. +/// +internal static class DapperMappingBootstrap +{ +#pragma warning disable CA2255 + [ModuleInitializer] +#pragma warning restore CA2255 + public static void Initialize() + { + Dapper.DefaultTypeMap.MatchNamesWithUnderscores = true; + } +} diff --git a/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/README.md b/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/README.md index 7d8d1ad7..ef1a4aba 100644 --- a/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/README.md +++ b/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/README.md @@ -1,9 +1,27 @@ -# VS-03: Model Approval Workflow (Maker-Checker Governance) +# VS-26 (formerly VS-03): Model Approval Workflow (Maker-Checker Governance) ## Overview This slice implements a maker-checker approval workflow for model activation with separation of duties and immutable audit trail. +**This is now the sole implementation of this slice.** A second, functionally-overlapping copy +(`src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/`, no `Features/` prefix) existed +alongside this one from 2026-08-07 to 2026-08-08; it was dead code (all 4 endpoints +`[DontRegister]`'d to avoid a duplicate-route crash at Host startup) despite having 20/20 +passing tests, while *this* implementation — the one actually wired into `Program.cs` and +reachable over HTTP — had no dedicated tests. See `TECH_DEBT_REGISTER.md` DEBT-017 and +`docs/DECISIONS/ADR-WBS-001-slice-renumbering.md` for the full history. The old implementation +and its test file were deleted on 2026-08-08 once this one gained equivalent integration-test +coverage (`tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs`). + +**Bug fixed 2026-08-08 (as part of DEBT-017):** `InsertProposalAsync` in `Sql.cs` passed +`EffectiveAt` (a `DateOnly`) directly as a Dapper parameter. The now-deleted implementation hit +the identical failure against a real database (commit `2ccf74c`) — Npgsql/Dapper in this +environment cannot bind a bare `DateOnly` value; it needs an explicit `::date` cast plus a +`"yyyy-MM-dd"` string parameter. That fix has been ported here. **This has not been re-verified +against a live PostgreSQL instance in this session** (none was reachable) — see "Test status" +below. + ## Architecture ### State Machine @@ -52,6 +70,16 @@ PROPOSED (maker submits) - POST /approvals (create proposal) - GET /approvals (list proposals) - POST /approvals/{id}/approve (approve proposal) + - ⚠️ **No `GET /approvals/{id}`.** The deleted duplicate implementation had a single-proposal + fetch endpoint that included the evidence list in its response; this implementation has no + equivalent, so evidence attached during approval is currently unreachable via HTTP (it can + only be read back through `ApprovalWorkflowSql` directly, e.g. in tests). Not fixed here — + out of scope for DEBT-017 (duplicate-implementation cleanup); tracked as **DEBT-025**. + - ⚠️ **No wired "submit for review" transition.** `ApprovalWorkflowPolicy.CanProposeForReview` + exists but no `Handler` or `Endpoint` calls it, so nothing in the running application ever + moves a proposal from `Draft` to `Proposed`. `ActivateModelHandler` and `ApproveApprovalHandler` + both require `Proposed`/`Approved` respectively, so as shipped a created proposal cannot + reach `Approved` through the HTTP API alone. Also not fixed here — tracked as **DEBT-026**. ## API Contracts @@ -178,15 +206,25 @@ CREATE TABLE model_operations.approval_events ( ## Tests -Unit tests cover: -- RBAC enforcement (Maker, Checker, SRE roles) -- Separation of duties (Checker ≠ Maker) -- State machine transitions -- RBAC violations +- `tests/KArtSell.Integration.Tests/ApprovalWorkflowPolicyTests.cs` — pure `ApprovalWorkflowPolicy` + unit tests (no DB): RBAC enforcement (Maker/Checker/SRE), separation of duties, valid/invalid + state transitions. Fast, deterministic. +- `tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs` — Handler + Sql + + real PostgreSQL integration tests: create (role-gated), approve (maker≠checker, evidence + attachment), activate (SRE-gated), `EffectiveAt` `DateOnly` round-trip through a real `date` + column, list filtering. + +**Test status as of 2026-08-08 (DEBT-017 resolution session): written but not run against a live +database.** No PostgreSQL was reachable at `127.0.0.1:5432` in that session (no SSH tunnel to +178.104.200.7 open). `dotnet build -c Release` was confirmed green; `dotnet test --filter +"FullyQualifiedName~ApprovalWorkflow"` was run and its actual outcome (pass, fail, or DB +connection error) is recorded in `TECH_DEBT_REGISTER.md` DEBT-017 and +`docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv` row `AEG-VS-26-01` — check those before treating +this slice as verified. Run tests: ```bash -dotnet test --filter "ApprovalWorkflowPolicyTests" +dotnet test --filter "FullyQualifiedName~ApprovalWorkflow" -c Release ``` ## AGENTS.md v16.0 Compliance @@ -194,25 +232,37 @@ dotnet test --filter "ApprovalWorkflowPolicyTests" - ✅ **SOLID:** Separate Endpoint/Handler/Policy/Sql per operation - ✅ **Complexity:** Each handler ≤200 lines - ✅ **Audit:** All state changes logged with correlation_id -- ✅ **Necessity:** Grounded in VS-03 SLICE_SPEC -- ✅ **Normalization:** 3NF schema, append-only events +- ✅ **Necessity:** Grounded in VS-26 (formerly VS-03) SLICE_SPEC +- ⚠️ **Normalization:** Writes mutate `approval_proposals` in place (`UPDATE ... revision = + revision + 1`) rather than appending a new revision row, because `id` is the sole `PRIMARY KEY` + in migration `0036_approval_workflow.sql` (no `(id, published_at)` composite key) — an + append-only INSERT would violate that constraint on the second write. This is a real deviation + from CLAUDE.md's "new state appended as new revision" rule; it is pre-existing (present before + this session) and schema-level, so fixing it is out of scope for DEBT-017. `GetProposalAsync` + correspondingly has no `published_at <= cutoff` PIT filter, since there is only ever one row. - ✅ **Simplicity:** State machine clearly visible - ✅ **Pattern:** Vertical Slice standard - ✅ **Guardrails:** RBAC enforced, no privilege escalation - ✅ **Traceability:** Correlation_id + evidence linking -- ✅ **Safety:** Idempotent, rollback-safe +- ⚠️ **Safety:** DateOnly parameter binding bug fixed 2026-08-08; unverified against a live DB + this session (see "Test status" above) - ✅ **Maturity:** Spec complete before code -- ✅ **Right-Way:** No shortcuts, formal approval workflow -- ✅ **Debt:** No new tech debt +- ✅ **Right-Way:** Duplicate implementation resolved per DEBT-017, not worked around +- ⚠️ **Debt:** DEBT-017 (duplicate implementation) resolved; two residual gaps discovered by this + cleanup (predate it, not introduced by it) are registered as DEBT-025 (no `GET /approvals/{id}`) + and DEBT-026 (no wired Draft→Proposed transition) ## Related Specifications - **VS-00:** PIT envelope (published_at, correlation_id, revision) - **VS-02:** Governance foundation (data sources, policies) -- **VS-04:** Audit trail (events logged by this slice) +- **VS-27 (formerly VS-04):** Audit trail (events logged by this slice) - **Compliance:** Maker-checker separation, evidence linkage --- -**Status:** ✅ IMPLEMENTATION COMPLETE -**Co-Authored-By:** Claude Haiku 4.5 +**Status:** Backend implementation is the canonical (sole) copy of this slice as of 2026-08-08; +integration tests exist but are unverified against a live database (see "Test status"). Not +"IMPLEMENTATION COMPLETE" until that verification runs and the two residual gaps above are +resolved or explicitly accepted. +**Co-Authored-By:** Claude Sonnet 5 diff --git a/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Sql.cs b/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Sql.cs index 774dbd8d..3b8f8b12 100644 --- a/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Sql.cs +++ b/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Sql.cs @@ -55,7 +55,7 @@ public class ApprovalWorkflowSql INSERT INTO model_operations.approval_proposals (id, model_id, status, created_by, created_at, justification, effective_at, published_at, revision, correlation_id) - VALUES (@id, @modelId, @status, @createdBy, @createdAt, @justification, @effectiveAt, + VALUES (@id, @modelId, @status, @createdBy, @createdAt, @justification, @effectiveAt::date, @publishedAt, @revision, @correlationId) RETURNING id """; @@ -69,7 +69,11 @@ public class ApprovalWorkflowSql proposal.CreatedBy, proposal.CreatedAt, proposal.Justification, - proposal.EffectiveAt, + // Dapper cannot bind DateOnly directly as an Npgsql parameter value (see TECH_DEBT_REGISTER.md + // DEBT-017); the identical failure was found and fixed the same way in the now-deleted + // ApprovalSql.cs (commit 2ccf74c) after it crashed 100% of proposal-creation calls against a + // real database. + effectiveAt = proposal.EffectiveAt.ToString("yyyy-MM-dd"), proposal.PublishedAt, proposal.Revision, proposal.CorrelationId diff --git a/src/KArtSell.Modules.ModelOperations/TradeExecution/Trade.cs b/src/KArtSell.Modules.ModelOperations/TradeExecution/Trade.cs index d7ce3783..49a73361 100644 --- a/src/KArtSell.Modules.ModelOperations/TradeExecution/Trade.cs +++ b/src/KArtSell.Modules.ModelOperations/TradeExecution/Trade.cs @@ -26,7 +26,7 @@ public class Trade public decimal? Commission { get; set; } public decimal? NetProceeds { get; set; } public string? ErrorMessage { get; set; } - public JsonElement? KisResponse { get; set; } + public string? KisResponse { get; set; } public DateTime? ExecutionTimestamp { get; set; } public DateTime? SettlementTimestamp { get; set; } public DateTime PublishedAt { get; set; } @@ -55,14 +55,14 @@ public class Trade { Status = TradeStatus.Submitted; KisOrderId = kisOrderId; - KisResponse = response; + KisResponse = response.ToString(); Revision++; } public void MarkAccepted(JsonElement response) { Status = TradeStatus.Accepted; - KisResponse = response; + KisResponse = response.ToString(); Revision++; } @@ -73,7 +73,7 @@ public class Trade TotalAmount = executedQty * unitPrice; Status = executedQty >= Quantity ? TradeStatus.FullyFilled : TradeStatus.PartiallyFilled; ExecutionTimestamp = now; - KisResponse = response; + KisResponse = response.ToString(); Revision++; } @@ -98,7 +98,7 @@ public class Trade public void MarkErrored(KisTradeExecutionException exception) { ErrorMessage = exception.Message; - KisResponse = exception.KisResponse; + KisResponse = exception.KisResponse?.ToString(); Revision++; } } diff --git a/src/KArtSell.Modules.ModelOperations/TradeExecution/TradeHandlers.cs b/src/KArtSell.Modules.ModelOperations/TradeExecution/TradeHandlers.cs index 1179e413..21aa73b3 100644 --- a/src/KArtSell.Modules.ModelOperations/TradeExecution/TradeHandlers.cs +++ b/src/KArtSell.Modules.ModelOperations/TradeExecution/TradeHandlers.cs @@ -57,14 +57,7 @@ public class SubmitTradeHandler ); trade.MarkSubmitted(orderId, response); - await _sql.UpdateTradeStatusAsync( - trade.Id, - TradeStatus.Submitted, - response, - null, - command.CorrelationId, - ct - ); + await _sql.UpdateTradeStatusAsync(trade, response, null, ct); await PublishEventAsync( "TradeSubmitted", @@ -84,14 +77,7 @@ public class SubmitTradeHandler catch (KisTradeExecutionException ex) { trade.MarkErrored(ex); - await _sql.UpdateTradeStatusAsync( - trade.Id, - trade.Status, - ex.KisResponse, - ex.Message, - command.CorrelationId, - ct - ); + await _sql.UpdateTradeStatusAsync(trade, ex.KisResponse, ex.Message, ct); _logger.LogError( "Trade submission failed: {TradeId} {Classification}", @@ -163,14 +149,7 @@ public class PollTradeStatusHandler trade.MarkFilled(executedQty, unitPrice, response, _clock.UtcNow.UtcDateTime); } - await _sql.UpdateTradeStatusAsync( - trade.Id, - trade.Status, - response, - null, - command.CorrelationId, - ct - ); + await _sql.UpdateTradeStatusAsync(trade, response, null, ct); if (trade.Status is TradeStatus.FullyFilled) { @@ -195,14 +174,7 @@ public class PollTradeStatusHandler } catch (KisTradeExecutionException ex) { - await _sql.UpdateTradeStatusAsync( - trade.Id, - trade.Status, - ex.KisResponse, - ex.Message, - command.CorrelationId, - ct - ); + await _sql.UpdateTradeStatusAsync(trade, ex.KisResponse, ex.Message, ct); _logger.LogError("Failed to poll trade status: {TradeId}", trade.Id); } @@ -262,14 +234,7 @@ public class ConfirmSettlementHandler if (success) { trade.MarkConfirmed(_clock.UtcNow.UtcDateTime, command.Commission); - await _sql.UpdateTradeStatusAsync( - trade.Id, - TradeStatus.Confirmed, - response, - null, - command.CorrelationId, - ct - ); + await _sql.UpdateTradeStatusAsync(trade, response, null, ct); await TradeOutboxPublisher.PublishAsync( _connectionFactory, @@ -290,14 +255,7 @@ public class ConfirmSettlementHandler } catch (KisTradeExecutionException ex) { - await _sql.UpdateTradeStatusAsync( - trade.Id, - trade.Status, - ex.KisResponse, - ex.Message, - command.CorrelationId, - ct - ); + await _sql.UpdateTradeStatusAsync(trade, ex.KisResponse, ex.Message, ct); _logger.LogError("Failed to confirm settlement: {TradeId}", trade.Id); } diff --git a/src/KArtSell.Modules.ModelOperations/TradeExecution/TradeSql.cs b/src/KArtSell.Modules.ModelOperations/TradeExecution/TradeSql.cs index 542ec168..a33dc955 100644 --- a/src/KArtSell.Modules.ModelOperations/TradeExecution/TradeSql.cs +++ b/src/KArtSell.Modules.ModelOperations/TradeExecution/TradeSql.cs @@ -12,7 +12,7 @@ public interface ITradeSql Task> GetTradesByStatusAsync(TradeStatus status, Guid correlationId, CancellationToken ct = default); Task> GetTradesByDecisionIdAsync(Guid sellDecisionId, Guid correlationId, CancellationToken ct = default); Task InsertTradeAsync(Trade trade, CancellationToken ct = default); - Task UpdateTradeStatusAsync(Guid tradeId, TradeStatus newStatus, JsonElement? kisResponse, string? errorMessage, Guid correlationId, CancellationToken ct = default); + Task UpdateTradeStatusAsync(Trade trade, JsonElement? kisResponse, string? errorMessage, CancellationToken ct = default); Task CountTradesByStatusAsync(TradeStatus status, CancellationToken ct = default); } @@ -33,7 +33,7 @@ public class TradeSql : ITradeSql const string sql = """ SELECT id, sell_decision_id, kis_order_id, status, quantity, executed_quantity, - unit_price, total_amount, commission, net_proceeds, error_message, kis_response, + unit_price, total_amount, commission, net_proceeds, error_message, kis_response::text as kis_response, execution_timestamp, settlement_timestamp, published_at, correlation_id, revision FROM model_operations.trades WHERE id = @tradeId @@ -61,7 +61,7 @@ public class TradeSql : ITradeSql const string sql = """ SELECT id, sell_decision_id, kis_order_id, status, quantity, executed_quantity, - unit_price, total_amount, commission, net_proceeds, error_message, kis_response, + unit_price, total_amount, commission, net_proceeds, error_message, kis_response::text as kis_response, execution_timestamp, settlement_timestamp, published_at, correlation_id, revision FROM model_operations.trades WHERE kis_order_id = @kisOrderId @@ -82,7 +82,7 @@ public class TradeSql : ITradeSql const string sql = """ SELECT id, sell_decision_id, kis_order_id, status, quantity, executed_quantity, - unit_price, total_amount, commission, net_proceeds, error_message, kis_response, + unit_price, total_amount, commission, net_proceeds, error_message, kis_response::text as kis_response, execution_timestamp, settlement_timestamp, published_at, correlation_id, revision FROM model_operations.trades WHERE status = @status @@ -102,7 +102,7 @@ public class TradeSql : ITradeSql const string sql = """ SELECT id, sell_decision_id, kis_order_id, status, quantity, executed_quantity, - unit_price, total_amount, commission, net_proceeds, error_message, kis_response, + unit_price, total_amount, commission, net_proceeds, error_message, kis_response::text as kis_response, execution_timestamp, settlement_timestamp, published_at, correlation_id, revision FROM model_operations.trades WHERE sell_decision_id = @sellDecisionId @@ -143,7 +143,7 @@ public class TradeSql : ITradeSql trade.Commission, trade.NetProceeds, trade.ErrorMessage, - kisResponse = trade.KisResponse?.ToString(), + trade.KisResponse, trade.ExecutionTimestamp, trade.SettlementTimestamp, trade.PublishedAt, @@ -155,11 +155,9 @@ public class TradeSql : ITradeSql } public async Task UpdateTradeStatusAsync( - Guid tradeId, - TradeStatus newStatus, + Trade trade, JsonElement? kisResponse, string? errorMessage, - Guid correlationId, CancellationToken ct = default) { using var connection = await _dataSource.OpenConnectionAsync(ct); @@ -173,6 +171,14 @@ public class TradeSql : ITradeSql UPDATE model_operations.trades SET status = @newStatus, + kis_order_id = COALESCE(@kisOrderId, kis_order_id), + executed_quantity = COALESCE(@executedQuantity, executed_quantity), + unit_price = COALESCE(@unitPrice, unit_price), + total_amount = COALESCE(@totalAmount, total_amount), + commission = COALESCE(@commission, commission), + net_proceeds = COALESCE(@netProceeds, net_proceeds), + execution_timestamp = COALESCE(@executionTimestamp, execution_timestamp), + settlement_timestamp = COALESCE(@settlementTimestamp, settlement_timestamp), kis_response = COALESCE(@kisResponse::jsonb, kis_response), error_message = COALESCE(@errorMessage, error_message), revision = revision + 1 @@ -182,14 +188,22 @@ public class TradeSql : ITradeSql await connection.ExecuteAsync(sql, new { id = Guid.NewGuid(), - tradeId, - newStatus = newStatus.ToString(), + tradeId = trade.Id, + newStatus = trade.Status.ToString(), + kisOrderId = trade.KisOrderId, + executedQuantity = trade.ExecutedQuantity, + unitPrice = trade.UnitPrice, + totalAmount = trade.TotalAmount, + commission = trade.Commission, + netProceeds = trade.NetProceeds, + executionTimestamp = trade.ExecutionTimestamp, + settlementTimestamp = trade.SettlementTimestamp, kisResponse = kisResponse?.ToString(), errorMessage, - correlationId + correlationId = trade.CorrelationId }); - _logger.LogInformation("Updated trade {TradeId} status to {Status}", tradeId, newStatus); + _logger.LogInformation("Updated trade {TradeId} status to {Status}", trade.Id, trade.Status); } public async Task CountTradesByStatusAsync(TradeStatus status, CancellationToken ct = default) diff --git a/src/KArtSell.Modules.SignalEngine/DapperMappingBootstrap.cs b/src/KArtSell.Modules.SignalEngine/DapperMappingBootstrap.cs new file mode 100644 index 00000000..f7f6ce1f --- /dev/null +++ b/src/KArtSell.Modules.SignalEngine/DapperMappingBootstrap.cs @@ -0,0 +1,21 @@ +using System.Runtime.CompilerServices; + +namespace KArtSell.Modules.SignalEngine; + +/// +/// See KArtSell.Modules.ModelOperations.DapperMappingBootstrap for the full rationale: Dapper's +/// snake_case-to-PascalCase column mapping is a process-wide static flag set via +/// KArtSell.BuildingBlocks.Data.DapperBootstrap's [ModuleInitializer], which only fires once +/// that assembly is loaded. This mirrors it locally so every Sql/reader class defined in this +/// assembly is guaranteed the mapping is on before its first query, regardless of load order. +/// +internal static class DapperMappingBootstrap +{ +#pragma warning disable CA2255 + [ModuleInitializer] +#pragma warning restore CA2255 + public static void Initialize() + { + Dapper.DefaultTypeMap.MatchNamesWithUnderscores = true; + } +} diff --git a/tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs b/tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs index 791498f2..89a62f20 100644 --- a/tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs +++ b/tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs @@ -2,198 +2,231 @@ namespace KArtSell.Integration.Tests.ApprovalWorkflow; using System; using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; -using Xunit; +using Dapper; using KArtSell.BuildingBlocks.Time; -using KArtSell.Modules.ModelOperations.ApprovalWorkflow; +using KArtSell.Modules.ModelOperations.Domain.ApprovalWorkflow; +using KArtSell.Modules.ModelOperations.Features.ApprovalWorkflow; +using Npgsql; +using Xunit; +/// +/// Integration coverage for the CANONICAL maker-checker approval slice +/// (KArtSell.Modules.ModelOperations.Features.ApprovalWorkflow — the one wired into +/// Program.cs and reachable over HTTP). Exercises Handler + Sql + a real PostgreSQL +/// database, per CLAUDE.md's Vertical Slice testing rules (no mocks for this layer). +/// +/// Replaces the old KArtSell.Modules.ModelOperations.ApprovalWorkflow namespace's test +/// coverage, deleted as part of resolving TECH_DEBT_REGISTER.md DEBT-017: that implementation +/// was dead code (all 4 endpoints [DontRegister]'d) even though it had 20/20 passing +/// tests, while this one — the implementation actually reachable at runtime — had none. +/// +/// Requires KARTSELL_POSTGRES (or appsettings.Development.json ConnectionStrings:Postgres) to +/// point at a reachable PostgreSQL instance with migrations 0035/0036 applied. See +/// TestDatabaseConnection.GetConnectionString(). +/// +[Collection("Database")] public class ApprovalWorkflowTests : IAsyncLifetime { private readonly string _connectionString; - private readonly ApprovalSql _sql; - private readonly ApprovalPolicy _policy; - private readonly IOutbox _outbox; + private readonly ApprovalWorkflowSql _sql; + private readonly IClock _clock = new SystemClock(); public ApprovalWorkflowTests() { - _connectionString = "Host=localhost;Port=5432;Database=kartselldb;Username=kartsell;Password=kartsell4321@!"; - _sql = new ApprovalSql(_connectionString, new SystemClock()); - _policy = new ApprovalPolicy(new SystemClock()); - _outbox = new InMemoryOutbox(); + _connectionString = TestDatabaseConnection.GetConnectionString(); + _sql = new ApprovalWorkflowSql(_connectionString); } public async Task InitializeAsync() { - // Ensure database is ready - await Task.CompletedTask; + await using var conn = new NpgsqlConnection(_connectionString); + await conn.OpenAsync(); + await conn.ExecuteAsync("SELECT 1"); } - public async Task DisposeAsync() + public Task DisposeAsync() => Task.CompletedTask; + + private async Task SeedModelAsync() { - await Task.CompletedTask; - } - - [Fact] - public void CanCreateProposal_WithMakerRole_ReturnsTrue() - { - // Arrange - var makerEmail = "maker@company.com"; - var makerRole = "Maker"; - - // Act - var result = _policy.CanCreateProposal(makerEmail, makerRole); - - // Assert - Assert.True(result); - } - - [Fact] - public void CanCreateProposal_WithoutMakerRole_ReturnsFalse() - { - // Arrange - var email = "user@company.com"; - var role = "Viewer"; - - // Act - var result = _policy.CanCreateProposal(email, role); - - // Assert - Assert.False(result); - } - - [Fact] - public void CanApproveApproval_WithDifferentChecker_ReturnsTrue() - { - // Arrange - var maker = "maker@company.com"; - var checker = "checker@company.com"; - var proposal = new ApprovalProposal { CreatedBy = maker, Status = ApprovalStatus.Proposed }; - - // Act - var result = _policy.CanApproveApproval(proposal, checker, maker); - - // Assert - Assert.True(result); - } - - [Fact] - public void CanApproveApproval_WithSameMaker_ReturnsFalse() - { - // Arrange - var maker = "maker@company.com"; - var proposal = new ApprovalProposal { CreatedBy = maker, Status = ApprovalStatus.Proposed }; - - // Act - var result = _policy.CanApproveApproval(proposal, maker, maker); - - // Assert - Assert.False(result); - } - - [Fact] - public void CreateProposal_SetsCorrectDefaults() - { - // Arrange var modelId = Guid.NewGuid(); - var maker = "maker@company.com"; - var justification = "Model passed OOS testing"; - var effectiveAt = DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)); + await using var conn = new NpgsqlConnection(_connectionString); + await conn.OpenAsync(); + await conn.ExecuteAsync( + "INSERT INTO model_operations.models (id, ticker, correlation_id) VALUES (@Id, @Ticker, @CorrelationId)", + new { Id = modelId, Ticker = "TEST", CorrelationId = Guid.NewGuid() }); + return modelId; + } + + [Fact] + public async Task CreateProposal_WithMakerRole_InsertsDraftProposal_AndEffectiveAtRoundTrips() + { + // Arrange: DateOnly binding is the specific regression this test guards against — the + // previously-deleted ApprovalSql.cs could not INSERT a DateOnly parameter through Dapper + // at all (fixed in commit 2ccf74c). Use a date with no "today" coincidence risk. + var modelId = await SeedModelAsync(); + var handler = new CreateApprovalProposalHandler(_sql, _clock); + var effectiveAt = new DateOnly(2028, 2, 29); // leap day: exercises DateOnly edge case too // Act - var proposal = _policy.CreateProposal(modelId, maker, justification, effectiveAt); + var proposalId = await handler.Handle( + "maker@company.com", "Maker", modelId, effectiveAt, "Model passed OOS testing", Guid.NewGuid()); // Assert + var proposal = await _sql.GetProposalAsync(proposalId); + Assert.NotNull(proposal); + Assert.Equal(ApprovalStatus.Draft, proposal!.Status); Assert.Equal(modelId, proposal.ModelId); - Assert.Equal(maker, proposal.CreatedBy); - Assert.Equal(ApprovalStatus.Draft, proposal.Status); - Assert.Equal(justification, proposal.Justification); + Assert.Equal("maker@company.com", proposal.CreatedBy); Assert.Equal(effectiveAt, proposal.EffectiveAt); } [Fact] - public void ProposeApproval_TransitionsToProposed() + public async Task CreateProposal_WithNonMakerRole_ThrowsUnauthorized() { // Arrange - var proposal = new ApprovalProposal - { - Id = Guid.NewGuid(), - CreatedBy = "maker@company.com", - Status = ApprovalStatus.Draft, - Justification = "Test", - EffectiveAt = DateOnly.FromDateTime(DateTime.UtcNow) - }; + var modelId = await SeedModelAsync(); + var handler = new CreateApprovalProposalHandler(_sql, _clock); - // Act - var updated = _policy.ProposeApproval(proposal, "maker@company.com"); - - // Assert - Assert.Equal(ApprovalStatus.Proposed, updated.Status); - Assert.NotNull(updated.ProposedAt); + // Act & Assert + await Assert.ThrowsAsync(() => + handler.Handle("viewer@company.com", "Viewer", modelId, DateOnly.FromDateTime(DateTime.UtcNow), "no role", Guid.NewGuid())); } [Fact] - public void ApproveApproval_AddsEvidence() + public async Task Approve_WithDifferentChecker_TransitionsToApproved_AndAttachesEvidence() { // Arrange - var proposal = new ApprovalProposal + var modelId = await SeedModelAsync(); + var createHandler = new CreateApprovalProposalHandler(_sql, _clock); + var proposalId = await createHandler.Handle( + "maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)), + "Model passed OOS testing; PBO score 0.95", Guid.NewGuid()); + await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Proposed); + + var approveHandler = new ApproveApprovalHandler(_sql, _clock); + var evidence = new List<(string Type, string Url, string? Comment)> { - Id = Guid.NewGuid(), - CreatedBy = "maker@company.com", - Status = ApprovalStatus.Proposed, - Evidence = [], - CorrelationId = Guid.NewGuid() - }; - var evidence = new List - { - new() { Type = "PBO_SCORE", Url = "s3://pbo-0.95.json", Comment = "Verified" } + ("PBO_SCORE", "s3://evidence/pbo-0.95.json", "Verified"), + ("OOS_RETURN", "s3://evidence/oos-returns.csv", "Acceptable"), }; // Act - var updated = _policy.ApproveApproval(proposal, "checker@company.com", "Looks good", evidence); + await approveHandler.Handle(proposalId, "checker@company.com", "Checker", "PBO verified, OOS metrics acceptable", evidence, Guid.NewGuid()); // Assert - Assert.Equal(ApprovalStatus.Approved, updated.Status); - Assert.Equal("checker@company.com", updated.ApprovedBy); - Assert.Single(updated.Evidence); - Assert.Equal("PBO_SCORE", updated.Evidence[0].EvidenceType); + var proposal = await _sql.GetProposalAsync(proposalId); + Assert.NotNull(proposal); + Assert.Equal(ApprovalStatus.Approved, proposal!.Status); + Assert.Equal("checker@company.com", proposal.ApprovedBy); + Assert.NotNull(proposal.ApprovedAt); + + await using var conn = new NpgsqlConnection(_connectionString); + await conn.OpenAsync(); + var evidenceCount = await conn.QuerySingleAsync( + "SELECT COUNT(*) FROM model_operations.approval_evidence WHERE approval_proposal_id = @ProposalId", + new { ProposalId = proposalId }); + Assert.Equal(2, evidenceCount); } [Fact] - public async Task InsertAndRetrieveProposal_RoundTrips() + public async Task Approve_BySameMakerAsChecker_ThrowsUnauthorized_SeparationOfDuties() { // Arrange - var id = Guid.NewGuid(); - var modelId = Guid.NewGuid(); - var correlationId = Guid.NewGuid(); + var modelId = await SeedModelAsync(); + var createHandler = new CreateApprovalProposalHandler(_sql, _clock); + var proposalId = await createHandler.Handle( + "maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)), + "Self-approval attempt", Guid.NewGuid()); + await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Proposed); + + var approveHandler = new ApproveApprovalHandler(_sql, _clock); + + // Act & Assert: same user, even with Checker role, cannot approve their own proposal + await Assert.ThrowsAsync(() => + approveHandler.Handle(proposalId, "maker@company.com", "Checker", "self-approved", [], Guid.NewGuid())); + } + + [Fact] + public async Task Approve_WithNonCheckerRole_ThrowsUnauthorized() + { + // Arrange + var modelId = await SeedModelAsync(); + var createHandler = new CreateApprovalProposalHandler(_sql, _clock); + var proposalId = await createHandler.Handle( + "maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)), + "Needs a Checker role, not just a different user", Guid.NewGuid()); + await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Proposed); + + var approveHandler = new ApproveApprovalHandler(_sql, _clock); + + // Act & Assert: different user, but wrong role + await Assert.ThrowsAsync(() => + approveHandler.Handle(proposalId, "other-maker@company.com", "Maker", "wrong role", [], Guid.NewGuid())); + } + + [Fact] + public async Task Activate_BySreAfterApproval_TransitionsToActive() + { + // Arrange + var modelId = await SeedModelAsync(); + var createHandler = new CreateApprovalProposalHandler(_sql, _clock); + var proposalId = await createHandler.Handle( + "maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)), + "Ready for activation", Guid.NewGuid()); + await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Proposed); + await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Approved, "checker@company.com", "Approved"); + + var activateHandler = new ActivateModelHandler(_sql, _clock); // Act - await _sql.InsertProposalAsync( - id, - modelId, - "Draft", - "maker@company.com", - "Test justification", - DateOnly.FromDateTime(DateTime.UtcNow), - DateTimeOffset.UtcNow, - correlationId); - - var retrieved = await _sql.GetProposalByIdAsync(id, DateTimeOffset.UtcNow.AddDays(1)); + await activateHandler.Handle(proposalId, "sre@company.com", "SRE", Guid.NewGuid()); // Assert - Assert.NotNull(retrieved); - Assert.Equal(id, retrieved.Id); - Assert.Equal(modelId, retrieved.ModelId); - Assert.Equal(correlationId, retrieved.CorrelationId); + var proposal = await _sql.GetProposalAsync(proposalId); + Assert.NotNull(proposal); + Assert.Equal(ApprovalStatus.Active, proposal!.Status); } -} -public class InMemoryOutbox : IOutbox -{ - public List<(string EventType, Guid CorrelationId, object Data)> Events { get; } = []; - - public Task PublishAsync(string eventType, Guid correlationId, object data) + [Fact] + public async Task Activate_WithNonSreRole_ThrowsUnauthorized() { - Events.Add((eventType, correlationId, data)); - return Task.CompletedTask; + // Arrange + var modelId = await SeedModelAsync(); + var createHandler = new CreateApprovalProposalHandler(_sql, _clock); + var proposalId = await createHandler.Handle( + "maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)), + "Should not activate without SRE", Guid.NewGuid()); + await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Proposed); + await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Approved, "checker@company.com", "Approved"); + + var activateHandler = new ActivateModelHandler(_sql, _clock); + + // Act & Assert + await Assert.ThrowsAsync(() => + activateHandler.Handle(proposalId, "maker@company.com", "Maker", Guid.NewGuid())); + } + + [Fact] + public async Task ListProposalsAsync_FiltersByStatusAndModelId() + { + // Arrange + var modelId = await SeedModelAsync(); + var createHandler = new CreateApprovalProposalHandler(_sql, _clock); + var draftId = await createHandler.Handle( + "maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)), + "Stays in Draft", Guid.NewGuid()); + var proposedId = await createHandler.Handle( + "maker@company.com", "Maker", modelId, DateOnly.FromDateTime(DateTime.UtcNow.AddDays(7)), + "Moves to Proposed", Guid.NewGuid()); + await _sql.UpdateProposalStatusAsync(proposedId, ApprovalStatus.Proposed); + + // Act + var proposedOnly = await _sql.ListProposalsAsync(ApprovalStatus.Proposed, modelId); + + // Assert + Assert.Contains(proposedOnly, p => p.Id == proposedId); + Assert.DoesNotContain(proposedOnly, p => p.Id == draftId); } } diff --git a/tests/KArtSell.Integration.Tests/ApprovalWorkflowPolicyTests.cs b/tests/KArtSell.Integration.Tests/ApprovalWorkflowPolicyTests.cs index 21675747..8321a8bc 100644 --- a/tests/KArtSell.Integration.Tests/ApprovalWorkflowPolicyTests.cs +++ b/tests/KArtSell.Integration.Tests/ApprovalWorkflowPolicyTests.cs @@ -13,6 +13,13 @@ public class ApprovalWorkflowPolicyTests Assert.True(result); } + [Fact] + public void CanCreateProposal_NonMakerRole_ReturnsFalse() + { + var result = ApprovalWorkflowPolicy.CanCreateProposal("user@test.com", "Viewer"); + Assert.False(result); + } + [Fact] public void CanApprove_CheckerDifferentFromMaker_ReturnsTrue() { @@ -29,6 +36,38 @@ public class ApprovalWorkflowPolicyTests Assert.False(result); } + [Fact] + public void CanApprove_NonCheckerRole_ReturnsFalse() + { + var proposal = new ApprovalProposal { CreatedBy = "maker@test.com", Justification = "test", Status = ApprovalStatus.Proposed }; + var result = ApprovalWorkflowPolicy.CanApprove(proposal, "other@test.com", "Maker"); + Assert.False(result); + } + + [Fact] + public void CanActivate_SreRoleWithApprovedStatus_ReturnsTrue() + { + var proposal = new ApprovalProposal { CreatedBy = "maker@test.com", Justification = "test", Status = ApprovalStatus.Approved }; + var result = ApprovalWorkflowPolicy.CanActivate(proposal, "sre@test.com", "SRE"); + Assert.True(result); + } + + [Fact] + public void CanActivate_NonSreRole_ReturnsFalse() + { + var proposal = new ApprovalProposal { CreatedBy = "maker@test.com", Justification = "test", Status = ApprovalStatus.Approved }; + var result = ApprovalWorkflowPolicy.CanActivate(proposal, "maker@test.com", "Maker"); + Assert.False(result); + } + + [Fact] + public void CanActivate_ApprovedRequired_NonApprovedStatus_ReturnsFalse() + { + var proposal = new ApprovalProposal { CreatedBy = "maker@test.com", Justification = "test", Status = ApprovalStatus.Proposed }; + var result = ApprovalWorkflowPolicy.CanActivate(proposal, "sre@test.com", "SRE"); + Assert.False(result); + } + [Fact] public void ValidateProposalState_ValidTransition_Succeeds() { diff --git a/tests/KArtSell.Integration.Tests/Compliance/AuditTrailTests.cs b/tests/KArtSell.Integration.Tests/Compliance/AuditTrailTests.cs index ea22d52e..bf1fdc2a 100644 --- a/tests/KArtSell.Integration.Tests/Compliance/AuditTrailTests.cs +++ b/tests/KArtSell.Integration.Tests/Compliance/AuditTrailTests.cs @@ -106,6 +106,11 @@ public class AuditTrailTests : IAsyncLifetime var customerId = Guid.NewGuid(); var retentionId = Guid.NewGuid(); + await _sql.InsertAuditEventAsync( + _db, eventId, AuditEventTypes.ModelActivated, AuditEntityTypes.Model, + Guid.NewGuid(), "customer@company.com", null, DateTime.UtcNow, "SUCCESS", null, + null, null, null, null, Guid.NewGuid(), CancellationToken.None); + // Act await _sql.InsertGdprRetentionAsync( _db, retentionId, eventId, customerId, @@ -175,7 +180,8 @@ public class AuditTrailTests : IAsyncLifetime // Assert var @event = await _sql.GetAuditEventByIdAsync(_db, eventId, CancellationToken.None); Assert.NotNull(@event); - Assert.Contains("", @event.Details?.ToString() ?? ""); + Assert.Equal("", @event.Details?["actor_email"].ToString()); + Assert.Equal("", @event.Details?["customer_id"].ToString()); } private const string TestConnectionString = diff --git a/tests/KArtSell.Integration.Tests/SellDecision/SellDecisionTests.cs b/tests/KArtSell.Integration.Tests/SellDecision/SellDecisionTests.cs index 67451a26..a9bb2fdb 100644 --- a/tests/KArtSell.Integration.Tests/SellDecision/SellDecisionTests.cs +++ b/tests/KArtSell.Integration.Tests/SellDecision/SellDecisionTests.cs @@ -118,8 +118,8 @@ public class SellPriorityRankerTests [Fact] public void CalculateScore_HardImpairment_ReturnsLowestScore() { - var score = _ranker.CalculateScore(SellPriority.HardImpairment, fundAgeDays: 200, liquidityPercent: 0.5m); - Assert.Equal(950m, score); // 1000 - 50 (age boost) + var score = _ranker.CalculateScore(SellPriority.HardImpairment, fundAgeDays: 400, liquidityPercent: 0.5m); + Assert.Equal(950m, score); // 1000 - 50 (age boost, fundAgeDays > 365 per VS-10-SLICE_SPEC.md) } [Fact] diff --git a/tests/KArtSell.Integration.Tests/TradeExecution/TradeExecutionTests.cs b/tests/KArtSell.Integration.Tests/TradeExecution/TradeExecutionTests.cs index b8d4e011..20e44a38 100644 --- a/tests/KArtSell.Integration.Tests/TradeExecution/TradeExecutionTests.cs +++ b/tests/KArtSell.Integration.Tests/TradeExecution/TradeExecutionTests.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using Dapper; using KArtSell.Modules.ModelOperations.TradeExecution; using Microsoft.Extensions.Logging; using Npgsql; @@ -31,11 +32,31 @@ public class TradeExecutionTests : IAsyncLifetime await _dataSource.DisposeAsync(); } + private async Task SeedSellDecisionAsync() + { + await using var connection = await _dataSource.OpenConnectionAsync(); + var modelId = Guid.NewGuid(); + await connection.ExecuteAsync( + "INSERT INTO model_operations.models (id, ticker, correlation_id) VALUES (@Id, @Ticker, @CorrelationId)", + new { Id = modelId, Ticker = "TEST", CorrelationId = Guid.NewGuid() }); + + var sellDecisionId = Guid.NewGuid(); + await connection.ExecuteAsync( + """ + INSERT INTO model_operations.sell_decisions + (id, model_id, status, created_by, published_at, correlation_id) + VALUES (@Id, @ModelId, 'PENDING', 'test@company.com', NOW(), @CorrelationId) + """, + new { Id = sellDecisionId, ModelId = modelId, CorrelationId = Guid.NewGuid() }); + + return sellDecisionId; + } + [Fact] public async Task CreateTrade_WithValidData_ShouldInsertSuccessfully() { var sql = new TradeSql(_dataSource, _logger); - var sellDecisionId = Guid.NewGuid(); + var sellDecisionId = await SeedSellDecisionAsync(); var correlationId = Guid.NewGuid(); var trade = Trade.Create(sellDecisionId, 1000, correlationId, DateTime.UtcNow); @@ -55,14 +76,14 @@ public class TradeExecutionTests : IAsyncLifetime { var sql = new TradeSql(_dataSource, _logger); var correlationId = Guid.NewGuid(); - var trade = Trade.Create(Guid.NewGuid(), 1000, correlationId, DateTime.UtcNow); + var trade = Trade.Create(await SeedSellDecisionAsync(), 1000, correlationId, DateTime.UtcNow); await sql.InsertTradeAsync(trade); var response = JsonDocument.Parse("{}").RootElement; trade.MarkSubmitted("KIS-ORDER-123", response); - await sql.UpdateTradeStatusAsync(trade.Id, TradeStatus.Submitted, response, null, correlationId); + await sql.UpdateTradeStatusAsync(trade, response, null); var retrieved = await sql.GetTradeByIdAsync(trade.Id, correlationId); @@ -76,14 +97,14 @@ public class TradeExecutionTests : IAsyncLifetime { var sql = new TradeSql(_dataSource, _logger); var correlationId = Guid.NewGuid(); - var trade = Trade.Create(Guid.NewGuid(), 1000, correlationId, DateTime.UtcNow); + var trade = Trade.Create(await SeedSellDecisionAsync(), 1000, correlationId, DateTime.UtcNow); await sql.InsertTradeAsync(trade); var response = JsonDocument.Parse("{}").RootElement; trade.MarkFilled(1000, 49.95m, response, DateTime.UtcNow); - await sql.UpdateTradeStatusAsync(trade.Id, TradeStatus.FullyFilled, response, null, correlationId); + await sql.UpdateTradeStatusAsync(trade, response, null); var retrieved = await sql.GetTradeByIdAsync(trade.Id, correlationId); @@ -100,8 +121,8 @@ public class TradeExecutionTests : IAsyncLifetime var sql = new TradeSql(_dataSource, _logger); var correlationId = Guid.NewGuid(); - var trade1 = Trade.Create(Guid.NewGuid(), 1000, correlationId, DateTime.UtcNow); - var trade2 = Trade.Create(Guid.NewGuid(), 2000, correlationId, DateTime.UtcNow); + var trade1 = Trade.Create(await SeedSellDecisionAsync(), 1000, correlationId, DateTime.UtcNow); + var trade2 = Trade.Create(await SeedSellDecisionAsync(), 2000, correlationId, DateTime.UtcNow); await sql.InsertTradeAsync(trade1); await sql.InsertTradeAsync(trade2); @@ -118,14 +139,14 @@ public class TradeExecutionTests : IAsyncLifetime { var sql = new TradeSql(_dataSource, _logger); var correlationId = Guid.NewGuid(); - var trade = Trade.Create(Guid.NewGuid(), 1000, correlationId, DateTime.UtcNow); + var trade = Trade.Create(await SeedSellDecisionAsync(), 1000, correlationId, DateTime.UtcNow); await sql.InsertTradeAsync(trade); trade.TotalAmount = 49950m; trade.MarkConfirmed(DateTime.UtcNow, 50m); - await sql.UpdateTradeStatusAsync(trade.Id, TradeStatus.Confirmed, null, null, correlationId); + await sql.UpdateTradeStatusAsync(trade, null, null); var retrieved = await sql.GetTradeByIdAsync(trade.Id, correlationId); @@ -140,11 +161,16 @@ public class TradeExecutionTests : IAsyncLifetime { var sql = new TradeSql(_dataSource, _logger); var correlationId = Guid.NewGuid(); - var trade = Trade.Create(Guid.NewGuid(), 1000, correlationId, DateTime.UtcNow); + var trade = Trade.Create(await SeedSellDecisionAsync(), 1000, correlationId, DateTime.UtcNow); await sql.InsertTradeAsync(trade); - await sql.UpdateTradeStatusAsync(trade.Id, TradeStatus.Submitted, null, null, correlationId); - await sql.UpdateTradeStatusAsync(trade.Id, TradeStatus.Accepted, null, null, correlationId); + + var response = JsonDocument.Parse("{}").RootElement; + trade.MarkSubmitted("KIS-1", response); + await sql.UpdateTradeStatusAsync(trade, response, null); + + trade.MarkAccepted(response); + await sql.UpdateTradeStatusAsync(trade, response, null); var retrieved = await sql.GetTradeByIdAsync(trade.Id, correlationId); @@ -158,8 +184,8 @@ public class TradeExecutionTests : IAsyncLifetime var sql = new TradeSql(_dataSource, _logger); var correlationId = Guid.NewGuid(); - var trade1 = Trade.Create(Guid.NewGuid(), 1000, correlationId, DateTime.UtcNow); - var trade2 = Trade.Create(Guid.NewGuid(), 2000, correlationId, DateTime.UtcNow); + var trade1 = Trade.Create(await SeedSellDecisionAsync(), 1000, correlationId, DateTime.UtcNow); + var trade2 = Trade.Create(await SeedSellDecisionAsync(), 2000, correlationId, DateTime.UtcNow); await sql.InsertTradeAsync(trade1); await sql.InsertTradeAsync(trade2);