From 9ffb740f0742b78dfc2945a491a5dea0348b00dc Mon Sep 17 00:00:00 2001 From: kjh2064 Date: Sun, 9 Aug 2026 00:32:02 +0900 Subject: [PATCH] fix: DEBT-028 - wire ActivateModelHandler, fix data-corrupting activation Systematic sweep of every *Handler registered in Program.cs (same method that found DEBT-026/027) found ActivateModelHandler was the last orphan in Features/ApprovalWorkflow/: no POST /approvals/{id}/activate endpoint existed, so an Approved proposal could never reach Active - the entire point of this maker-checker slice. While wiring it up, found the handler's original call would have overwritten the checker's approved_by/approval_notes with the activating SRE's identity (it passed userEmail through UpdateProposalStatusAsync's approvedBy parameter), and never set activated_by/activated_at at all despite those columns existing since migration 0036. Added a dedicated ApprovalWorkflowSql.ActivateProposalAsync that only touches activation-specific columns, and a regression test asserting the checker's approval record survives activation unchanged. Also documents DEBT-029 (discovered, not fixed - genuine cross-cutting scope): LogAuditEventCommandHandler is never called by any other slice, so VS-27's audit trail is empty in production regardless of activity even though its own tests pass. Downgraded AEG-VS-27-01 from COMPLETED to BLOCKED in the tracker to reflect that honestly. dotnet build KArtSell.sln -c Release: clean. Not run against a live database this session. Co-Authored-By: Claude Sonnet 5 --- TECH_DEBT_REGISTER.md | 2 + .../CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv | 4 +- .../Features/ApprovalWorkflow/Endpoints.cs | 37 +++++++++++++++++++ .../Features/ApprovalWorkflow/Handlers.cs | 5 ++- .../Features/ApprovalWorkflow/Sql.cs | 20 ++++++++++ .../ApprovalWorkflow/ApprovalWorkflowTests.cs | 12 +++++- 6 files changed, 75 insertions(+), 5 deletions(-) diff --git a/TECH_DEBT_REGISTER.md b/TECH_DEBT_REGISTER.md index fecbbaad..09ce8bcf 100644 --- a/TECH_DEBT_REGISTER.md +++ b/TECH_DEBT_REGISTER.md @@ -60,6 +60,8 @@ | DEBT-025 | `Features/ApprovalWorkflow` has no `GET /approvals/{id}` endpoint | Medium (2) | Low (1) | Completed (DB verification pending) | Added `GetApprovalByIdEndpoint` (`GET /approvals/{id}`) + `ApprovalDetailResponse` (includes `Evidence`), and `ApprovalWorkflowSql.GetEvidenceForProposalAsync`. Evidence attached during approval (PBO/DSR/OOS artifact links) is now readable via HTTP. Two new tests added (`GetEvidenceForProposalAsync_ReturnsEvidenceAttachedDuringApproval` + the endpoint itself). `dotnet build -c Release` clean (0/0). **Not verified against a live database** — same 127.0.0.1:5432 connection-refused blocker as DEBT-017/026; do not mark fully verified until a real Postgres run passes. | @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) | Completed (DB verification pending) | Added `ProposeForReviewHandler` + `POST /approvals/{id}/propose`, wired into `Program.cs` DI. Calls the pre-existing `ApprovalWorkflowPolicy.CanProposeForReview` (creator-only) and `ValidateProposalState` (Draft→Proposed), then updates status and emits a `PROPOSED` event — same pattern as `ApproveApprovalHandler`/`ActivateModelHandler`. A proposal created via `POST /approvals` can now reach `Approved`/`Active` through the HTTP API end-to-end. Two new tests added (`ProposeForReview_ByCreatingMaker_TransitionsDraftToProposed`, `ProposeForReview_ByDifferentUserThanCreator_ThrowsUnauthorized`). `dotnet build -c Release` clean (0/0). **Not verified against a live database** — same 127.0.0.1:5432 connection-refused blocker as DEBT-017/025; `dotnet test --filter FullyQualifiedName~ApprovalWorkflowTests -c Release` run 2026-08-08, all 17 matched tests fail with connection-refused (includes this file's tests plus an unrelated top-level `ApprovalWorkflowTests.cs` the substring filter also matches). Do not mark fully verified until a real Postgres run passes. | @claude | DEBT-017 (2026-08-08), `src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/README.md` | | DEBT-027 | `PollTradeStatusHandler`/`ConfirmSettlementHandler` registered in DI but never invoked by anything | High (3) | Low (1) | Completed (DB verification pending) | Discovered while looking for BE/scheduler priority work (2026-08-09) — same class of gap as DEBT-026 (a fully-implemented handler with no caller). `TradeEndpoints.cs` only has `POST /trades` (→`SubmitTradeHandler`) and `GET /trades`; nothing ever called `PollTradeStatusHandler` or `ConfirmSettlementHandler`, and no Hangfire job did either, so a trade could reach `Submitted` and never progress — KIS fills and settlement confirmations were never picked up. Added `src/KArtSell.Host/Jobs/TradeStatusPollingJob.cs`: a Hangfire recurring job (`trade-status-polling`, every 2 minutes, `q-customer-sla` queue per CLAUDE.md's queue-isolation guidance since this affects real trade completion, not research) that queries `Submitted`/`Accepted`/`PartiallyFilled` trades and calls `PollTradeStatusHandler`, then queries `FullyFilled` trades and calls `ConfirmSettlementHandler`. Registered in `Program.cs` alongside the other recurring jobs. `dotnet build -c Release` clean (0/0). **No dedicated test added** (the job is thin orchestration over the already-implemented, already-covered-elsewhere handlers, and writing a fake `IKisTradeExecutionService`/`ITradeSql` test double would be a new testing pattern not used anywhere else in this codebase — flagged rather than done rashly) **and not run against a live database or KIS** — same connection blocker as the rest of this session's work. | @claude | Session 2026-08-09 (BE/scheduler priority pass) | +| DEBT-028 | `ActivateModelHandler` had no HTTP endpoint, and would have corrupted approval data if wired naively | High (3) | Low (1) | Completed (DB verification pending) | Found via a systematic sweep of every `*Handler` registered in `Program.cs`'s DI container, checking whether each is actually referenced by an `Endpoint.cs` or a job (the same method that found DEBT-026/027) — `ActivateModelHandler` was the only remaining orphan in `Features/ApprovalWorkflow/`: no `POST /approvals/{id}/activate` existed, so an `Approved` proposal could never reach `Active`, the step this whole slice exists for. While wiring it up, found the handler's original call — `_sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Active, userEmail, "Model activated by SRE", ct)` — would have passed the *activating SRE's* email/note through the `approvedBy`/`approvalNotes` parameters, overwriting the checker's real `approved_by`/`approval_notes` on activation, and never touched the schema's `activated_by`/`activated_at` columns at all (they existed since migration `0036` but nothing ever wrote them). Added a dedicated `ApprovalWorkflowSql.ActivateProposalAsync(proposalId, activatedBy, ct)` that only sets `status='ACTIVE'`, `activated_by`, `activated_at`, leaving `approved_by`/`approval_notes` untouched, and switched `ActivateModelHandler` to call it. Added `ActivateApprovalEndpoint` (`POST /approvals/{id}/activate`). Strengthened the existing `Activate_BySreAfterApproval_TransitionsToActive` test to assert `activated_by`/`activated_at` are set and the checker's `approved_by`/`approval_notes` survive activation unchanged — this would have caught the bug. `dotnet build -c Release` clean (0/0). Not run against a live database this session. | @claude | Session 2026-08-09 (BE/scheduler priority pass) | +| DEBT-029 | `LogAuditEventCommandHandler` (VS-27 audit trail) is never called by any other slice | High (3) | Medium (2) | Backlog | Found during the same orphaned-handler sweep that found DEBT-027/028 — but unlike those, this one is NOT a missing single endpoint; it's a missing *cross-cutting integration*. `LogAuditEventCommandHandler` (`src/KArtSell.Modules.ModelOperations/Compliance/LogAuditEventHandler.cs`) is the intended call point for every other slice to record an auditable action (per `compliance.audit_event_types`'s seed data: `APPROVAL_PROPOSED`, `APPROVAL_APPROVED`, `MODEL_ACTIVATED`, `SELL_DECISION_MADE`, `SELL_EXECUTED`, etc.) — but nothing in `ApprovalWorkflow/Handlers.cs`, `TradeExecution/TradeHandlers.cs`, `SellDecision/*`, or `PortfolioReconciliation/ReconcileTradeHandler.cs` actually calls it. VS-27 ("Immutable Audit Trail") is marked `COMPLETED` in the WBS tracker with 5/5 tests passing, but those tests only exercise `AuditSql` directly — they don't prove the rest of the system ever produces an audit trail in practice. Net effect: the compliance/GDPR audit trail this system's governance model depends on (CLAUDE.md's "Evidence & Audit: Update/delete are blocked; new state appended as new revision") is currently empty in production regardless of how many approvals/trades/sell-decisions happen, because nothing populates it outside of direct `AuditSql` test calls. **Not fixed this session** — wiring it in touches 4+ handler classes across 3+ slices (a genuine cross-cutting integration, not a single bounded fix like DEBT-026/027/028), and each call site needs to decide what `EventType`/`Details`/`EvidenceLinks` are correct for that action rather than a mechanical change. Recommend one slice at a time, starting with `ApprovalWorkflow` (highest governance stakes) via its outbox events (`ApprovalWorkflowPolicy.CreateStateChangeEvent` already emits an event per transition — a downstream consumer job could call `LogAuditEventCommandHandler` from there instead of wiring it into every handler directly, matching this repo's existing Outbox→Inbox→consumer pattern). | @claude | Session 2026-08-09 (BE/scheduler priority pass, discovery only) | --- diff --git a/docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv b/docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv index a16d31d2..00f52c9d 100644 --- a/docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv +++ b/docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv @@ -27,8 +27,8 @@ 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/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 (12 cases); tests/KArtSell.Integration.Tests/ApprovalWorkflowPolicyTests.cs (10 cases); TECH_DEBT_REGISTER.md DEBT-017/DEBT-023/DEBT-025/DEBT-026; commit a2e742c (original duplication, superseded)",PM/BE Lead,"🟡 UPDATED 2026-08-08 (second pass, BE priority work) — DEBT-025 and DEBT-026 (both discovered during the DEBT-017 cleanup earlier the same day) are now also code-complete: added `ProposeForReviewHandler` + `POST /approvals/{id}/propose` (wires the previously-dead `ApprovalWorkflowPolicy.CanProposeForReview`, so a proposal created via `POST /approvals` can now reach Approved/Active through the HTTP API end-to-end — this was the higher-impact gap, DEBT-026), and `GetApprovalByIdEndpoint` (`GET /approvals/{id}`) + `ApprovalWorkflowSql.GetEvidenceForProposalAsync` so approval evidence is readable via HTTP (DEBT-025). 4 new tests added (12 total in this file). `dotnet build KArtSell.sln -c Release` clean (0 warnings/0 errors). **Still BLOCKED, not COMPLETED: no PostgreSQL reachable in this session (127.0.0.1:5432 connection refused, no SSH tunnel open) — `dotnet test --filter FullyQualifiedName~ApprovalWorkflowTests -c Release` run 2026-08-08, all 17 matched tests fail with connection-refused (includes an unrelated top-level ApprovalWorkflowTests.cs the substring filter also matches). None of the 12 tests in this file have been confirmed to pass 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. ⚠️ Frontend UI built 2026-08-08 (frontend/src/features/audit-trail/, route /compliance/audit-trail, pnpm typecheck/build clean, 19 new tests passing) but on an isolated worktree branch (worktree-agent-a2cc5afe46a7e16b1) not yet merged into this branch — deprioritized behind BE work per user direction 2026-08-08; also flagged DEBT-025 (Compliance endpoints AllowAnonymous with no real RBAC, and no PermissionGuard component exists in this repo despite CLAUDE.md listing it as always-shared) and DEBT-026 (~130 stray committed .js files regenerate on every pnpm build) — note both DEBT-025 numbers collide with a different DEBT-025 added the same day on this branch (GET /approvals/{id}); renumber on merge. 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-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 (12 cases); tests/KArtSell.Integration.Tests/ApprovalWorkflowPolicyTests.cs (10 cases); TECH_DEBT_REGISTER.md DEBT-017/DEBT-023/DEBT-025/DEBT-026; commit a2e742c (original duplication, superseded)",PM/BE Lead,"🟡 UPDATED 2026-08-08 (second pass, BE priority work) — DEBT-025 and DEBT-026 (both discovered during the DEBT-017 cleanup earlier the same day) are now also code-complete: added `ProposeForReviewHandler` + `POST /approvals/{id}/propose` (wires the previously-dead `ApprovalWorkflowPolicy.CanProposeForReview`, so a proposal created via `POST /approvals` can now reach Approved/Active through the HTTP API end-to-end — this was the higher-impact gap, DEBT-026), and `GetApprovalByIdEndpoint` (`GET /approvals/{id}`) + `ApprovalWorkflowSql.GetEvidenceForProposalAsync` so approval evidence is readable via HTTP (DEBT-025). 4 new tests added (12 total in this file). `dotnet build KArtSell.sln -c Release` clean (0 warnings/0 errors). **Still BLOCKED, not COMPLETED: no PostgreSQL reachable in this session (127.0.0.1:5432 connection refused, no SSH tunnel open) — `dotnet test --filter FullyQualifiedName~ApprovalWorkflowTests -c Release` run 2026-08-08, all 17 matched tests fail with connection-refused (includes an unrelated top-level ApprovalWorkflowTests.cs the substring filter also matches). None of the 12 tests in this file have been confirmed to pass 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. 2026-08-09 (BE priority pass, third pass): DEBT-028 fixed — `ActivateModelHandler` had no endpoint at all (an Approved proposal could never reach Active) and, if wired naively, would have overwritten the checker's approved_by/approval_notes with the activating SRE's identity and never populated activated_by/activated_at; added `POST /approvals/{id}/activate` + a dedicated `ActivateProposalAsync` that only touches activation columns, plus a regression test. DEBT-029 discovered (not fixed — genuine cross-cutting scope, needs its own session): `LogAuditEventCommandHandler` is never called by ApprovalWorkflow/TradeExecution/SellDecision/PortfolioReconciliation, so the compliance audit trail (VS-27) is empty in production regardless of activity — VS-27's 'COMPLETED' status only reflects `AuditSql` being directly tested, not that anything actually calls it. See TECH_DEBT_REGISTER.md DEBT-029 for a suggested Outbox-consumer-based fix approach. `dotnet build -c Release` clean; DB-unverified." +AEG-VS-27-01,S2,VS-27,불변 감사 추적 구현 (Audit Trail / GDPR),BLOCKED,-,"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. ⚠️ Frontend UI built 2026-08-08 (frontend/src/features/audit-trail/, route /compliance/audit-trail, pnpm typecheck/build clean, 19 new tests passing) but on an isolated worktree branch (worktree-agent-a2cc5afe46a7e16b1) not yet merged into this branch — deprioritized behind BE work per user direction 2026-08-08; also flagged DEBT-025 (Compliance endpoints AllowAnonymous with no real RBAC, and no PermissionGuard component exists in this repo despite CLAUDE.md listing it as always-shared) and DEBT-026 (~130 stray committed .js files regenerate on every pnpm build) — note both DEBT-025 numbers collide with a different DEBT-025 added the same day on this branch (GET /approvals/{id}); renumber on merge. 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. 🔴 DOWNGRADED 2026-08-09 from COMPLETED to BLOCKED: DEBT-029 found that `LogAuditEventCommandHandler` (this slice's intended write path) is never called by ApprovalWorkflow/TradeExecution/SellDecision/PortfolioReconciliation — nothing in the running application populates `compliance.audit_events` outside of this file's own direct-`AuditSql` tests. 'Backend implementation + tests complete' above is true only for `AuditSql` in isolation, not for the audit trail actually existing in production. See TECH_DEBT_REGISTER.md DEBT-029 for the suggested fix (an Outbox-consumer hooked to each slice's existing state-change events, rather than direct calls threaded into every handler)." 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." AEG-X-038,S3,Cross,Reconfirm Fee/Tax/FX valid-time schedule decisions,BLOCKED,-,docs/CURRENT/AEG-X-038_BLOCKER.md,Ops/Tax,"2026-08-08: source-gap audit completed. AEG-X-009 dependency is complete, but no approved source authority, decision log, data contract, or temporal/preference rules were found. Legacy references include persisted execution Commission and an unapproved 0.001m fee default; neither is a valid-time schedule. REQ-COST-001/T-COST-PIT cannot be safely implemented until Ops/Tax and Compliance/Owner approve the decisions in the blocker record. No build/test/migration claimed." diff --git a/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Endpoints.cs b/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Endpoints.cs index 44686760..5879de13 100644 --- a/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Endpoints.cs +++ b/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Endpoints.cs @@ -90,6 +90,43 @@ public class ProposeForReviewEndpoint : EndpointWithoutRequest +/// DEBT-028: before this endpoint existed, ActivateModelHandler was registered in DI and fully +/// implemented but had no HTTP entry point — an Approved proposal could never reach Active, the +/// final step of the maker-checker gate this whole slice exists for. +/// +public class ActivateApprovalEndpoint : EndpointWithoutRequest +{ + private readonly ActivateModelHandler _handler; + private readonly ApprovalWorkflowSql _sql; + + public ActivateApprovalEndpoint(ActivateModelHandler handler, ApprovalWorkflowSql sql) + { + _handler = handler; + _sql = sql; + } + + public override void Configure() + { + Post("/approvals/{id}/activate"); + AllowAnonymous(); + } + + public override async Task HandleAsync(CancellationToken ct) + { + var proposalId = Route("id"); + var userEmail = HttpContext.User.FindFirst("email")?.Value ?? "anonymous"; + var userRole = HttpContext.User.FindFirst("role")?.Value ?? "Guest"; + + await _handler.Handle(proposalId, userEmail, userRole, Guid.NewGuid(), ct); + + var proposal = await _sql.GetProposalAsync(proposalId, ct); + await Send.OkAsync(new ActivateApprovalResponse(proposalId, "ACTIVE", proposal!.ActivatedAt), ct); + } +} + public record ApprovalDetailResponse(Guid Id, Guid ModelId, string Status, string CreatedBy, DateTime CreatedAt, string Justification, DateOnly EffectiveAt, string? ApprovedBy, DateTime? ApprovedAt, string? ApprovalNotes, string? ActivatedBy, DateTime? ActivatedAt, List Evidence); diff --git a/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Handlers.cs b/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Handlers.cs index 6c4722be..6f0a5cd4 100644 --- a/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Handlers.cs +++ b/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Handlers.cs @@ -139,7 +139,10 @@ public class ActivateModelHandler ApprovalWorkflowPolicy.ValidateProposalState(proposal.Status, ApprovalStatus.Active); - await _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Active, userEmail, "Model activated by SRE", ct); + // DEBT-028: UpdateProposalStatusAsync would overwrite approved_by/approval_notes with + // the activating SRE's identity and never set activated_by/activated_at. Use the + // dedicated activation method instead. + await _sql.ActivateProposalAsync(proposalId, userEmail, ct); var activateEvent = ApprovalWorkflowPolicy.CreateStateChangeEvent(proposalId, ApprovalStatus.Active, userEmail, correlationId, _clock.UtcNow.UtcDateTime, new Dictionary { { "effectiveAt", proposal.EffectiveAt.ToString("O") } }); diff --git a/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Sql.cs b/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Sql.cs index c95a7e59..dbd55281 100644 --- a/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Sql.cs +++ b/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/Sql.cs @@ -99,6 +99,26 @@ public class ApprovalWorkflowSql }); } + /// + /// DEBT-028: activation must NOT go through UpdateProposalStatusAsync — that method's + /// approvedBy/approvalNotes parameters would overwrite the checker's approved_by/ + /// approval_notes with the activating SRE's identity, and it never touches + /// activated_by/activated_at at all (those columns exist in the schema but nothing wrote + /// them). This is a dedicated method so activation only touches activation-specific columns. + /// + public async Task ActivateProposalAsync(Guid proposalId, string activatedBy, CancellationToken ct = default) + { + const string sql = """ + UPDATE model_operations.approval_proposals + SET status = 'ACTIVE', activated_by = @activatedBy, activated_at = NOW(), + published_at = NOW(), revision = revision + 1 + WHERE id = @proposalId + """; + + using var conn = new NpgsqlConnection(_connectionString); + await conn.ExecuteAsync(sql, new { proposalId, activatedBy }); + } + public async Task> GetEvidenceForProposalAsync(Guid proposalId, CancellationToken ct = default) { const string sql = """ diff --git a/tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs b/tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs index 60bb0aa4..bb07b0b9 100644 --- a/tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs +++ b/tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs @@ -238,9 +238,12 @@ public class ApprovalWorkflowTests : IAsyncLifetime } [Fact] - public async Task Activate_BySreAfterApproval_TransitionsToActive() + public async Task Activate_BySreAfterApproval_TransitionsToActive_AndPreservesApprovalFields() { - // Arrange + // Arrange: DEBT-028 regression guard — ActivateModelHandler used to call + // UpdateProposalStatusAsync with the activating SRE's email as the "approvedBy" + // parameter, which overwrote the checker's approved_by/approval_notes, and never set + // activated_by/activated_at at all despite those columns existing in the schema. var modelId = await SeedModelAsync(); var createHandler = new CreateApprovalProposalHandler(_sql, _clock); var proposalId = await createHandler.Handle( @@ -258,6 +261,11 @@ public class ApprovalWorkflowTests : IAsyncLifetime var proposal = await _sql.GetProposalAsync(proposalId); Assert.NotNull(proposal); Assert.Equal(ApprovalStatus.Active, proposal!.Status); + Assert.Equal("sre@company.com", proposal.ActivatedBy); + Assert.NotNull(proposal.ActivatedAt); + // The checker's approval record must survive activation, not be overwritten by the SRE's identity. + Assert.Equal("checker@company.com", proposal.ApprovedBy); + Assert.Equal("Approved", proposal.ApprovalNotes); } [Fact]