ahmido/TenantAtlas
1
0
Fork 0
Code Issues Pull Requests 2 Actions Packages Projects 1 Releases Wiki Activity

spec(110): resolve scope, UI matrix, guard targets

Browse Source
This commit is contained in:
Ahmed Darrazi 2026-02-23 22:48:30 +01:00
parent b5166c02db
commit 3c5f4c209b
9 changed files with 570 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
.github/agents/copilot-instructions.md vendored
View File

@ -58,8 +58,8 @@ ## Code Style
PHP 8.4.15: Follow standard conventions
## Recent Changes
- 110-ops-ux-enforcement: Added PHP 8.4.x + Laravel 12, Filament v5, Livewire v4
- 109-review-pack-export: Added PHP 8.4 (Laravel 12) + Filament v5, Livewire v4, Laravel Framework v12
- 109-review-pack-export: Added [if applicable, e.g., PostgreSQL, CoreData, files or N/A]
- 109-review-pack-export: Added [if applicable, e.g., PostgreSQL, CoreData, files or N/A]
<!-- MANUAL ADDITIONS START -->
<!-- MANUAL ADDITIONS END -->

8
specs/110-ops-ux-enforcement/checklists/requirements.md
View File

@ -31,7 +31,7 @@ ## Feature Readiness
## Notes
- All items pass. Spec is ready for `/speckit.plan`.
- Scope violation tables provide concrete remediation targets (11 files across 4 violation categories).
- Guard tests (FR-012) are explicitly specced as static analysis (grep-based Pest), not runtime — no application booting required.
- P2 tasks (T110-040/041) are optional and explicitly gated; they do not block P0/P1 delivery.
- All items pass.
- Spec includes the mandatory UI Action Matrix and explicitly states “no new screens” while allowing targeted start-surface cleanup.
- Remediation targets are enumerated in the spec’s “Known Violations” tables; the executable task list is the single source of truth in `tasks.md`.
- Guard tests (FR-012) are specced as static analysis (filesystem scan) with explicit allowlist, so they fail fast with actionable output.

9
specs/110-ops-ux-enforcement/contracts/no-api-changes.md Normal file
View File

@ -0,0 +1,9 @@
# Contracts: No API Changes
This feature introduces **no new HTTP endpoints**, no new API resources, and no changes to existing request/response contracts.
Scope is limited to internal operation flow behavior:
- OperationRun status/outcome transitions must go through `OperationRunService`.
- Queued/running DB notifications are banned.
- Terminal completion notification is initiator-only and emitted exactly once.

39
specs/110-ops-ux-enforcement/data-model.md Normal file
View File

@ -0,0 +1,39 @@
# Phase 1 Design: Data Model (No Schema Changes)
This feature does not introduce schema changes. It enforces consistent usage of existing entities.
## Entity: OperationRun (`operation_runs`)
**Ownership/scoping**:
- Tenant-scoped operational artifact.
- Initiator user is optional (system/scheduled runs).
**Key fields (existing)**:
- `id`
- `workspace_id` / `tenant_id` (scoping)
- `user_id` (initiator; nullable)
- `type` (operation type string)
- `status` (`queued`/`running`/`completed`)
- `outcome` (terminal outcome; nullable until completed)
- `started_at`, `completed_at`
- `summary_counts` (JSON/array of numeric-only whitelisted keys)
- `failure_summary` (sanitized bounded array)
- `context` (additional metadata; mutable)
**Invariants enforced by this feature**:
- All transitions of `status` and `outcome` happen through `OperationRunService::updateRun()`.
- The only operation-related DB notification is the terminal `OperationRunCompleted`, emitted when transitioning into `completed` and only when `user_id` exists.
## Entity: Database Notifications (`notifications`)
**Ownership/scoping**:
- User-scoped records (`notifiable_type=User`), used for persistent notification audit.
**Invariants enforced by this feature**:
- No queued/running state notifications are persisted.
- Exactly one terminal operation completion notification is persisted per OperationRun + initiator.

157
specs/110-ops-ux-enforcement/plan.md Normal file
View File

