sentryagent-idp/openspec/vv_audit/VV_ISSUE_004.md

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:

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:

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.