# Phase 0 Research — SP-05 Agent Management & Investor Logic

**Branch**: `006-agent-investor` | **Date**: 2026-06-15
**Spec**: [spec.md](spec.md)
**Plan**: [plan.md](plan.md)

This document captures the research conducted during Phase 0 of `/speckit.plan` to resolve all technical unknowns and confirm best practices for SP-05.

---

## R-001: Pessimistic Row Lock (`lockForUpdate()`) in Laravel + PostgreSQL

**Decision**: Use Laravel's `Model::lockForUpdate()` (which emits `SELECT ... FOR UPDATE` in PostgreSQL) at the start of the PATCH/DELETE share-log transaction, immediately after fetching the target row by primary key.

**Rationale**:
- PostgreSQL's `SELECT ... FOR UPDATE` is the canonical, ACID-compliant way to serialize concurrent writes on a single row.
- Laravel's `lockForUpdate()` method maps directly to `FOR UPDATE` and integrates with `DB::transaction()`.
- The lock is held for the duration of the transaction; the second writer blocks until the first commits or rolls back.
- This satisfies the Q1 clarification (SP-05-NFR-D-006) and the AC-039 concurrency test.

**Alternatives considered**:
- **Optimistic concurrency control (`version` column)**: rejected — adds schema noise and Eloquent event complexity for a low-frequency operation. Pessimistic locking is sufficient because the share-log table has only 1 row locked per transaction.
- **PostgreSQL trigger that raises on `status != 'active'`**: rejected — moves business logic to the database layer, violating the project's clean-architecture posture.
- **No special handling (last-write-wins)**: rejected — allows two concurrent PATCHes to both succeed, breaking the "most recent active" invariant.

**Implementation sketch**:
```php
DB::transaction(function () use ($shareLogId) {
    $row = AgentSharesLog::lockForUpdate()->findOrFail($shareLogId);

    // Re-evaluate guards (after lock is acquired)
    $latest = AgentSharesLog::where('client_id', $row->client_id)
        ->where('status', 'active')
        ->orderByDesc('created_at')->orderByDesc('id')
        ->first();
    if ($latest->id !== $row->id) {
        throw new ShareNotLatestException();
    }
    if ($row->created_at->lt(now()->subDays(30))) {
        throw new ShareLockPeriodExpiredException();
    }

    // Apply mutation
    $row->update([...]);
});
```

---

## R-002: JSONB Flag Updates with Transaction Safety

**Decision**: All flag mutations (`agent`, `investor`) MUST be inside the same `DB::transaction()` as the triggering write. Use the existing `Client::markAsAgent()` and `Client::markAsInvestor()` methods (defined in SP-03, `07_01` Model Conventions) which use `save()` (not `saveQuietly()`).

**Rationale**:
- The Constitution's "NEVER `saveQuietly()` for flag updates" rule exists because quiet saves skip model events (auditing, observers) — bad for financial integrity.
- The `markAsRole()` method (existing from SP-03) is idempotent: if the flag is already present, it does not write a new row.
- PostgreSQL JSONB is fine for atomic append operations; the `client_type_flags` column has a `DEFAULT '[]'` and the GIN index (`idx_clients_type_flags`) supports the containment query.

**Alternatives considered**:
- **JSONB merge via raw SQL**: rejected — bypasses the Eloquent lifecycle; loses event firing.
- **Separate `role` enum column**: rejected — explicitly prohibited by `01_03` BR-001-3 ("No separate type or role column exists. Use flags.").

**Implementation note**:
The `markAsInvestor()` call is invoked from inside `AgentService::create` and `AgentSharesService::addShares` only. It is NEVER invoked from a withdrawal, a PATCH, or a DELETE — this protects the "permanent flag" invariant (BR-001-6, Q2 follow-up).

---

## R-003: PostgreSQL `FOR UPDATE` Semantics & Deadlock Avoidance

**Decision**: When a PATCH/DELETE share-log request arrives, acquire the lock on the target row in primary-key order. The single-row lock is short-lived and the second writer blocks (not deadlocks) because the locks are on distinct rows when targets differ.

