ahmido
3030dd9af2
Summary Kurz: Implementiert Feature 054 — canonical OperationRun-flow, Monitoring UI, dispatch-safety, notifications, dedupe, plus small UX safety clarifications (RBAC group search delegated; Restore group mapping DB-only). What Changed Core service: OperationRun lifecycle, dedupe and dispatch helpers — OperationRunService.php. Model + migration: OperationRun model and migration — OperationRun.php, 2026_01_16_180642_create_operation_runs_table.php. Notifications: queued + terminal DB notifications (initiator-only) — OperationRunQueued.php, OperationRunCompleted.php. Monitoring UI: Filament list/detail + Livewire pieces (DB-only render) — OperationRunResource.php and related pages/views. Start surfaces / Jobs: instrumented start surfaces, job middleware, and job updates to use canonical runs — multiple app/Jobs/* and app/Filament/* updates (see tests for full coverage). RBAC + Restore UX clarifications: RBAC group search is delegated-Graph-based and disabled without delegated token; Restore group mapping remains DB-only (directory cache) and helper text always visible — TenantResource.php, RestoreRunResource.php. Specs / Constitution: updated spec & quickstart and added one-line constitution guideline about Graph usage: spec.md quickstart.md constitution.md Tests & Verification Unit / Feature tests added/updated for run lifecycle, notifications, idempotency, and UI guards: see tests/Feature/* (notably OperationRunServiceTest, MonitoringOperationsTest, OperationRunNotificationTest, and various Filament feature tests). Full test run locally: ./vendor/bin/sail artisan test → 587 passed, 5 skipped. Migrations Adds create_operation_runs_table migration; run php artisan migrate in staging after review. Notes / Rationale Monitoring pages are explicitly DB-only at render time (no Graph calls). Start surfaces enqueue work only and return a “View run” link. Delegated Graph access is used only for explicit user actions (RBAC group search); restore mapping intentionally uses cached DB data only to avoid render-time Graph calls. Dispatch wrapper marks runs failed immediately if background dispatch throws synchronously to avoid misleading “queued” states. Upgrade / Deploy Considerations Run migrations: ./vendor/bin/sail artisan migrate. Background workers should be running to process queued jobs (recommended to monitor queue health during rollout). No secret or token persistence changes. PR checklist Tests updated/added for changed behavior Specs updated: 054-unify-runs-suitewide docs + quickstart Constitution note added (.specify) Pint formatting applied Co-authored-by: Ahmed Darrazi <ahmeddarrazi@adsmac.local> Reviewed-on: #63
62 lines
6.9 KiB
Markdown
62 lines
6.9 KiB
Markdown
# Spec Review Checklist: Unified Operations Runs Suitewide (054)
|
||
|
||
**Purpose**: Validate that `spec.md` is PR-reviewable and implementable by checking requirement quality (clarity, completeness, consistency, and testability), with emphasis on Audit-only vs OperationRun boundaries and tenant isolation/privacy/sanitization.
|
||
**Created**: 2026-01-17
|
||
**Feature**: `specs/054-unify-runs-suitewide/spec.md`
|
||
|
||
**Note**: This checklist is generated by the `/speckit.checklist` command based on feature context and requirements.
|
||
|
||
## Requirement Completeness
|
||
|
||
- [x] CHK001 Are Phase 1 adoption-set operations explicitly enumerated and scoped? [Completeness] Evidence: `spec.md` §Scope & Assumptions (“Phase 1 adoption set”)
|
||
- [x] CHK002 Are required Phase 1 run types explicitly listed and stable (including restore adapter and backup schedules)? [Completeness] Evidence: `spec.md` §FR-003
|
||
- [x] CHK003 Are mandatory run record fields specified (initiator, type, status/timestamps, outcome, counts, failures, identity, context)? [Completeness] Evidence: `spec.md` §FR-001
|
||
- [x] CHK004 Are lifecycle states and outcome buckets defined with allowed values? [Completeness] Evidence: `spec.md` §FR-011–FR-012
|
||
- [x] CHK005 Are idempotency identity rules specified for each Phase 1 run type (effective inputs included/excluded)? [Completeness] Evidence: `spec.md` §FR-010
|
||
- [x] CHK006 Are role/permission requirements defined for viewing runs vs starting operations? [Completeness] Evidence: `spec.md` §FR-018 + §User Story 1 (Scenario 5) + §User Story 2 (Scenario 2)
|
||
- [x] CHK007 Are notification requirements defined for queued and terminal outcomes (recipient, summary, “View run” link)? [Completeness] Evidence: `spec.md` §FR-015 + §User Story 2 (Scenario 3)
|
||
|
||
## Audit-only vs OperationRun Boundaries
|
||
|
||
- [x] CHK008 Is the eligibility criteria for Audit-only actions fully specified (DB-only, ≤2s, no remote calls, no background work)? [Clarity] Evidence: `spec.md` §FR-019
|
||
- [x] CHK009 Is it unambiguous when an action may remain Audit-only versus must be an OperationRun (e.g., operational relevance vs queued/remote)? [Clarity] Evidence: `spec.md` §FR-019 + §Rule (Run vs Audit-only)
|
||
- [x] CHK010 Are “security-relevant” / “operational behavior change” triggers for mandatory AuditLog entries defined beyond examples (so classification is reviewable)? [Clarity] Evidence: `spec.md` §FR-019 (“Trigger guidance”)
|
||
- [x] CHK011 Are required AuditLog fields complete and unambiguous (tenant, actor, stable action ID, target, before/after or diff, timestamp)? [Completeness] Evidence: `spec.md` §FR-019
|
||
- [x] CHK012 Does the spec define sanitization expectations for `before/after/diff` in AuditLog (what must be excluded) rather than assuming it? [Privacy] Evidence: `spec.md` §FR-019 (“Sanitization (AuditLog before/after/diff)”)
|
||
- [x] CHK013 Does the adoption matrix cover the required feature areas and assign a stable `run_type` or audit action id for each? [Completeness] Evidence: `spec.md` §Run vs Audit-only Adoption Matrix (Phase 1)
|
||
- [x] CHK014 Are the adoption matrix audit action identifiers consistent with the “stable action identifier” requirement for AuditLog entries? [Consistency] Evidence: `spec.md` §FR-019 + §Run vs Audit-only Adoption Matrix
|
||
- [x] CHK015 Are FR-019 acceptance checks sufficient and non-contradictory (no OperationRun; exactly one AuditLog; tenant-scoped; cross-tenant forbidden)? [Acceptance Criteria] Evidence: `spec.md` §FR-019 (Acceptance checks)
|
||
|
||
## Tenant Isolation & Privacy / Sanitization
|
||
|
||
- [x] CHK016 Are tenant isolation requirements explicitly stated for run list access and run detail access? [Completeness] Evidence: `spec.md` §FR-016 + §User Story 1 (Scenarios 1, 6)
|
||
- [x] CHK017 Is “cross-tenant access is denied without disclosing run details” sufficiently specified to be reviewable (what must not be exposed)? [Clarity] Evidence: `spec.md` §FR-016 + §User Story 1 (Scenario 6)
|
||
- [x] CHK018 Are start-surface authorization requirements explicit enough to prevent unauthorized run creation (especially Readonly)? [Completeness] Evidence: `spec.md` §FR-007 + §FR-018 + §User Story 2 (Scenario 2)
|
||
- [x] CHK019 Are persisted failure requirements explicit about what MUST NOT be stored (tokens/credentials/PII/raw payload dumps)? [Clarity] Evidence: `spec.md` §FR-013 + §SC-004
|
||
- [x] CHK020 Are “stable reason codes” and “short sanitized messages” defined enough to be objectively reviewable (format expectations or examples)? [Clarity] Evidence: `spec.md` §FR-013 (Reason codes + Messages)
|
||
- [x] CHK021 Does the spec define what may be stored in run `context` and require it to be safe/sanitized (no secrets/PII)? [Privacy] Evidence: `spec.md` §FR-001 (“Context safety”)
|
||
- [x] CHK022 Are Monitoring render-time constraints explicit (DB-only; no external calls; no remote fetches at render time)? [Completeness] Evidence: `spec.md` §FR-017 + §FR-008
|
||
|
||
## Requirement Consistency
|
||
|
||
- [x] CHK023 Are the terms “status” and “outcome bucket” used consistently (and are queued/running treated consistently across the doc)? [Consistency] Evidence: `spec.md` §FR-001 (Status/Outcome semantics) + §FR-011 (Run state presentation)
|
||
- [x] CHK024 Are run type naming rules consistent across taxonomy, Phase 1 run list, and the adoption matrix (spelling/casing)? [Consistency] Evidence: `spec.md` §FR-002 + §FR-003 + §Run vs Audit-only Adoption Matrix
|
||
- [x] CHK025 Is restore adapter behavior described consistently across Clarifications, Scope (“Restore visibility”), FR-003, and Key Entities? [Consistency] Evidence: `spec.md` §Clarifications (restore) + §Scope & Assumptions (“Restore visibility”) + §FR-003 + §Key Entities
|
||
- [x] CHK026 Are retention and default monitoring window expectations consistent (retention vs default list time range)? [Consistency] Evidence: `spec.md` §Clarifications (retention) + §Assumptions + §FR-004
|
||
|
||
## Acceptance Criteria Quality
|
||
|
||
- [x] CHK027 Are success criteria measurable and objectively verifiable without implementation details? [Measurability] Evidence: `spec.md` §SC-001–SC-004
|
||
- [x] CHK028 Is the ≥99% dedupe target defined with a measurement scope (what counts as an attempt; “normal conditions” definition)? [Clarity] Evidence: `spec.md` §SC-003 (Measurement scope) + §FR-009
|
||
- [x] CHK029 Is “no secrets/PII” defined with an explicit boundary sufficient for reviewers to validate completeness? [Clarity] Evidence: `spec.md` §SC-004 + §FR-013
|
||
|
||
## Scenario Coverage
|
||
|
||
- [x] CHK030 Are primary, forbidden, and “background unavailable” scenarios covered with explicit, testable outcomes (including “must not claim queued”)? [Coverage] Evidence: `spec.md` §User Stories 1–3 + §User Story 2 (Scenario 4) + §Edge Cases
|
||
|
||
## Notes
|
||
|
||
- Check items off as completed: `[x]`
|
||
- Add findings inline (e.g., under a checklist item) with links to the relevant spec section
|
||
- This checklist evaluates requirements quality, not implementation correctness
|