@ -0,0 +1,157 @@
# Implementation Plan: Ops-UX Enforcement & Cleanup (Enterprise Standard Rollout)
**Branch**: `110-ops-ux-enforcement` | **Date**: 2026-02-23 | **Spec**: [specs/110-ops-ux-enforcement/spec.md](specs/110-ops-ux-enforcement/spec.md)
**Input**: Feature specification from `/specs/110-ops-ux-enforcement/spec.md`
**Note**: This template is filled in by the `/speckit.plan` command. See `.specify/scripts/` for helper scripts.
## Summary
Enforce the Ops-UX “3-surface contract” by removing non-canonical operation-run status/outcome transitions and banning queued/running DB notifications across in-scope operation flows.
Implementation is split into:
- Cleanup: move all terminal transitions to `OperationRunService`, remove job-level `sendToDatabase()` completion/queued notifications, and delete the legacy `RunStatusChangedNotification`.
- Enforcement: add Pest guard tests (filesystem scans) that fail CI when these patterns reappear.
- Verification: add focused regression tests asserting “exactly one terminal `OperationRunCompleted` notification for initiator; none for system runs; and zero queued/running notifications”.
## Technical Context
<!--
ACTION REQUIRED: Replace the content in this section with the technical details
for the project. The structure here is presented in advisory capacity to guide
the iteration process.
-->
**Language/Version**: PHP 8.4.x
**Primary Dependencies**: Laravel 12, Filament v5, Livewire v4
**Storage**: PostgreSQL (Sail)
**Testing**: Pest v4 (`vendor/bin/sail artisan test --compact`)
**Target Platform**: Docker via Laravel Sail (local); Dokploy (staging/prod)
**Project Type**: Laravel monolith (Filament admin panel)
**Performance Goals**: Guard tests run in < 1s locally; overall targeted test pack < 30s
**Constraints**:
- No schema changes and no new routes.
- Terminal DB notification is initiator-only and emitted exactly once.
- No queued/running DB notifications anywhere (including `OperationRunQueued`).
- Existing RBAC/capability gates for starting operations remain unchanged.
**Scale/Scope**: Repo-wide enforcement via guard tests; remediation limited to enumerated violating flows in the spec.
## Constitution Check
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
This spec is a cleanup/enforcement layer and does not introduce new screens, Graph calls, or operation types.
- Inventory-first / Read-write separation: PASS (no new inventory/backups semantics; no new write UX)
- Graph contract path: PASS (no Graph call additions)
- Deterministic capabilities: PASS (no capability model changes)
- RBAC-UX / isolation: PASS (no new routes or cross-plane access)
- Run observability: PASS (this spec strengthens dedupe + terminal notifications by removing ad-hoc notifications)
- Automation: PASS (no scheduling behavior changes; only notification/transition handling)
- Data minimization: PASS (removes notification spam; no payload storage changes)
- Badge semantics: PASS (no new badges)
- Filament UI contracts: PASS (no new screens; targeted updates to existing start surfaces only). The spec includes a mandatory UI Action Matrix.
## Project Structure
### Documentation (this feature)
```text
specs/110-ops-ux-enforcement/
├── plan.md # This file (/speckit.plan command output)
├── research.md # Phase 0 output (/speckit.plan command)
├── data-model.md # Phase 1 output (/speckit.plan command)
├── quickstart.md # Phase 1 output (/speckit.plan command)
├── contracts/ # Phase 1 output (/speckit.plan command)
└── tasks.md # Phase 2 output (/speckit.tasks command - NOT created by /speckit.plan)
```
### Source Code (repository root)
<!--
ACTION REQUIRED: Replace the placeholder tree below with the concrete layout
for this feature. Delete unused options and expand the chosen structure with
real paths (e.g., apps/admin, packages/something). The delivered plan must
not include Option labels.
-->
```text
app/
├── Jobs/
├── Notifications/
├── Services/
└── Support/
config/
database/
├── migrations/
└── factories/
resources/
├── views/
└── css/
routes/
tests/
└── Feature/
```
**Structure Decision**: Laravel monolith. Enforcement is implemented as:
- Targeted production code edits under `app/` (jobs/services/notifications).
- Guard + regression tests under `tests/Feature/OpsUx/**`.
## Complexity Tracking
> **Fill ONLY if Constitution Check has violations that must be justified**
| Violation | Why Needed | Simpler Alternative Rejected Because |
|-----------|------------|-------------------------------------|
| [e.g., 4th project] | [current need] | [why 3 projects insufficient] |
| [e.g., Repository pattern] | [specific problem] | [why direct DB access insufficient] |
## Phase 0 — Outline & Research
**Inputs**: [specs/110-ops-ux-enforcement/spec.md](specs/110-ops-ux-enforcement/spec.md), [constitution](.specify/memory/constitution.md)
Research is captured in [specs/110-ops-ux-enforcement/research.md](specs/110-ops-ux-enforcement/research.md) and resolves:
- Guard-test heuristics (regex/scan approach) to keep false positives low.
- Where/how queued DB notifications are emitted today, and how to disable them by default.
- Preferred test strategy for “exactly once terminal notification”.
## Phase 1 — Design & Contracts
Design artifacts are captured in:
- [specs/110-ops-ux-enforcement/data-model.md](specs/110-ops-ux-enforcement/data-model.md) (no schema changes; key entity behavior)
- [specs/110-ops-ux-enforcement/contracts/](specs/110-ops-ux-enforcement/contracts/) (no new API endpoints; documented as “no contract changes”)
- [specs/110-ops-ux-enforcement/quickstart.md](specs/110-ops-ux-enforcement/quickstart.md) (how to run the focused tests)
**Design highlights**:
- The only terminal DB notification is `OperationRunCompleted`, emitted from `OperationRunService` when an initiator exists.
- Guard A scans `app/**/*.php` for `->update([...])` arrays that include `status` and/or `outcome` keys, excluding `OperationRunService`.
- Guard B scans `app/**/*.php` for files that both reference an OperationRun signal and emit DB notifications (`sendToDatabase(` or `->notify(`), with a strict allowlist.
- Guard C ensures the legacy notification class is fully removed.
## Phase 2 — Execution Plan (Implementation)
Work is implemented in small, reviewable steps:
1. **P0 fixes (silent completions)**: replace direct terminal updates with `OperationRunService` transitions in the enumerated services/jobs.
2. **P0 fixes (notification spam)**: remove queued/running and custom completion DB notifications from the enumerated jobs.
3. **P0 fixes (legacy removal)**: delete `RunStatusChangedNotification` and remove any invocation.
4. **Enforcement**: add Pest guard tests (A/B/C) + minimal allowlist.
5. **Verification**: add focused regression tests for the key flows (inventory sync, retention, backup schedule run) proving the “exactly once terminal notification” contract.
**Out of scope**: new UI pages, schema changes, new operation types, Graph contract changes.
## Constitution Check (Post-Phase 1 Re-check)
- PASS: No new routes, no Graph calls, no new Filament screens.
- PASS: Strengthens Run Observability by centralizing terminal notification emission.
- PASS: RBAC-UX/isolation unaffected (no new access paths).