**Rationale**:
- PostgreSQL `SELECT ... FOR UPDATE` on a single primary key is a row-level lock; it does not block reads (`FOR SHARE` or no-clause) or writes to other rows.
- The "second writer waits for the first" semantics produce a clean serialization. If the first commits successfully, the second re-evaluates the guards and is rejected with 403.
- Deadlock risk: only if two transactions try to lock multiple rows in opposite orders. SP-05 only locks one row per transaction → no deadlock.

**Alternatives considered**:
- **`FOR UPDATE NOWAIT`**: rejected — would cause the second writer to fail immediately, producing a confusing error.
- **Advisory locks (`pg_advisory_lock`)**: rejected — adds a separate lock-namespace that must be manually managed; over-engineering for this case.

---

## R-004: Computed Field Patterns (Computed-Not-Stored) without DTOs

**Decision**: Use Eloquent's query builder with raw aggregations wrapped in a dedicated `AgentSummaryService`. Return associative arrays (or a typed value object) — do NOT create a DTO unless the same shape is reused ≥ 3 times.

**Rationale**:
- Constitution §I: "A DTO class is permitted only when it serves a clear, documented purpose — not a default layer."
- For SP-05, the summary is consumed only by `AgentDetailResource` (one consumer). Direct array return is sufficient.
- If a future Spec needs the same shape (e.g., a quarterly investor report), a DTO can be introduced then.

**Implementation sketch**:
```php
// AgentSummaryService::getSummary(int $agentId): array
public function getSummary(int $agentId): array
{
    $client = Client::findOrFail($agentId);

    return [
        'total_shares' => $this->computeTotalShares($agentId),
        'total_installments_via_agent' => $this->computeTotalInstallments($agentId),
        'total_collected_via_agent' => $this->computeCollected($agentId),
        'total_remaining_via_agent' => $this->computeRemaining($agentId),
        'computed_profit' => $this->computeProfit($agentId),
        'referred_customers' => $this->loadReferredCustomers($agentId),
    ];
}

private function computeProfit(int $agentId): string
{
    $sum = DB::table('contracts')
        ->where('agent_id', $agentId)
        ->whereIn('status', ['active', 'completed'])
        ->selectRaw('SUM(total_after_profit - purchase_amount) AS profit')
        ->value('profit') ?? '0';
    return bcmul($sum, '0.05', 2);
}
```

**Alternatives considered**:
- **Eloquent relationships + collection aggregation**: rejected — produces N+1 queries for the referred-customers list.
- **Cached snapshot in `agent_summaries` table**: rejected — explicit violation of the "computed, not stored" principle (BR-005-3 "No table stores investor profits").
- **Single DTO `AgentSummaryDTO`**: deferred — revisit if a second consumer surfaces.

---

## R-005: RefreshCustomerListingJob Trigger Coordination

**Decision**: Dispatch the `RefreshCustomerListingJob` (existing artifact from SP-04) whenever a `client_type_flags` mutation occurs in the agent domain AND the resulting flag set includes `customer`. For pure agent/investor operations (where the client was not previously a customer), the dispatch is OPTIONAL but RECOMMENDED for safety.

**Rationale**:
- The customer_listing_mv (`04_02_MATERIALIZED_VIEWS.md`) is filtered by `WHERE 'customer' = ANY(client_type_flags)`. Any change to a client's `customer` flag membership invalidates a row in the MV.
- Agent-creation and share-add operations that target a client *who was already a customer* do not change the `customer` flag, so MV is unaffected.
- Agent-creation and share-add operations that target a client *who was not a customer* do not change the `customer` flag either, so MV is unaffected.
- Conclusion: in SP-05, the only path that could change the `customer` flag is `markAsCustomer()`, which is NOT triggered by agent operations. **SP-05 does not need to dispatch `RefreshCustomerListingJob` at all.**
- For safety / future-proofing, the implementation MAY dispatch the job unconditionally in `AgentService` and `AgentSharesService`; this adds ≤ 100ms to the write path and provides defense-in-depth.

