# Phase 0 Research: Ops-UX Enforcement & Cleanup

This research document resolves implementation uncertainties for the enforcement spec and records key decisions.

## Decision 1 — Implement guards as Pest “architecture tests” (filesystem scan)

**Decision**: Implement Guard A/B/C as Pest tests that scan PHP source files in `app/` (and for Guard C also `tests/`) and fail CI with actionable file + snippet output.

**Rationale**:

- Runs in the same CI pipeline as the rest of the test suite (no new tooling step).
- Enforcement is repo-wide and does not rely on developer discipline or code review memory.
- Can produce highly actionable failure output (file path + snippet) without external dependencies.

**Alternatives considered**:

- PHPStan custom rules: higher precision, but adds configuration + toolchain scope.
- `nikic/php-parser` AST scanning: high precision but adds a dependency (out of bounds per repo constraints).

## Decision 2 — Guard A detection uses tokenizer-assisted scanning (not pure regex)

**Decision**: Implement Guard A using PHP’s built-in tokenizer (`token_get_all()`) to detect `->update([...])` call sites and then inspect the array literal for `status` / `outcome` keys.

**Rationale**:

- Pure multi-line regex is brittle with nested arrays, comments, and strings.
- Tokenization gives a dependency-free way to avoid common false positives and to compute an approximate line number.
- The spec’s intended heuristic (“`->update([...])` block contains status/outcome keys”) maps naturally to tokens.

**Alternatives considered**:

- Regex-only `DOTALL` matching with string/comment skipping (`(*SKIP)(*F)`): simpler, but can still drift across the wrong closing bracket and become noisy.
- Manual char-by-char scanner after a simple “find start” regex: workable, but token-based is typically easier to maintain in PHP.

**Repo evidence**:

- Canonical status/outcome transitions live in `OperationRunService::updateRun()`.
- Violations exist in jobs/services that call `$operationRun->update([... 'status' => ..., 'outcome' => ...])` directly (enumerated in the spec).

## Decision 3 — Guard B flags “OperationRun in scope” + “DB notification emission”

**Decision**: Implement Guard B as a two-signal scan:

- **OperationRun signal**: file contains any of: `use App\\Models\\OperationRun;`, `OperationRun`, `$this->operationRun`, `$operationRun`.
- **DB emission signal**: file contains either `sendToDatabase(` or `->notify(`.
- **Allowlist**: only `app/Services/OperationRunService.php` and `app/Notifications/OperationRunCompleted.php`.

**Rationale**:

- Enforces the constitution: completion notifications are centralized, queued/running DB notifications are banned.
- Intentionally errs on the side of flagging new ad-hoc notification producers.

**Alternatives considered**:

- “DB emission only” = `sendToDatabase(` and ignore `->notify(`: lower noise, but would miss notify-based DB notifications.
- Restrict scan to `app/Jobs/**` and `app/Services/**`: lower noise, but spec requires `app/**/*.php` for defense-in-depth.

**Repo evidence (high-level)**:

- `OperationRunService` currently emits both `OperationRunCompleted` and `OperationRunQueued` via `->notify(...)`.
- There are multiple job files with `->sendToDatabase(...)` in code paths that also handle an OperationRun.
- There are Filament resource actions that both dispatch jobs with `$operationRun` and call `->sendToDatabase(...)` for queued feedback. These are queued DB notifications and conflict with FR-004.

## Decision 4 — Ban queued DB notifications by changing service defaults

**Decision**: Change `OperationRunService::dispatchOrFail(..., bool $emitQueuedNotification = true)` (and any upstream helper that takes `emitQueuedNotification`) so the default is `false`, and ensure no call site opts back into queued DB notifications.

**Rationale**:

- Matches FR-004 / FR-012b: queued/running DB notifications are forbidden repo-wide.
- Prevents new call sites from accidentally enabling queued DB notifications by omission.

**Alternatives considered**:

- Delete `OperationRunQueued` entirely: higher churn; leaving the class unused is acceptable as long as it’s never emitted.
- Keep default `true` but require opt-out everywhere: violates FR-012b and is easy to regress.

## Decision 5 — “Exactly once terminal notification” relies on `previousStatus` guard

**Decision**: Rely on `OperationRunService::updateRun()` behavior that emits `OperationRunCompleted` only when transitioning into `Completed` from a non-completed status.

**Rationale**:

- Centralizes “exactly once” semantics in a single place.
- Keeps jobs/services free of notification-specific logic.

**Alternatives considered**:

- Add additional DB-level dedupe: unnecessary if the service is the only producer and transitions are canonical.