23
specs/110-ops-ux-enforcement/quickstart.md Normal file
View File

@ -0,0 +1,23 @@
# Quickstart: Ops-UX Enforcement & Cleanup
## Prereqs
- Sail running: `vendor/bin/sail up -d`
## Run the focused guard tests
Once implemented, run the guard tests only:
- `vendor/bin/sail artisan test --compact --filter=OpsUx`
If the tests are split by directory as in the spec:
- `vendor/bin/sail artisan test --compact tests/Feature/OpsUx/Constitution`
## Run the focused regression tests
- `vendor/bin/sail artisan test --compact tests/Feature/OpsUx/Regression`
## Format touched files
- `vendor/bin/sail bin pint --dirty --format agent`

89
specs/110-ops-ux-enforcement/research.md Normal file
View File

@ -0,0 +1,89 @@
# 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.

80
specs/110-ops-ux-enforcement/spec.md
View File

@ -5,10 +5,18 @@ # Feature Specification: Ops-UX Enforcement & Cleanup (Enterprise Standard Rollo
**Status**: Draft
**Depends On**: `specs/055-ops-ux-rollout/spec.md`
## Clarifications
### Session 2026-02-23
- Q: For Guard A, what detection strategy should we use to catch forbidden status/outcome transitions while allowing context-only updates? → A: Scan `app/` for `->update([...])` blocks that include `status` or `outcome` keys (same call block, multi-line), excluding `app/Services/OperationRunService.php`.
- Q: Should queued/running DB notifications be allowed (e.g., `OperationRunQueued`) or is the DB notification surface terminal-only? → A: Ban queued/running DB notifications entirely (including `OperationRunQueued`); the only DB notification for operations is the terminal `OperationRunCompleted` notification.
- Q: For Guard B (DB-notification guard), what scanning scope/heuristic should we use? → A: Scan `app/**/*.php` and fail when a file both references an OperationRun (import or variable/property usage) and emits any database notification (`sendToDatabase(` or `->notify(`), except for an explicit allowlist.
## Spec Scope Fields *(mandatory)*
- **Scope**: tenant (all operation-run-producing flows within a tenant)
- **Primary Routes**: No new routes. Affected internal flows: Inventory Sync, Backup Schedule Retention, Backup Schedule Run, Bulk Policy Export, Bulk Restore Run Force Delete, Add/Remove Policies to Backup Set, Restore Run execution.
- **Primary Routes**: No new routes. Affected internal flows: Inventory Sync, Backup Schedule Retention, Backup Schedule Run, Bulk Policy Export, Bulk Restore Run Restore, Bulk Restore Run Force Delete, Bulk Policy Unignore, Entra Group manual sync, Add/Remove Policies to Backup Set, Restore Run execution.
- **Data Ownership**: `operation_runs` (tenant-scoped), `notifications` (tenant user-scoped). No schema changes required.
- **RBAC**: No new RBAC surfaces. Existing capability gates on triggering operations remain unchanged. Notification delivery is limited to the initiator user. System/scheduled runs with no initiator receive no DB notification.
@ -104,7 +112,18 @@ ## Requirements *(mandatory)*
**Constitution alignment — OPS-UX-001**: This spec enforces the Ops-UX 3-surface contract. All status/outcome transitions must be routed through `OperationRunService`. Terminal DB notifications are sent exclusively via `OperationRunCompleted` from within the service. No job or feature code may send its own completion DB notification.
**Constitution alignment — No new Filament screens**: This spec makes no changes to Filament Resources, RelationManagers, or Pages (beyond the optional P2 presenter helper). The UI Action Matrix is not required.
**Constitution alignment — Filament scope**: This spec introduces **no new Filament screens**. It DOES include targeted updates to existing Filament start surfaces (Resources/Pages) to remove queued/running DB notifications and ensure start-surface feedback is toast-only.
**UI Action Matrix (mandatory)**
| Surface | User action | Authorization | Start surface feedback | Progress surface | Terminal surface |
|---------|-------------|---------------|------------------------|------------------|-----------------|
| `PolicyResource` (tenant context) | Trigger an operation that creates/reuses an OperationRun | Existing capability checks (unchanged) | Toast-only (no DB notification) + optional “View run” link | Monitoring → Operations + run detail | Exactly one `OperationRunCompleted` DB notification to initiator (service-owned) |
| `PolicyVersionResource` (tenant context) | Trigger an operation that creates/reuses an OperationRun | Existing capability checks (unchanged) | Toast-only (no DB notification) + optional “View run” link | Monitoring → Operations + run detail | Exactly one `OperationRunCompleted` DB notification to initiator (service-owned) |
| `BackupScheduleResource` (tenant context) | Run / queue backup schedule operation | Existing capability checks (unchanged) | Toast-only (no DB notification) + optional “View run” link | Monitoring → Operations + run detail | Exactly one `OperationRunCompleted` DB notification to initiator (service-owned) |
| `TenantResource` (tenant context) | Trigger an operation that creates/reuses an OperationRun | Existing capability checks (unchanged) | Toast-only (no DB notification) + optional “View run” link | Monitoring → Operations + run detail | Exactly one `OperationRunCompleted` DB notification to initiator (service-owned) |
| `EntraGroupResource` → `ListEntraGroups` (tenant context) | Start manual sync operation | Existing capability checks (unchanged) | Toast-only (no DB notification) + optional “View run” link | Monitoring → Operations + run detail | Exactly one `OperationRunCompleted` DB notification to initiator (service-owned) |
| Scheduled/system operations | Jobs run without an initiator | Existing schedule/lock semantics (unchanged) | N/A | Monitoring → Operations + run detail | No DB notification (initiator is null); Monitoring remains canonical |
**Constitution alignment — No new Graph calls / OperationRun types**: This spec modifies existing operation flows only; it does not introduce new operation types or Graph calls.
@ -115,18 +134,20 @@ ### Functional Requirements
- **FR-001**: All OperationRun `status` and `outcome` field transitions MUST go through `OperationRunService` canonical transition methods. Direct `$operationRun->update(['status' => ...])`, `$operationRun->status = ...`, `$operationRun->outcome = ...`, or bulk query updates on status/outcome are forbidden outside `OperationRunService`.
- **FR-002**: `OperationRunService` MUST emit exactly one `OperationRunCompleted` DB notification to the initiator when transitioning a run to a terminal outcome (when an initiator exists).
- **FR-003**: No job or service code outside `OperationRunService` MAY emit a DB notification representing operation completion, abort, or terminal state.
- **FR-004**: No code anywhere MAY emit a DB notification for queued or running operation states.
- **FR-004**: No code anywhere MAY emit a DB notification for queued or running operation states. This includes `OperationRunQueued` and any other “queued” / “started” / “running” database notifications.
- **FR-005**: `RunStatusChangedNotification` MUST be deleted. No references to it may remain in `app/` or `tests/`.
- **FR-006**: `InventorySyncService` MUST transition to terminal state exclusively via `OperationRunService`, not via direct model updates.
- **FR-007**: `ApplyBackupScheduleRetentionJob` MUST transition to terminal state exclusively via `OperationRunService`.
- **FR-008**: `TenantpilotBackfillWorkspaceIds` console command MUST use the canonical transition if it transitions OperationRun status (initiator may be null; no DB notification emitted in that case).
- **FR-009**: `RunBackupScheduleJob` MUST NOT emit any queued or completion DB notifications; outcome notification is handled by `OperationRunService` terminal transition.
- **FR-010**: `BulkPolicyExportJob`, `BulkRestoreRunForceDeleteJob`, `AddPoliciesToBackupSetJob`, and `RemovePoliciesFromBackupSetJob` MUST NOT call `sendToDatabase()` for operation completion or abort feedback.
- **FR-010**: `BulkPolicyExportJob`, `BulkRestoreRunForceDeleteJob`, `BulkRestoreRunRestoreJob`, `BulkPolicyUnignoreJob`, `AddPoliciesToBackupSetJob`, and `RemovePoliciesFromBackupSetJob` MUST NOT call `sendToDatabase()` for operation completion, abort, or queued/running feedback.
- **FR-010b**: Filament start surfaces that initiate operation-run-producing flows MUST NOT persist queued/running DB notifications (including any `sendToDatabase()` “queued” notifications). Start feedback is toast-only.
- **FR-011**: Context-only updates (e.g., updating `context`, `message`, `reason_code` fields without touching `status` or `outcome`) are permitted directly on the model outside `OperationRunService`.
- **FR-012**: Three Pest guard tests MUST exist and pass in CI:
- Guard A: Detects direct status/outcome transitions outside `OperationRunService`; reports file + snippet.
- Guard B: Detects `sendToDatabase()` calls in operation-flow jobs that also reference OperationRun; reports file path.
- Guard A: Detects direct status/outcome transitions outside `OperationRunService`; reports file + snippet. Implementation MUST scan `app/**/*.php` for `->update(` calls whose update array includes a `status` and/or `outcome` key (multi-line block match allowed), excluding `app/Services/OperationRunService.php`. Context-only updates without `status`/`outcome` MUST NOT fail this guard.
- Guard B: Detects DB-notification emissions in operation-flow code; reports file path. Implementation MUST scan `app/**/*.php` and fail when a file contains BOTH (a) an OperationRun signal (`use App\\Models\\OperationRun;` OR `OperationRun` token OR `$this->operationRun` OR `$operationRun`) AND (b) a DB-notification emission (`sendToDatabase(` OR `->notify(`). Allowed exceptions (explicit allowlist): `app/Services/OperationRunService.php`, `app/Notifications/OperationRunCompleted.php`.
- Guard C: Detects any reference to `RunStatusChangedNotification` in `app/` or `tests/`.
- **FR-012b**: Operation enqueue helpers MUST NOT emit queued DB notifications by default. Any helper param like `emitQueuedNotification` MUST default to `false`, and all current call sites MUST comply.
- **FR-013** *(P2)*: `OperationUxPresenter` MUST expose an `alreadyQueuedToast(...)` static helper returning canonical copy + duration (+ optional "View run" action).
- **FR-014** *(P2)*: `BackupSetPolicyPickerTable` dedup toast MUST use `OperationUxPresenter::alreadyQueuedToast(...)`.
@ -147,9 +168,21 @@ ### Job-level DB notifications (duplicates / queued spam)
| `app/Jobs/RunBackupScheduleJob.php` | Queued DB notification + custom finished notification | P0 |
| `app/Jobs/BulkPolicyExportJob.php` | Multiple `sendToDatabase()` paths | P1 |
| `app/Jobs/BulkRestoreRunForceDeleteJob.php` | Multiple `sendToDatabase()` paths | P1 |
| `app/Jobs/BulkRestoreRunRestoreJob.php` | Multiple `sendToDatabase()` paths | P1 |
| `app/Jobs/BulkPolicyUnignoreJob.php` | Multiple `sendToDatabase()` paths | P1 |
| `app/Jobs/AddPoliciesToBackupSetJob.php` | Custom completion DB notifications | P1 |
| `app/Jobs/RemovePoliciesFromBackupSetJob.php` | Custom completion DB notifications | P1 |
### Start-surface queued DB notifications (forbidden)
| File | Violation | Priority |
|------|-----------|----------|
| `app/Filament/Resources/PolicyResource.php` | Queued/running DB notification emitted from a start surface | P1 |
| `app/Filament/Resources/PolicyVersionResource.php` | Queued/running DB notification emitted from a start surface | P1 |
| `app/Filament/Resources/BackupScheduleResource.php` | Queued/running DB notification emitted from a start surface | P1 |
| `app/Filament/Resources/TenantResource.php` | Queued/running DB notification emitted from a start surface | P1 |
| `app/Filament/Resources/EntraGroupResource/Pages/ListEntraGroups.php` | Queued/running DB notification emitted from a start surface | P1 |
### Legacy notification outside system
| File | Violation | Priority |
@ -165,40 +198,9 @@ ### Optional polish (P2)
---
## Tasks
## Execution Plan Reference
### Phase 1 — P0: Fix silent completions
- [ ] T110-001 (P0) `InventorySyncService` — replace direct terminal `update([status/outcome])` with `OperationRunService` canonical transition method
- [ ] T110-002 (P0) `ApplyBackupScheduleRetentionJob` — replace direct terminal `update([...])` with canonical transition method
- [ ] T110-003 (P1) `TenantpilotBackfillWorkspaceIds` — replace direct status update with canonical transition method (initiator may be null)
### Phase 2 — P0: Remove legacy notification
- [ ] T110-010 (P0) `ExecuteRestoreRunJob` — remove `RunStatusChangedNotification` invocation; rely on service terminal notification
- [ ] T110-011 (P0) Delete `RunStatusChangedNotification` class; assert zero references remain
### Phase 3 — P0/P1: Remove job-level DB notifications
- [ ] T110-020 (P0) `RunBackupScheduleJob` — remove queued DB notification producer
- [ ] T110-021 (P0) `RunBackupScheduleJob` — remove custom finished DB notification producer
- [ ] T110-022 (P1) `BulkPolicyExportJob` — remove all `sendToDatabase()` branches for completion/abort
- [ ] T110-023 (P1) `BulkRestoreRunForceDeleteJob` — remove all completion/abort `sendToDatabase()` branches
- [ ] T110-024 (P1) `AddPoliciesToBackupSetJob` — remove custom completion/failure DB notifications
- [ ] T110-025 (P1) `RemovePoliciesFromBackupSetJob` — remove custom completion/failure DB notifications
### Phase 4 — Guards (mandatory)
- [ ] T110-030 (P0) Pest guard: No direct OperationRun status/outcome transitions outside OperationRunService (file + snippet report; context-only updates allowed)
- [ ] T110-031 (P0) Pest guard: Jobs must not emit `sendToDatabase()` in operation flows (allowlist escape hatch if needed, minimal)
- [ ] T110-032 (P0) Pest guard: No references to `RunStatusChangedNotification` in `app/` or `tests/`
### Phase 5 — Optional polish (P2)
- [ ] T110-040 (P2) Add `OperationUxPresenter::alreadyQueuedToast(...)` canonical helper
- [ ] T110-041 (P2) Migrate `BackupSetPolicyPickerTable` dedup toast to `alreadyQueuedToast`
---
The executable work breakdown for this spec lives in [specs/110-ops-ux-enforcement/tasks.md](specs/110-ops-ux-enforcement/tasks.md). The spec intentionally avoids duplicating a task list to prevent drift.
## Testing Plan (Pest)