**Alternatives considered**:
- **Always dispatch on every flag mutation (no conditional check)**: chosen for safety — the performance overhead is negligible (job is queued) and the correctness risk of missing a refresh is high.
- **Listen to model events (`Client::saved`) globally**: rejected — would dispatch on every save, including non-flag saves; noisy.
- **Skip the dispatch entirely**: rejected for SP-05 since the impact analysis shows zero flag changes to `customer`; defensible position but adds risk for future code paths.

**Implementation**: At the end of `AgentService::create` and `AgentSharesService::addShares`, after the transaction commits:
```php
RefreshCustomerListingJob::dispatch(); // queued, non-blocking
```

---

## R-006 (Bonus): Testing the 30-Day Boundary

**Decision**: Use `Carbon::setTestNow()` in feature tests to control "now" relative to the share log row's `created_at`. Test three cases:
1. `created_at = now() − 29 days` → within window, PATCH succeeds
2. `created_at = now() − 30 days` → exactly at boundary, PATCH succeeds (inclusive per Q3)
3. `created_at = now() − 31 days` → beyond window, PATCH returns 403

**Rationale**: The boundary semantics were clarified in Q3 (24-hour inclusive). Test must exercise all three points to lock the behavior.

**Implementation sketch**:
```php
public function test_modify_within_30_days_succeeds(): void
{
    $agent = $this->createAgentWithShares();
    Carbon::setTestNow(now()->addDays(29));
    $this->patchJson(...)->assertOk();
}

public function test_modify_at_exactly_30_days_succeeds(): void
{
    // ...
    Carbon::setTestNow(now()->addDays(30));
    $this->patchJson(...)->assertOk();
}

public function test_modify_after_30_days_rejected(): void
{
    // ...
    Carbon::setTestNow(now()->addDays(31));
    $this->patchJson(...)->assertStatus(403)->assertJson([...]);
}
```

---

## R-007 (Bonus): Cursor Pagination with Multi-Field Sort

**Decision**: Use Laravel's `cursorPaginate()` with a secondary sort key for stability (per the spec's `customer_listing_mv` pattern from `04_02_MATERIALIZED_VIEWS.md`).

**Rationale**:
- Cursor pagination is mandated by `07_06_API_CONSTRAINTS.md`.
- For multi-field sort (e.g., `name ASC` for agents, `created_at DESC` for shares log), the secondary sort by `id` ensures stable ordering when the primary sort key has ties.
- The `PaginationHelper::cursorMeta()` (existing from SP-03) extracts the metadata for the response.

**Implementation sketch**:
```php
// AgentController::index
$agents = Client::query()
    ->whereJsonContains('client_type_flags', 'agent')
    ->when($search, fn($q) => $q->where('name', 'ILIKE', "%{$search}%"))
    ->orderBy('name', 'asc')
    ->orderBy('id', 'asc')  // stability
    ->cursorPaginate($request->input('per_page', 20));

return success(
    data: AgentListResource::collection($agents)->resolve(),
    meta: cursorMeta($agents)
);
```

---

## Summary of Phase 0 Resolutions

| Research Item | Decision | Reference |
|---------------|----------|-----------|
| R-001 Pessimistic lock | `lockForUpdate()` on share log row | SP-05-NFR-D-006 |
| R-002 JSONB flag updates | `markAs*` inside `DB::transaction()` | BR-001-5/6, AC-010 |
| R-003 Deadlock avoidance | Single-row locks; no multi-row lock ordering | PostgreSQL semantics |
| R-004 Computed fields | Direct query builder + `bcmath`; no DTO | Constitution §I |
| R-005 MV refresh | Optional unconditional dispatch for safety | SP-04 + clarification |
| R-006 Boundary test | `Carbon::setTestNow()` for 3 points | Q3 clarification |
| R-007 Pagination | `cursorPaginate()` + secondary sort by `id` | `07_06` |

**All NEEDS CLARIFICATION items resolved.** No items deferred to Phase 1.