TenantAtlas/specs/110-ops-ux-enforcement/research.md

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.