Files
KArtSell.Aegis/src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/README.md
T
kjh2064 14e2cedc4f fix: resolve DEBT-017 duplicate ApprovalWorkflow implementation
Adopt Features/ApprovalWorkflow/ (wired into Program.cs, reachable over
HTTP) as the sole VS-26 (formerly VS-03) maker-checker approval slice.
Delete the dead, [DontRegister]'d duplicate under
ApprovalWorkflow/ (Workstream H) and its dedicated test file, which had
been misleadingly credited with "20/20 tests PASS" while being
unreachable at runtime.

- Sql.cs: fix the same Dapper DateOnly-parameter-binding bug that was
  already found and fixed in the now-deleted implementation
  (commit 2ccf74c) but had not been ported to this one; InsertProposalAsync
  would have failed 100% of the time against a real database.
- tests/.../ApprovalWorkflow/ApprovalWorkflowTests.cs: new Handler+Sql+
  real-Postgres integration coverage (create/approve/activate role
  gating, maker!=checker separation of duties, evidence attachment,
  DateOnly round-trip, list filtering) replacing the deleted dead-code
  suite at the same path.
- ApprovalWorkflowPolicyTests.cs: extended (5->10 cases) rather than
  replaced, since it already tested the kept implementation's Policy.
- Program.cs: drop the reference comment to the deleted namespace.
- TECH_DEBT_REGISTER.md: DEBT-017 marked Completed (DB verification
  pending); corrected stale DEBT-023 to point at this resolution;
  registered two residual gaps discovered (not introduced) by this
  cleanup as DEBT-025 (no GET /approvals/{id}, evidence unreachable via
  HTTP) and DEBT-026 (no wired Draft->Proposed transition, so the
  approve/activate path is currently unreachable end-to-end via HTTP).
- WBS_PROGRESS_TRACKER.csv / CURRENT_ROADMAP.md: AEG-VS-26-01 kept
  BLOCKED, not COMPLETED — no PostgreSQL was reachable in this session
  (127.0.0.1:5432 connection refused), so the 8 new integration tests
  are unverified; only the 10 pure-Policy tests were confirmed passing.

Cherry-picked cedc8d7/8c777df from docs/wbs-tracker-current-state onto
this worktree branch first, to bring in the VS-26 renumbering and
ADR-WBS-001 that this task's brief assumed already existed.

dotnet build -c Release: 0 errors/0 warnings.
dotnet test --filter "FullyQualifiedName~ApprovalWorkflow" -c Release:
10 passed (Policy, no DB), 15 failed (DB connection refused - includes
6 unrelated pre-existing tests matched by the filter substring).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-08 13:05:03 +09:00

9.6 KiB

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

DRAFT (created)
  ↓
PROPOSED (maker submits)
  ├→ APPROVED (checker approves)
  │    ↓
  │  ACTIVE (SRE activates)
  │
  └→ REJECTED (checker rejects)

RBAC Roles

  • Maker: Creates approval proposals (own proposals only)
  • Checker: Reviews and approves (must be different from Maker)
  • SRE: Activates approved proposals

Components

  1. ApprovalProposal (Domain Entity)

    • Model approval proposals with PIT tracking
    • Stores justification, effective date, approval notes
    • Immutable except for status transitions
  2. ApprovalWorkflowSql (Data Access)

    • Dapper queries for INSERT/SELECT operations
    • PIT tracking with correlation_id
    • No UPDATE/DELETE (append-only)
  3. ApprovalWorkflowPolicy (Domain Logic)

    • State machine validation
    • RBAC enforcement
    • Event generation
  4. Handlers (Application Layer)

    • CreateApprovalProposalHandler
    • ApproveApprovalHandler
    • ActivateModelHandler
    • Outbox events on each state change
  5. Endpoints (HTTP Layer)

    • 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

POST /approvals (Create Proposal)

Request:

{
  "modelId": "uuid",
  "effectiveAt": "2026-09-15",
  "justification": "Model passed OOS testing; PBO score 0.95"
}

Response (201):

{
  "id": "uuid",
  "status": "DRAFT",
  "createdAt": "2026-08-07T10:00:00Z"
}

GET /approvals (List Proposals)

Query Params:

  • status=PROPOSED (filter by status)
  • modelId=uuid (filter by model)
  • limit=50, offset=0 (pagination)

Response (200):

{
  "items": [
    {
      "id": "uuid",
      "modelId": "uuid",
      "status": "PROPOSED",
      "createdBy": "maker@company.com",
      "createdAt": "2026-08-07T10:00:00Z",
      "justification": "..."
    }
  ],
  "total": 1,
  "pages": 1
}

POST /approvals/{id}/approve (Approve Proposal)

Request:

{
  "approvalNotes": "PBO verified, OOS metrics acceptable",
  "evidence": [
    {"type": "PBO_SCORE", "url": "s3://evidence/pbo-0.95.json", "comment": "Confirmed"},
    {"type": "OOS_RETURN", "url": "s3://evidence/oos-returns.csv", "comment": "Acceptable"}
  ]
}

Response (200):

{
  "id": "uuid",
  "status": "APPROVED",
  "approvedAt": "2026-08-07T11:00:00Z"
}

Database Schema

approval_proposals

CREATE TABLE model_operations.approval_proposals (
  id UUID PRIMARY KEY,
  model_id UUID NOT NULL,
  status VARCHAR(50),  -- DRAFT, PROPOSED, APPROVED, ACTIVE, REJECTED
  created_by VARCHAR(255),
  created_at TIMESTAMPTZ,
  justification TEXT,
  effective_at DATE,
  proposed_at TIMESTAMPTZ,
  approved_by VARCHAR(255),
  approved_at TIMESTAMPTZ,
  approval_notes TEXT,
  activated_by VARCHAR(255),
  activated_at TIMESTAMPTZ,
  published_at TIMESTAMPTZ,
  revision INT,
  correlation_id UUID
);

approval_evidence

CREATE TABLE model_operations.approval_evidence (
  id UUID PRIMARY KEY,
  approval_proposal_id UUID NOT NULL,
  evidence_type VARCHAR(50),  -- PBO_SCORE, DSR_METRIC, OOS_RETURN, BACKTEST_REPORT
  evidence_url TEXT,
  reviewer_comment TEXT,
  published_at TIMESTAMPTZ,
  correlation_id UUID
);

approval_events

CREATE TABLE model_operations.approval_events (
  id UUID PRIMARY KEY,
  approval_proposal_id UUID NOT NULL,
  event_type VARCHAR(50),  -- CREATED, PROPOSED, APPROVED, REJECTED, ACTIVATED
  actor_email VARCHAR(255),
  event_at TIMESTAMPTZ,
  details JSONB,
  published_at TIMESTAMPTZ,
  correlation_id UUID
);

Tests

  • 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:

dotnet test --filter "FullyQualifiedName~ApprovalWorkflow" -c Release

AGENTS.md v16.0 Compliance

  • 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-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: 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: 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)
  • VS-00: PIT envelope (published_at, correlation_id, revision)
  • VS-02: Governance foundation (data sources, policies)
  • VS-27 (formerly VS-04): Audit trail (events logged by this slice)
  • Compliance: Maker-checker separation, evidence linkage

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 noreply@anthropic.com