207
specs/110-ops-ux-enforcement/tasks.md Normal file
View File

@ -0,0 +1,207 @@
---
description: "Executable task list for Ops-UX Enforcement & Cleanup"
---
# Tasks: Ops-UX Enforcement & Cleanup (Enterprise Standard Rollout)
**Input**: Design documents from `/specs/110-ops-ux-enforcement/`
- plan.md: [specs/110-ops-ux-enforcement/plan.md](specs/110-ops-ux-enforcement/plan.md)
- spec.md: [specs/110-ops-ux-enforcement/spec.md](specs/110-ops-ux-enforcement/spec.md)
- research.md: [specs/110-ops-ux-enforcement/research.md](specs/110-ops-ux-enforcement/research.md)
- data-model.md: [specs/110-ops-ux-enforcement/data-model.md](specs/110-ops-ux-enforcement/data-model.md)
- contracts/: [specs/110-ops-ux-enforcement/contracts/](specs/110-ops-ux-enforcement/contracts/)
**Tests**: REQUIRED (Pest) — both guard tests and focused regressions.
## Phase 1: Setup (Shared Test Infrastructure)
**Purpose**: Create minimal shared helpers for guard tests and keep failure output consistent.
- [ ] T001 [P] Add source scanning helper in tests/Support/OpsUx/SourceFileScanner.php
---
## Phase 2: Foundational (Blocking Prerequisites)
**Purpose**: Cross-cutting invariants that must be true before user story work can be considered “done”.
- [ ] T002 Update queued notification defaults in app/Services/OperationRunService.php (dispatchOrFail + enqueue helpers default emitQueuedNotification=false)
- [ ] T003 Confirm no call sites opt into queued DB notifications in app/Services/OperationRunService.php (remove/forbid emitQueuedNotification:true usage)
**Checkpoint**: No queued/running DB notifications can be emitted by default.
---
## Phase 3: User Story 1 — No Silent Completions (Priority: P1) 🎯 MVP
**Goal**: Terminal transitions always go through `OperationRunService`, producing exactly one terminal `OperationRunCompleted` notification for initiators.
**Independent Test**: Inventory sync + retention flow transitions to terminal and persists exactly one `OperationRunCompleted` notification for the initiator; system runs persist none.
### Tests (write first)
- [ ] T004 [P] [US1] Add inventory sync terminal notification regression test in tests/Feature/OpsUx/Regression/InventorySyncTerminalNotificationTest.php
- [ ] T005 [P] [US1] Add retention terminal notification regression test in tests/Feature/OpsUx/Regression/BackupRetentionTerminalNotificationTest.php
### Implementation
- [ ] T006 [US1] Refactor terminal transition in app/Services/Inventory/InventorySyncService.php to use OperationRunService::updateRun()
- [ ] T007 [US1] Refactor terminal transition in app/Jobs/ApplyBackupScheduleRetentionJob.php to use OperationRunService::updateRun()
- [ ] T008 [US1] Refactor OperationRun status/outcome update in app/Console/Commands/TenantpilotBackfillWorkspaceIds.php to use OperationRunService::updateRun() (initiator may be null)
**Checkpoint**: US1 regressions pass, with no silent completions.
---
## Phase 4: User Story 2 — No Notification Spam (Priority: P1)
**Goal**: Remove job-level queued/completion DB notifications and eliminate queued DB notifications from start surfaces.
**Independent Test**: Backup schedule run + representative bulk flow complete with exactly one terminal `OperationRunCompleted` and zero queued/running DB notifications.
### Tests (write first)
- [ ] T009 [P] [US2] Add backup schedule run notification regression test in tests/Feature/OpsUx/Regression/BackupScheduleRunNotificationTest.php
- [ ] T010 [P] [US2] Add bulk job “abort/circuit-break” regression test in tests/Feature/OpsUx/Regression/BulkJobCircuitBreakerTest.php
### Implementation (Jobs)
- [ ] T011 [P] [US2] Remove queued + custom finished DB notifications in app/Jobs/RunBackupScheduleJob.php
- [ ] T012 [P] [US2] Remove completion/abort sendToDatabase branches in app/Jobs/BulkPolicyExportJob.php
- [ ] T013 [P] [US2] Remove completion/abort sendToDatabase branches in app/Jobs/BulkRestoreRunForceDeleteJob.php
- [ ] T029 [P] [US2] Remove completion/abort sendToDatabase branches in app/Jobs/BulkRestoreRunRestoreJob.php
- [ ] T030 [P] [US2] Remove completion/abort sendToDatabase branches in app/Jobs/BulkPolicyUnignoreJob.php
- [ ] T014 [P] [US2] Remove custom completion/failure DB notifications in app/Jobs/AddPoliciesToBackupSetJob.php
- [ ] T015 [P] [US2] Remove custom completion/failure DB notifications in app/Jobs/RemovePoliciesFromBackupSetJob.php
### Implementation (Start surfaces / Filament)
- [ ] T016 [P] [US2] Replace queued DB notification with toast-only queued feedback in app/Filament/Resources/PolicyResource.php (remove sendToDatabase for queued ops)
- [ ] T017 [P] [US2] Replace queued DB notification with toast-only queued feedback in app/Filament/Resources/BackupScheduleResource.php (remove sendToDatabase for queued ops)
- [ ] T018 [P] [US2] Replace queued DB notification with toast-only queued feedback in app/Filament/Resources/TenantResource.php (remove sendToDatabase for queued ops)
- [ ] T019 [P] [US2] Replace queued DB notification with toast-only queued feedback in app/Filament/Resources/PolicyVersionResource.php (remove sendToDatabase for queued ops)
- [ ] T031 [P] [US2] Replace queued DB notification with toast-only queued feedback in app/Filament/Resources/EntraGroupResource/Pages/ListEntraGroups.php (remove sendToDatabase for queued ops)
**Checkpoint**: US2 regressions pass and notifications remain terminal-only.
---
## Phase 5: User Story 3 — Legacy Notification Removed (Priority: P1)
**Goal**: Remove the out-of-system `RunStatusChangedNotification` and rely exclusively on canonical terminal `OperationRunCompleted`.
**Independent Test**: Restore run completion produces exactly one `OperationRunCompleted` notification and there are zero references to `RunStatusChangedNotification` in `app/` and `tests/`.
### Tests (write first)
- [ ] T020 [P] [US3] Add restore run terminal notification regression test in tests/Feature/OpsUx/Regression/RestoreRunTerminalNotificationTest.php
### Implementation
- [ ] T021 [US3] Remove legacy notification invocation in app/Jobs/ExecuteRestoreRunJob.php
- [ ] T022 [US3] Delete legacy notification class app/Notifications/RunStatusChangedNotification.php
**Checkpoint**: US3 regression passes; no legacy notification remains.
---
## Phase 6: User Story 4 — Regression Guards Enforce the Constitution (Priority: P1)
**Goal**: CI guard tests fail fast when forbidden patterns reappear.
**Independent Test**: Guards fail with actionable output on a synthetic violation and pass on a clean codebase.
### Guard tests
- [ ] T023 [P] [US4] Implement Guard A in tests/Feature/OpsUx/Constitution/DirectStatusTransitionGuardTest.php (scan app/** for ->update([...]) containing status/outcome; exclude app/Services/OperationRunService.php; print snippet)
- [ ] T024 [P] [US4] Implement Guard B in tests/Feature/OpsUx/Constitution/JobDbNotificationGuardTest.php (scan app/** for OperationRun signal + DB notify emission; allowlist app/Services/OperationRunService.php and app/Notifications/OperationRunCompleted.php)
- [ ] T025 [P] [US4] Implement Guard C in tests/Feature/OpsUx/Constitution/LegacyNotificationGuardTest.php (scan app/** and tests/** for RunStatusChangedNotification)
**Checkpoint**: Guard tests pass green and provide clear failure output.
---
## Phase 7: User Story 5 — Canonical "Already Queued" Toast (Priority: P2)
**Goal**: Dedup “already queued” messaging is canonical and consistent.
**Independent Test**: Trigger dedup path and confirm toast uses `OperationUxPresenter::alreadyQueuedToast(...)`.
- [ ] T026 [P] [US5] Add OperationUxPresenter::alreadyQueuedToast(...) helper in app/Support/OpsUx/OperationUxPresenter.php
- [ ] T027 [US5] Migrate dedup toast to canonical helper in app/Livewire/BackupSetPolicyPickerTable.php
---
## Phase 8: Polish & Cross-Cutting Concerns
**Purpose**: Keep docs and developer workflow aligned with final test locations.
- [ ] T028 [P] Update quickstart commands/paths if needed in specs/110-ops-ux-enforcement/quickstart.md
---
## Dependencies & Execution Order
### User Story Completion Order
- US1 → US2 → US3 → US4 → US5
Rationale:
- US1–US3 remove known violations so the guard suite (US4) can pass on a clean codebase.
- US5 is optional polish and can land after enforcement is stable.
### Dependency Graph
```text
Phase 1 (Setup) ─┬─> Phase 2 (Foundational) ─┬─> US1 ─┬─> US2 ─┬─> US3 ─┬─> US4 ─┬─> US5
│ │ │ │ │ └─> Polish
└───────────────────────────┴────────┴────────┴────────┴───────────────
```
## Parallel Execution Examples
### US1 (tests + implementation)
```bash
T004: tests/Feature/OpsUx/Regression/InventorySyncTerminalNotificationTest.php
T005: tests/Feature/OpsUx/Regression/BackupRetentionTerminalNotificationTest.php
```
### US2 (jobs + start surfaces)
```bash
T011: app/Jobs/RunBackupScheduleJob.php
T012: app/Jobs/BulkPolicyExportJob.php
T013: app/Jobs/BulkRestoreRunForceDeleteJob.php
T014: app/Jobs/AddPoliciesToBackupSetJob.php
T015: app/Jobs/RemovePoliciesFromBackupSetJob.php
T016–T019: app/Filament/Resources/*Resource.php
```
### US4 (guards)
```bash
T023: tests/Feature/OpsUx/Constitution/DirectStatusTransitionGuardTest.php
T024: tests/Feature/OpsUx/Constitution/JobDbNotificationGuardTest.php
T025: tests/Feature/OpsUx/Constitution/LegacyNotificationGuardTest.php
```
## Implementation Strategy
### MVP Scope (recommended)
- Foundational (Phase 2) + US1 only.
This yields the highest-value guarantee quickly: “no silent completions” and exactly-once terminal notifications for the most critical flows.
### Incremental Delivery
1. Foundational → US1 → validate
2. US2 → validate
3. US3 → validate
4. US4 guards → enforce
5. US5 polish