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 <noreply@anthropic.com>
This commit is contained in:
@@ -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) |
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user