Compare commits
3 Commits
dev
...
110-ops-ux
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
29112225b6 | ||
|
|
3c5f4c209b | ||
|
|
b5166c02db |
2
.github/agents/copilot-instructions.md
vendored
2
.github/agents/copilot-instructions.md
vendored
@ -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 -->
|
||||
|
||||
@ -1,11 +1,15 @@
|
||||
<!--
|
||||
Sync Impact Report
|
||||
|
||||
- Version change: 1.8.2 → 1.9.0
|
||||
- Version change: 1.9.0 → 1.10.0
|
||||
- Modified principles:
|
||||
- Filament UI — Action Surface Contract (NON-NEGOTIABLE) (tightened UX requirements; added layout/view/empty-state rules)
|
||||
- Operations / Run Observability Standard (clarified as non-negotiable 3-surface contract; added lifecycle, summary, guards, system-run policy)
|
||||
- Added sections:
|
||||
- Filament UI — Layout & Information Architecture Standards (UX-001)
|
||||
- Operations UX — 3-Surface Feedback (OPS-UX-055) (NON-NEGOTIABLE)
|
||||
- OperationRun lifecycle is service-owned (OPS-UX-LC-001)
|
||||
- Summary counts contract (OPS-UX-SUM-001)
|
||||
- Ops-UX regression guards are mandatory (OPS-UX-GUARD-001)
|
||||
- Scheduled/system runs (OPS-UX-SYS-001)
|
||||
- Removed sections: None
|
||||
- Templates requiring updates:
|
||||
- ✅ .specify/templates/plan-template.md
|
||||
@ -158,6 +162,72 @@ ### Operations / Run Observability Standard
|
||||
- Monitoring pages MUST be DB-only at render time (no external calls).
|
||||
- Start surfaces MUST NOT perform remote work inline; they only: authorize, create/reuse run (dedupe), enqueue work,
|
||||
confirm + “View run”.
|
||||
|
||||
### Operations UX — 3-Surface Feedback (OPS-UX-055) (NON-NEGOTIABLE)
|
||||
|
||||
If a feature creates/reuses `OperationRun`, it MUST use exactly three feedback surfaces — no others:
|
||||
|
||||
1) Toast (intent only / queued-only)
|
||||
- A toast MAY be shown only when the run is accepted/queued (intent feedback).
|
||||
- The toast MUST use `OperationUxPresenter::queuedToast($operationType)->send()`.
|
||||
- Feature code MUST NOT craft ad-hoc operation toasts.
|
||||
- A dedicated dedupe message MUST use the presenter (e.g., `alreadyQueuedToast(...)`), not `Notification::make()`.
|
||||
|
||||
2) Progress (active awareness only)
|
||||
- Live progress MUST exist only in:
|
||||
- the global active-ops widget, and
|
||||
- Monitoring → Operation Run Detail.
|
||||
- These surfaces MUST show only active runs (`queued|running`) and MUST never show terminal runs.
|
||||
- Determinate progress is allowed ONLY when `summary_counts.total` and `summary_counts.processed` are valid numeric values.
|
||||
- Determinate progress MUST be clamped to 0–100. Otherwise render indeterminate + elapsed time.
|
||||
- The widget MUST NOT show percentage text (optional `processed/total` is allowed).
|
||||
|
||||
3) Terminal DB Notification (audit outcome only)
|
||||
- Each run MUST emit exactly one persistent terminal DB notification when it becomes terminal.
|
||||
- Delivery MUST be initiator-only (no tenant-wide fan-out).
|
||||
- Completion notifications MUST be `OperationRunCompleted` only.
|
||||
- Feature code MUST NOT send custom completion DB notifications for operations (no `sendToDatabase()` for completion/abort).
|
||||
|
||||
Canonical navigation:
|
||||
- All “View run” links MUST use the canonical helper and MUST point to Monitoring → Operations → Run Detail.
|
||||
|
||||
### OperationRun lifecycle is service-owned (OPS-UX-LC-001)
|
||||
|
||||
Any change to `OperationRun.status` or `OperationRun.outcome` MUST go through `OperationRunService` (canonical transition method).
|
||||
This is the only allowed path because it enforces normalization, summary sanitization, idempotency, and terminal notification emission.
|
||||
|
||||
Forbidden outside `OperationRunService`:
|
||||
- `$operationRun->update(['status' => ...])` / `$operationRun->update(['outcome' => ...])`
|
||||
- `$operationRun->status = ...` / `$operationRun->outcome = ...`
|
||||
- Query-based updates that transition `status`/`outcome`
|
||||
|
||||
Allowed outside the service:
|
||||
- Updates to `context`, `message`, `reason_code` that do not change `status`/`outcome`.
|
||||
|
||||
### Summary counts contract (OPS-UX-SUM-001)
|
||||
|
||||
- `operation_runs.summary_counts` is the canonical metrics source for Ops-UX.
|
||||
- All keys MUST come from `OperationSummaryKeys::all()` (single source of truth).
|
||||
- Values MUST be flat numeric-only; no nested objects/arrays; no free-text.
|
||||
- Producers MUST NOT introduce new keys without:
|
||||
1) updating `OperationSummaryKeys::all()`,
|
||||
2) updating the spec canonical list,
|
||||
3) adding/adjusting tests.
|
||||
|
||||
### Ops-UX regression guards are mandatory (OPS-UX-GUARD-001)
|
||||
|
||||
The repo MUST include automated guards (Pest) that fail CI if:
|
||||
- any direct `OperationRun` status/outcome transition occurs outside `OperationRunService`,
|
||||
- jobs emit DB notifications for operation completion/abort (`OperationRunCompleted` is the single terminal notification),
|
||||
- deprecated legacy operation notification classes are referenced again.
|
||||
|
||||
These guards MUST fail with actionable output (file + snippet).
|
||||
|
||||
### Scheduled/system runs (OPS-UX-SYS-001)
|
||||
|
||||
- If a run has no initiator user, no terminal DB notification is emitted (initiator-only policy).
|
||||
- Outcomes remain auditable via Monitoring → Operations / Run Detail.
|
||||
- Any tenant-wide alerting MUST go through the Alerts system (not `OperationRun` notifications).
|
||||
- Active-run dedupe MUST be enforced at DB level (partial unique index/constraint for active states).
|
||||
- Failures MUST be stored as stable reason codes + sanitized messages; never persist secrets/tokens/PII/raw payload dumps
|
||||
in failures or notifications.
|
||||
@ -274,4 +344,4 @@ ### Versioning Policy (SemVer)
|
||||
- **MINOR**: new principle/section or materially expanded guidance.
|
||||
- **MAJOR**: removing/redefining principles in a backward-incompatible way.
|
||||
|
||||
**Version**: 1.9.0 | **Ratified**: 2026-01-03 | **Last Amended**: 2026-02-19
|
||||
**Version**: 1.10.0 | **Ratified**: 2026-01-03 | **Last Amended**: 2026-02-23
|
||||
|
||||
@ -41,6 +41,11 @@ ## Constitution Check
|
||||
- RBAC-UX: global search is tenant-scoped; non-members get no hints; inaccessible results are treated as not found (404 semantics)
|
||||
- Tenant isolation: all reads/writes tenant-scoped; cross-tenant views are explicit and access-checked
|
||||
- Run observability: long-running/remote/queued work creates/reuses `OperationRun`; start surfaces enqueue-only; Monitoring is DB-only; DB-only <2s actions may skip runs but security-relevant ones still audit-log; auth handshake exception OPS-EX-AUTH-001 allows synchronous outbound HTTP on `/auth/*` without `OperationRun`
|
||||
- Ops-UX 3-surface feedback: if `OperationRun` is used, feedback is exactly toast intent-only + progress surfaces + exactly-once terminal `OperationRunCompleted` (initiator-only); no queued/running DB notifications
|
||||
- Ops-UX lifecycle: `OperationRun.status` / `OperationRun.outcome` transitions are service-owned (only via `OperationRunService`); context-only updates allowed outside
|
||||
- Ops-UX summary counts: `summary_counts` keys come from `OperationSummaryKeys::all()` and values are flat numeric-only
|
||||
- Ops-UX guards: CI has regression guards that fail with actionable output (file + snippet) when these patterns regress
|
||||
- Ops-UX system runs: initiator-null runs emit no terminal DB notification; audit remains via Monitoring; tenant-wide alerting goes through Alerts (not OperationRun notifications)
|
||||
- Automation: queued/scheduled ops use locks + idempotency; handle 429/503 with backoff+jitter
|
||||
- Data minimization: Inventory stores metadata + whitelisted meta; logs contain no secrets/tokens
|
||||
- Badge semantics (BADGE-001): status-like badges use `BadgeCatalog` / `BadgeRenderer`; no ad-hoc mappings; new values include tests
|
||||
|
||||
@ -94,6 +94,13 @@ ## Requirements *(mandatory)*
|
||||
(preview/confirmation/audit), tenant isolation, run observability (`OperationRun` type/identity/visibility), and tests.
|
||||
If security-relevant DB-only actions intentionally skip `OperationRun`, the spec MUST describe `AuditLog` entries.
|
||||
|
||||
**Constitution alignment (OPS-UX):** If this feature creates/reuses an `OperationRun`, the spec MUST:
|
||||
- explicitly state compliance with the Ops-UX 3-surface feedback contract (toast intent-only, progress surfaces, terminal DB notification),
|
||||
- state that `OperationRun.status` / `OperationRun.outcome` transitions are service-owned (only via `OperationRunService`),
|
||||
- describe how `summary_counts` keys/values comply with `OperationSummaryKeys::all()` and numeric-only rules,
|
||||
- clarify scheduled/system-run behavior (initiator null → no terminal DB notification; audit is via Monitoring),
|
||||
- list which regression guard tests are added/updated to keep these rules enforceable in CI.
|
||||
|
||||
**Constitution alignment (RBAC-UX):** If this feature introduces or changes authorization behavior, the spec MUST:
|
||||
- state which authorization plane(s) are involved (tenant/admin `/admin` + tenant-context `/admin/t/{tenant}/...` vs platform `/system`),
|
||||
- ensure any cross-plane access is deny-as-not-found (404),
|
||||
|
||||
@ -14,6 +14,13 @@ # Tasks: [FEATURE NAME]
|
||||
If security-relevant DB-only actions skip `OperationRun`, include tasks for `AuditLog` entries (before/after + actor + tenant).
|
||||
Auth handshake exception (OPS-EX-AUTH-001): OIDC/SAML login handshakes may perform synchronous outbound HTTP on `/auth/*` endpoints
|
||||
without an `OperationRun`.
|
||||
If this feature creates/reuses an `OperationRun`, tasks MUST also include:
|
||||
- enforcing the Ops-UX 3-surface feedback contract (toast intent-only via `OperationUxPresenter`, progress only in widget + run detail, terminal notification is `OperationRunCompleted` exactly-once, initiator-only),
|
||||
- ensuring no queued/running DB notifications exist anywhere for operations (no `sendToDatabase()` for queued/running/completion/abort in feature code),
|
||||
- ensuring `OperationRun.status` / `OperationRun.outcome` transitions happen only via `OperationRunService`,
|
||||
- ensuring `summary_counts` keys come from `OperationSummaryKeys::all()` and values are flat numeric-only,
|
||||
- adding/updating Ops-UX regression guards (Pest) that fail CI with actionable output (file + snippet) when these patterns regress,
|
||||
- clarifying scheduled/system-run behavior (initiator null → no terminal DB notification; audit via Monitoring; tenant-wide alerting via Alerts system).
|
||||
**RBAC**: If this feature introduces or changes authorization, tasks MUST include:
|
||||
- explicit Gate/Policy enforcement for all mutation endpoints/actions,
|
||||
- explicit 404 vs 403 semantics:
|
||||
|
||||
37
specs/110-ops-ux-enforcement/checklists/requirements.md
Normal file
37
specs/110-ops-ux-enforcement/checklists/requirements.md
Normal file
@ -0,0 +1,37 @@
|
||||
# Specification Quality Checklist: Ops-UX Enforcement & Cleanup (Enterprise Standard Rollout)
|
||||
|
||||
**Purpose**: Validate specification completeness and quality before proceeding to planning
|
||||
**Created**: 2026-02-23
|
||||
**Feature**: [spec.md](../spec.md)
|
||||
|
||||
## Content Quality
|
||||
|
||||
- [x] No implementation details (languages, frameworks, APIs)
|
||||
- [x] Focused on user value and business needs
|
||||
- [x] Written for non-technical stakeholders
|
||||
- [x] All mandatory sections completed
|
||||
|
||||
## Requirement Completeness
|
||||
|
||||
- [x] No [NEEDS CLARIFICATION] markers remain
|
||||
- [x] Requirements are testable and unambiguous
|
||||
- [x] Success criteria are measurable
|
||||
- [x] Success criteria are technology-agnostic (no implementation details)
|
||||
- [x] All acceptance scenarios are defined
|
||||
- [x] Edge cases are identified
|
||||
- [x] Scope is clearly bounded
|
||||
- [x] Dependencies and assumptions identified
|
||||
|
||||
## Feature Readiness
|
||||
|
||||
- [x] All functional requirements have clear acceptance criteria
|
||||
- [x] User scenarios cover primary flows
|
||||
- [x] Feature meets measurable outcomes defined in Success Criteria
|
||||
- [x] No implementation details leak into specification
|
||||
|
||||
## Notes
|
||||
|
||||
- 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
9
specs/110-ops-ux-enforcement/contracts/no-api-changes.md
Normal 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
39
specs/110-ops-ux-enforcement/data-model.md
Normal 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
157
specs/110-ops-ux-enforcement/plan.md
Normal 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
23
specs/110-ops-ux-enforcement/quickstart.md
Normal 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
89
specs/110-ops-ux-enforcement/research.md
Normal 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.
|
||||
268
specs/110-ops-ux-enforcement/spec.md
Normal file
268
specs/110-ops-ux-enforcement/spec.md
Normal file
@ -0,0 +1,268 @@
|
||||
# Feature Specification: Ops-UX Enforcement & Cleanup (Enterprise Standard Rollout)
|
||||
|
||||
**Feature Branch**: `110-ops-ux-enforcement`
|
||||
**Created**: 2026-02-23
|
||||
**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 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.
|
||||
|
||||
*Canonical-view fields not applicable — no new views introduced.*
|
||||
|
||||
## User Scenarios & Testing *(mandatory)*
|
||||
|
||||
### User Story 1 — No Silent Completions (Priority: P1)
|
||||
|
||||
As a tenant admin, when I trigger any tracked operation (Inventory Sync, Retention, Backup Schedule Run), I always receive exactly one terminal DB notification upon completion, so I can audit the outcome without checking the Monitoring hub manually.
|
||||
|
||||
**Why this priority**: Silent completions break auditability — the core promise of the Ops-UX system. Missing terminal notifications mean admins have no persistent outcome record outside the Monitoring hub.
|
||||
|
||||
**Independent Test**: Trigger (or simulate) an inventory sync run to terminal state and assert exactly one `OperationRunCompleted` DB notification exists for the initiator.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** an `inventory_sync` OperationRun with an initiator, **When** `InventorySyncService` transitions it to a terminal outcome, **Then** exactly one `OperationRunCompleted` DB notification is persisted for the initiator user.
|
||||
2. **Given** an `apply_backup_retention` OperationRun with an initiator, **When** `ApplyBackupScheduleRetentionJob` completes (success or failure), **Then** exactly one `OperationRunCompleted` DB notification is persisted for the initiator user.
|
||||
3. **Given** an OperationRun with **no initiator** (system/scheduled run), **When** the run transitions to terminal, **Then** zero DB notifications are emitted.
|
||||
|
||||
---
|
||||
|
||||
### User Story 2 — No Notification Spam (Priority: P1)
|
||||
|
||||
As a tenant admin, I never receive duplicate completion DB notifications for a single run, and I never receive queued/running state DB notifications for any operation.
|
||||
|
||||
**Why this priority**: Notification spam erodes trust in the notification surface. Even one duplicate makes admins ignore notifications — defeating the entire audit layer.
|
||||
|
||||
**Independent Test**: Simulate a `RunBackupScheduleJob` to completion and assert zero queued/running DB notifications exist, and exactly one terminal DB notification.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** a backup schedule run job enqueued and completed, **When** all job code paths execute, **Then** zero queued/running DB notifications are persisted, and exactly one terminal `OperationRunCompleted` exists for the initiator.
|
||||
2. **Given** a `BulkPolicyExportJob` that reaches terminal state via any path (success / abort / circuit-break), **Then** exactly one terminal `OperationRunCompleted` DB notification exists and zero notifications from job-level `sendToDatabase()` calls.
|
||||
3. **Given** any bulk job that previously sent custom completion DB notifications (`BulkRestoreRunForceDeleteJob`, `AddPoliciesToBackupSetJob`, `RemovePoliciesFromBackupSetJob`), **When** the job completes, **Then** no job-level DB notifications are emitted.
|
||||
|
||||
---
|
||||
|
||||
### User Story 3 — Legacy Notification Removed (Priority: P1)
|
||||
|
||||
As a tenant admin running a restore operation, my completion feedback comes exclusively from the canonical `OperationRunCompleted` notification, not from a legacy `RunStatusChangedNotification` with inconsistent copy or link behavior.
|
||||
|
||||
**Why this priority**: The legacy class is an out-of-system notification that bypasses canonical delivery, creating inconsistent UX copy and a second notification channel that cannot be centrally controlled.
|
||||
|
||||
**Independent Test**: Confirm `RunStatusChangedNotification` class does not exist in `app/` and `ExecuteRestoreRunJob` no longer references it. Restore run completion produces exactly one `OperationRunCompleted`.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** `ExecuteRestoreRunJob` completes a restore run to terminal state, **Then** no `RunStatusChangedNotification` is dispatched, and exactly one `OperationRunCompleted` DB notification is persisted for the initiator.
|
||||
2. **Given** a developer searches `app/` and `tests/` for `RunStatusChangedNotification`, **Then** no results are found (class deleted, all references removed).
|
||||
|
||||
---
|
||||
|
||||
### User Story 4 — Regression Guards Enforce the Constitution (Priority: P1)
|
||||
|
||||
As a developer working on the repo, if I accidentally introduce a direct `$operationRun->update(['status' => ...])` outside `OperationRunService`, a CI guard test immediately fails with a clear file + snippet report so I know exactly what to fix.
|
||||
|
||||
**Why this priority**: Without automated guards, the enforcement degrades over time as new features are added. Guards are the only scalable way to maintain the constitution without manual code review on every PR.
|
||||
|
||||
**Independent Test**: Introduce a synthetic violation in a temp file, run the guard test, confirm it fails with actionable output. Remove the violation, confirm the test passes.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** the codebase contains a direct `$operationRun->update(['status' => ...])` outside `OperationRunService`, **When** the guard test runs, **Then** it fails and outputs the violating file path and snippet.
|
||||
2. **Given** a job file contains both an OperationRun reference and a `sendToDatabase()` call (not on the allowlist), **When** the DB-notification guard test runs, **Then** it fails with the file path.
|
||||
3. **Given** any file in `app/` or `tests/` references `RunStatusChangedNotification`, **When** the legacy-class guard test runs, **Then** it fails.
|
||||
4. **Given** the codebase has no violations, **When** all guard tests run, **Then** all pass green.
|
||||
|
||||
---
|
||||
|
||||
### User Story 5 — Canonical "Already Queued" Toast (Priority: P2)
|
||||
|
||||
As a tenant admin triggering an operation that is already queued, I receive a consistent, canonical "already queued" toast message rather than ad-hoc copy from individual feature components, so the experience is uniform across all operation types.
|
||||
|
||||
**Why this priority**: P2 polish — non-blocking, but addresses the last remaining ad-hoc UX copy point identified in the audit.
|
||||
|
||||
**Independent Test**: Trigger the "already queued" dedup path in `BackupSetPolicyPickerTable` and assert the toast uses the canonical presenter output.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** an operation is already queued, **When** a user attempts to queue it again via the policy picker, **Then** a toast is shown using `OperationUxPresenter::alreadyQueuedToast(...)` with canonical copy.
|
||||
|
||||
---
|
||||
|
||||
### Edge Cases
|
||||
|
||||
- What happens when a job fails with an unhandled exception — does the OperationRun get stuck in a non-terminal state? *(Assumption: existing job `failed()` callback or `finally` block already transitions via service; confirm during implementation per-file.)*
|
||||
- What happens when `OperationRunService` itself throws during terminal transition? *(Assumption: run stays non-terminal; pre-existing behavior outside scope of this spec.)*
|
||||
- What if an OperationRun has no initiator and a job previously sent a DB notification — will removing that path cause silence? *(Confirmed acceptable: system runs are auditable via Monitoring hub only.)*
|
||||
|
||||
## 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 — 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.
|
||||
|
||||
**Constitution alignment — BADGE-001**: No new status badge values introduced.
|
||||
|
||||
### 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. 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`, `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. 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(...)`.
|
||||
|
||||
## Scope (Known Violations — Remediation Targets)
|
||||
|
||||
### Status transition bypass (direct model update — silent completion)
|
||||
|
||||
| File | Violation | Priority |
|
||||
|------|-----------|----------|
|
||||
| `app/Services/Inventory/InventorySyncService.php` | Direct `update([status/outcome])` — silent completion | P0 |
|
||||
| `app/Jobs/ApplyBackupScheduleRetentionJob.php` | Direct `update([...])` — silent completion | P0 |
|
||||
| `app/Console/Commands/TenantpilotBackfillWorkspaceIds.php` | Direct status update | P1 |
|
||||
|
||||
### Job-level DB notifications (duplicates / queued spam)
|
||||
|
||||
| File | Violation | Priority |
|
||||
|------|-----------|----------|
|
||||
| `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 |
|
||||
|------|-----------|----------|
|
||||
| `app/Jobs/ExecuteRestoreRunJob.php` | References `RunStatusChangedNotification` | P0 |
|
||||
| `app/Notifications/RunStatusChangedNotification.php` | Class to delete | P0 |
|
||||
|
||||
### Optional polish (P2)
|
||||
|
||||
| File | Violation | Priority |
|
||||
|------|-----------|----------|
|
||||
| `app/Livewire/BackupSetPolicyPickerTable.php` | Ad-hoc "already queued" toast (non-canonical copy) | P2 |
|
||||
|
||||
---
|
||||
|
||||
## Execution Plan Reference
|
||||
|
||||
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)
|
||||
|
||||
### Guard tests (mandatory — CI enforcement layer)
|
||||
- `tests/Feature/OpsUx/Constitution/DirectStatusTransitionGuardTest.php`
|
||||
- `tests/Feature/OpsUx/Constitution/JobDbNotificationGuardTest.php`
|
||||
- `tests/Feature/OpsUx/Constitution/LegacyNotificationGuardTest.php`
|
||||
|
||||
### Regression tests (mandatory for P0 fixes)
|
||||
- `tests/Feature/OpsUx/Regression/InventorySyncTerminalNotificationTest.php` — exactly one `OperationRunCompleted` for initiator; none for system run
|
||||
- `tests/Feature/OpsUx/Regression/BackupRetentionTerminalNotificationTest.php` — exactly one `OperationRunCompleted`; no direct model transitions
|
||||
- `tests/Feature/OpsUx/Regression/BackupScheduleRunNotificationTest.php` — zero queued DB notifications; exactly one terminal notification
|
||||
- `tests/Feature/OpsUx/Regression/BulkJobCircuitBreakerTest.php` — circuit-breaker / abort path yields exactly one terminal `OperationRunCompleted` and zero job-level DB notifications (representative bulk job)
|
||||
|
||||
### Recommended test locations
|
||||
- Guards: `tests/Feature/OpsUx/Constitution/`
|
||||
- Regressions: `tests/Feature/OpsUx/Regression/`
|
||||
|
||||
## Success Criteria *(mandatory)*
|
||||
|
||||
### Measurable Outcomes
|
||||
|
||||
- **SC-001**: Every tracked operation with an initiator produces exactly one persistent DB notification upon terminal completion — zero operations complete silently.
|
||||
- **SC-002**: Zero queued/running DB notifications are emitted across all operation flows.
|
||||
- **SC-003**: Zero duplicate completion DB notifications for any single operation run across all exit paths (success, failure, partial, circuit-break).
|
||||
- **SC-004**: `RunStatusChangedNotification` class is fully deleted — zero references remain in the codebase.
|
||||
- **SC-005**: All three CI guard tests pass green on a clean codebase and fail with actionable output when a synthetic violation is introduced.
|
||||
- **SC-006**: All existing OpsUx test suites and related test files pass without regression after changes.
|
||||
- **SC-007** *(P2)*: All "already queued" dedup feedback paths use identical canonical copy, verifiable by a single source-of-truth presenter call.
|
||||
|
||||
---
|
||||
|
||||
## Definition of Done (DoD)
|
||||
|
||||
- [ ] All in-scope files no longer directly update status/outcome outside `OperationRunService`
|
||||
- [ ] All in-scope jobs no longer emit completion/abort/queued DB notifications
|
||||
- [ ] `RunStatusChangedNotification` deleted; zero references in `app/` and `tests/`
|
||||
- [ ] Three guard tests exist and pass in CI (and fail with actionable output on synthetic violations)
|
||||
- [ ] All OpsUx regression tests pass
|
||||
- [ ] Full test suite green (no regressions)
|
||||
- [ ] Pint formatting clean for all touched files
|
||||
- [ ] `AGENTS.md` / constitution updated to reference the non-negotiable Ops-UX rule if not already present from 055 follow-up
|
||||
|
||||
---
|
||||
|
||||
## Assumptions
|
||||
|
||||
1. `OperationRunService` already exposes a canonical terminal transition method (from Spec 055); this spec calls it, not redeclares it.
|
||||
2. The `failed()` job lifecycle callback or try/finally blocks in affected jobs already exist (or will be added) to ensure terminal transitions even on unhandled exceptions — confirmed during implementation per-file.
|
||||
3. Guard tests are static analysis (filesystem grep-based) Pest tests, not runtime tests. They do not require a running application.
|
||||
4. The allowlist for Guard B (job DB notifications) is intentionally minimal. Any new entry requires justification in a spec comment.
|
||||
5. P2 tasks (T110-040/041) are optional and do not gate release of P0/P1 work.
|
||||
|
||||
---
|
||||
|
||||
## Rollout / PR Slicing
|
||||
|
||||
| PR | Tasks | Priority |
|
||||
|----|-------|----------|
|
||||
| PR-A | T110-001, T110-002 + regression tests (inventory sync, retention) | P0 |
|
||||
| PR-B | T110-010, T110-011 + restore run tests | P0 |
|
||||
| PR-C | T110-020, T110-021 + backup schedule tests | P0 |
|
||||
| PR-D | T110-022–T110-025 + bulk job tests | P1 |
|
||||
| PR-E | T110-030, T110-031, T110-032 (guards — can land early) | P0 |
|
||||
| PR-F | T110-040, T110-041 (optional polish) | P2 |
|
||||
207
specs/110-ops-ux-enforcement/tasks.md
Normal file
207
specs/110-ops-ux-enforcement/tasks.md
Normal 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
|
||||
Loading…
Reference in New Issue
Block a user