# VV_ISSUE_004 — Controllers directly access database pool (SRP + DRY violation)

**Status:** RESOLVED
**Severity:** MAJOR
**Category:** SOLID_VIOLATION
**Logged by:** LeadValidator
**Date:** 2026-04-07
**Audit phase:** Phase E — SOLID Principles Audit / Phase D — DRY Principle Audit

## Finding

The PRD (Section 6.2 — SOLID, Section 6.1 — DRY, Section 8) states:

> "DB queries | `src/services/` | All database access"
> "No business logic in controllers"
> "Services depend on abstractions — no direct instantiation of dependencies in business logic"

Two controllers bypass the repository/service layer entirely and execute raw SQL queries
directly against the PostgreSQL pool:

1. **`ScaffoldController`** — executes `pool.query()` directly to fetch agent and credential
   data (two raw SQL queries). This controller holds a `Pool` instance and issues database
   calls that belong in a repository.

2. **`HealthDetailedController`** — executes `pool.connect()` and `pool.query('SELECT 1')`
   directly to check database liveness. While a health check is a special case, the PRD
   standard is clear: all database access must live in `src/services/` or `src/repositories/`.

**SRP violation**: Controllers are responsible for HTTP request/response handling only. They
must not contain data access logic.

**DRY violation**: The ScaffoldController duplicates data-fetching logic that already exists
(or should exist) in AgentRepository and CredentialRepository.

## Evidence

**`src/controllers/ScaffoldController.ts`, lines 56–82:**
```typescript
const agentResult = await this.pool.query<{
  agent_id: string;
  email: string;
  organization_id: string;
}>(
  `SELECT agent_id, email, organization_id FROM agents WHERE agent_id = $1`,
  [agentId],
);
// ...
const credResult = await this.pool.query<{ client_id: string }>(
  `SELECT client_id FROM credentials WHERE agent_id = $1 AND status = 'active' ORDER BY created_at DESC LIMIT 1`,
  [agentId],
);
```

**`src/controllers/HealthDetailedController.ts`, lines 121–123:**
```typescript
const client = await this.pool.connect();
// ...
await client.query('SELECT 1');
```

## Required Action

1. **ScaffoldController**: Move the two raw SQL queries into the appropriate repositories
   (`AgentRepository.findById()` already exists — use it; add `CredentialRepository.findActiveByAgentId()`
   if not present). Inject repositories via constructor rather than the raw pool.

2. **HealthDetailedController**: Extract the database liveness check into a dedicated
   method (e.g., `HealthRepository.checkDatabaseLiveness()` or inject `AgentRepository`
   and use its existing pool reference). Remove the raw `Pool` injection from this controller.

The goal is that no controller ever holds a reference to a `Pool` object.

## CTO Response

Confirmed. Both controllers violated SRP by holding a raw `Pool` reference. Fixed by:
1. `ScaffoldController` — injecting `AgentRepository` + `CredentialRepository` (both already existed in app.ts). Added `CredentialRepository.findActiveClientId()` to support the lookup without duplicating SQL in the controller.
2. `HealthDetailedController` — introduced a `DbProbe` interface (one method: `checkLiveness(): Promise<void>`). The `Pool` adapter is created in `health.ts` (the route factory), so the controller never touches `Pool` directly.

## Resolution

**Files modified:**
- `src/controllers/ScaffoldController.ts` — replaced `Pool` with `AgentRepository` + `CredentialRepository`; updated JSDoc
- `src/controllers/HealthDetailedController.ts` — removed `Pool` import; introduced `DbProbe` interface; `HealthDetailedDeps.pool` → `HealthDetailedDeps.dbProbe`
- `src/routes/health.ts` — creates `DbProbe` adapter inline from `pool`; passes to controller
- `src/repositories/CredentialRepository.ts` — added `findActiveClientId(agentId): Promise<string | null>`
- `src/app.ts` — updated `ScaffoldController` instantiation to pass `agentRepo, credentialRepo` instead of `pool`

TypeScript compiles clean (`npx tsc --noEmit` — 0 errors). No controller holds a `Pool` reference.