TenantAtlas/specs/254-remove-acknowledged-compat/plan.md
ahmido b511b08371
Some checks failed
PR Fast Feedback / fast-feedback (pull_request) Failing after 1m0s
feat: remove findings acknowledged compatibility and unify canonical operation types (#296)
This PR removes the legacy "acknowledged" status compatibility for findings and unifies the canonical operation types (e.g., transitioning from baseline_capture to baseline.capture). It includes updated tests, models, and services to reflect these changes.

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #296
2026-04-29 07:34:39 +00:00

267 lines
25 KiB
Markdown

# Implementation Plan: Remove Legacy Acknowledged Finding Status Compatibility
**Branch**: `254-remove-acknowledged-compat` | **Date**: 2026-04-29 | **Spec**: `/Users/ahmeddarrazi/Documents/projects/wt-plattform/specs/254-remove-acknowledged-compat/spec.md`
**Input**: Feature specification from `/Users/ahmeddarrazi/Documents/projects/wt-plattform/specs/254-remove-acknowledged-compat/spec.md`
**Note**: This template is filled in by the `/speckit.plan` command. See `.specify/scripts/` for helper scripts.
## Summary
- Remove productive `acknowledged` compatibility from the findings domain by collapsing canonical status, query, badge, filter, capability, policy, and workflow seams onto the surviving findings lifecycle only.
- Keep the slice subtractive and repo-based: no new state, no migration shim, no repair tooling, no broader lifecycle redesign, and no changes to verification-check or onboarding acknowledgement domains.
- Validate the cleanup through focused workflow, summary, badge or filter, capability, and guard coverage so shared findings-derived counts and operator surfaces converge on one canonical language at the same time.
## Technical Context
**Language/Version**: PHP 8.4, Laravel 12
**Primary Dependencies**: Filament v5, Livewire v4, Pest v4, existing findings workflow services, shared badge and filter catalogs, capability registry, and canonical summary builders
**Storage**: PostgreSQL existing `findings`, `finding_exceptions`, `audit_logs`, `operation_runs`, and related read models only; no new persistence or migration is planned
**Testing**: Pest unit, feature, and bounded heavy-governance guard coverage
**Validation Lanes**: fast-feedback, confidence, heavy-governance
**Target Platform**: Sail-backed Laravel web application with tenant `/admin/t/{tenant}` findings surfaces and canonical `/admin` summary or inbox surfaces
**Project Type**: web
**Performance Goals**: shared query helpers, inboxes, and summary builders keep their current bounded DB-only render profile; the slice should reduce branching and suite noise rather than add overhead
**Constraints**: LEAN-001 replacement over shims; no schema drop by default; preserve current `404` versus `403` isolation semantics; no panel or provider changes; no new assets; no widening into creation-time invariant hardening, backfill-runtime-surface work, or external support handoff
**Scale/Scope**: 1 cleanup slice touching the `Finding` model and factory, findings workflow service and policy seam, shared badge and filter catalog paths, findings resource and inbox surfaces, canonical summary builders, capability and role maps, and the related findings and guard tests
## Likely Affected Repo Surfaces
- Canonical findings status and workflow seams: `apps/platform/app/Models/Finding.php`, `apps/platform/app/Services/Findings/FindingWorkflowService.php`, `apps/platform/app/Policies/FindingPolicy.php`, and `apps/platform/database/factories/FindingFactory.php`
- Shared operator vocabulary seams: `apps/platform/app/Support/Badges/Domains/FindingStatusBadge.php`, `apps/platform/app/Support/Filament/FilterOptionCatalog.php`, `apps/platform/app/Support/Auth/Capabilities.php`, and `apps/platform/app/Services/Auth/RoleCapabilityMap.php`
- Tenant findings Filament surfaces: `apps/platform/app/Filament/Resources/FindingResource.php`, `apps/platform/app/Filament/Resources/FindingResource/Pages/ListFindings.php`, `apps/platform/app/Filament/Pages/Findings/MyFindingsInbox.php`, and related workflow concerns
- Findings-derived summary and review helpers still relying on shared open-status handling or explicit `acknowledged` strings: `apps/platform/app/Support/CustomerHealth/WorkspaceHealthSummaryQuery.php`, `apps/platform/app/Support/Workspaces/WorkspaceOverviewBuilder.php`, `apps/platform/app/Support/GovernanceInbox/GovernanceInboxSectionBuilder.php`, `apps/platform/app/Support/Baselines/BaselineCompareStats.php`, and `apps/platform/app/Services/TenantReviews/TenantReviewSectionFactory.php`
- Query and generator consumers of `Finding::openStatusesForQuery()`: `apps/platform/app/Services/PermissionPosture/PermissionPostureFindingGenerator.php`, `apps/platform/app/Services/EntraAdminRoles/EntraAdminRolesFindingGenerator.php`, `apps/platform/app/Services/Baselines/BaselineAutoCloseService.php`, `apps/platform/app/Services/Findings/FindingAssignmentHygieneService.php`, `apps/platform/app/Jobs/Alerts/EvaluateAlertsJob.php`, and `apps/platform/app/Support/SupportDiagnostics/SupportDiagnosticBundleBuilder.php`
- Current proof surface likely requiring update or replacement: `apps/platform/tests/Feature/Models/FindingResolvedTest.php`, `apps/platform/tests/Feature/Findings/FindingsIntakeQueueTest.php`, `apps/platform/tests/Feature/PermissionPosture/PermissionPostureFindingGeneratorTest.php`, `apps/platform/tests/Feature/EntraAdminRoles/EntraAdminRolesFindingGeneratorTest.php`, `apps/platform/tests/Unit/Badges/FindingBadgesTest.php`, `apps/platform/tests/Feature/Support/Badges/FindingBadgeTest.php`, `apps/platform/tests/Feature/Guards/NoAdHocStatusBadgesTest.php`, and `apps/platform/tests/Feature/Guards/FilamentTableStandardsGuardTest.php`
## Domain / Model Fit
- `Finding` remains the single tenant-owned source of truth with required `workspace_id` and `tenant_id` anchors. No new entity, enum family, compatibility table, or derived persistence layer is introduced.
- The canonical active lifecycle stays `new`, `triaged`, `in_progress`, and `reopened`, with `resolved`, `closed`, and `risk_accepted` remaining the canonical terminal set. `acknowledged` is removed as an active status contract rather than remapped through runtime helpers.
- `openStatusesForQuery()` and related status helpers should collapse onto the canonical open-status set instead of preserving an extra compatibility branch for `acknowledged`.
- Existing `acknowledged_at` and `acknowledged_by_user_id` columns are not justification for compatibility behavior in this slice. Default plan posture is to leave schema shape unchanged and remove productive semantics only.
- Legacy factory or fixture helpers that create `acknowledged` findings should be deleted or confined to explicitly documented stale-data edge proof only if implementation later proves that is still needed. They should not remain the default way to express current workflow truth.
## UI / Filament & Livewire Fit
- All touched operator surfaces remain native Filament v5 on Livewire v4. No custom dashboard framework, no panel change, and no new provider registration work are needed.
- `FindingResource` already has a `view` page, so the feature does not create a Filament global-search compliance problem. No new searchable resource is introduced.
- Shared badge rendering stays on `BadgeCatalog` plus `BadgeRenderer`, and shared filter vocabulary stays on `FilterOptionCatalog`; the cleanup must remove `acknowledged` from those shared paths rather than introducing page-local label overrides.
- Findings table, detail, and inbox surfaces should remove `acknowledged` from triage or progress visibility checks, filter options, summary counts, and helper wording together so operator UI does not drift between list, detail, and summary shells.
- No new destructive action is added. Existing destructive-like finding actions remain out of scope except that any touched action surface must preserve current `->requiresConfirmation()` and server-side capability or policy enforcement.
- No panel-only or shared asset changes are planned, so deployment keeps the existing `cd apps/platform && php artisan filament:assets` expectation unchanged only for already-registered assets.
## RBAC / Policy Fit
- Tenant membership and workspace membership remain the isolation boundaries: non-members stay `404`, entitled members missing the surviving capability stay `403`, and no resource existence leak is introduced while cleaning status language.
- `Capabilities::TENANT_FINDINGS_ACKNOWLEDGE` is removed instead of preserved as an alias. `RoleCapabilityMap`, `FindingPolicy`, and `FindingWorkflowService` should converge on the surviving findings capabilities only.
- The feature does not add a new role, a new authorization plane, or any page-local permission dialect. It narrows existing capability vocabulary.
- Disabled helper text and action affordances should continue to rely on the existing shared UI enforcement path while referencing only canonical findings workflow permissions.
## Audit / Logging Fit
- No new `AuditActionId` is introduced.
- Existing findings workflow audit verbs such as triage, assign, start progress, resolve, close, reopen, and risk acceptance remain canonical. The cleanup should not revive or add a finding-acknowledged audit dialect.
- Historical pre-production audit or metadata values do not justify runtime label shims. Verification-check acknowledgement audit behavior remains untouched.
## Migration / Data Shape Fit
- No new migration, no historical data backfill, and no fallback reader are planned.
- Repo evidence shows the findings status is a string column and the `acknowledged` behavior lives in code, factories, and tests rather than in a database enum or required migration path.
- Default implementation posture is to leave `findings` table columns intact for now and remove productive status compatibility from code and fixtures only. If schema removal appears necessary later, that must be split or re-justified instead of silently widening this cleanup.
- Local pre-production rows that still contain `acknowledged` are not a product compatibility requirement. Dev data reset or fixture replacement is preferred over runtime support.
## UI / Surface Guardrail Plan
- **Guardrail scope**: changed surfaces
- **Native vs custom classification summary**: native Filament plus shared badge, filter, and summary primitives
- **Shared-family relevance**: status messaging, findings workflow actions, shared badge semantics, shared filter vocabularies, canonical summary counts and previews
- **State layers in scope**: page, detail, URL-query
- **Audience modes in scope**: operator-MSP
- **Decision/diagnostic/raw hierarchy plan**: decision-first / diagnostics-second / support-raw-third
- **Raw/support gating plan**: existing capability-gated diagnostics remain unchanged; no new raw or support surface is introduced
- **One-primary-action / duplicate-truth control**: findings surfaces keep one canonical workflow language so triage and progress remain the dominant next actions without a duplicate `acknowledged` branch competing in badges, filters, or summaries
- **Handling modes by drift class or surface**: review-mandatory
- **Repository-signal treatment**: review-mandatory now; any leftover productive `acknowledged` findings seam after implementation is a blocker
- **Special surface test profiles**: standard-native-filament, global-context-shell
- **Required tests or manual smoke**: functional-core, state-contract
- **Exception path and spread control**: none; the removal must happen in the shared seams themselves rather than through local exemptions
- **Active feature PR close-out entry**: Guardrail
## Shared Pattern & System Fit
- **Cross-cutting feature marker**: yes
- **Systems touched**: `Finding`, `FindingWorkflowService`, `FindingPolicy`, `FindingStatusBadge`, `FilterOptionCatalog`, `Capabilities`, `RoleCapabilityMap`, `FindingResource`, `ListFindings`, `MyFindingsInbox`, `WorkspaceHealthSummaryQuery`, `WorkspaceOverviewBuilder`, `GovernanceInboxSectionBuilder`, `BaselineCompareStats`, `TenantReviewSectionFactory`, and the generator or alert consumers of shared open-status helpers
- **Shared abstractions reused**: `BadgeCatalog`, `BadgeRenderer`, `FilterOptionCatalog`, shared findings status helpers, `UiEnforcement`, and the canonical capability registry
- **New abstraction introduced? why?**: none; the correct move is to remove one legacy branch from existing shared seams
- **Why the existing abstraction was sufficient or insufficient**: the shared seams are sufficient once the compatibility branch is removed centrally; they are the reason the cleanup cannot stay local to one page or test file
- **Bounded deviation / spread control**: none; any repo surface still naming productive findings `acknowledged` after the cleanup is drift and should be removed rather than wrapped
## OperationRun UX Impact
- **Touches OperationRun start/completion/link UX?**: no
- **Central contract reused**: `N/A`
- **Delegated UX behaviors**: `N/A`
- **Surface-owned behavior kept local**: existing findings and summary surfaces only; no `OperationRun` start or link semantics change
- **Queued DB-notification policy**: `N/A`
- **Terminal notification path**: `N/A`
- **Exception path**: none
## Provider Boundary & Portability Fit
- **Shared provider/platform boundary touched?**: no
- **Provider-owned seams**: `N/A`
- **Platform-core seams**: existing findings, review, and governance vocabulary only
- **Neutral platform terms / contracts preserved**: existing platform and findings vocabulary remains; the cleanup narrows a finding-domain alias rather than spreading provider language
- **Retained provider-specific semantics and why**: none
- **Bounded extraction or follow-up path**: `N/A`
## Constitution Check
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
- Inventory-first / snapshots-second: PASS - findings remain the last-observed tenant truth, and no backup or snapshot contract changes are introduced
- Read/write separation: PASS - the slice does not add a new write path; it only narrows status semantics on existing findings workflows
- Graph contract path: PASS - no Microsoft Graph contract or provider endpoint change is involved
- Deterministic capabilities: PASS - the capability registry becomes simpler by removing a stale alias instead of expanding capability derivation
- RBAC-UX and isolation: PASS - `/admin/t/{tenant}` findings surfaces remain tenant-scoped; non-members stay `404`; in-scope members missing surviving findings capability stay `403`; no raw capability strings should survive after cleanup
- Workspace isolation / tenant isolation: PASS - tenant-owned findings and derived canonical summaries keep current workspace plus tenant entitlement rules
- Destructive confirmation standard: PASS - no new destructive action is introduced; any touched destructive-like findings action must preserve current confirmation and authorization semantics
- Global search safety: PASS - `FindingResource` already has a view page, and no new searchable resource is added
- OperationRun observability and Ops-UX: PASS - no new operation type, run start surface, or run-notification path is introduced
- Data minimization: PASS - no new payload, no raw evidence expansion, and no new audit family is introduced
- Test governance (`TEST-GOV-001`): PASS - proof stays in focused unit plus feature coverage with one explicit retained heavy-governance guard layer for shared badge or filter drift
- Proportionality (`PROP-001`) and no premature abstraction (`ABSTR-001`): PASS - the plan is subtractive and introduces no new abstraction, registry, or semantic framework
- Persisted truth (`PERSIST-001`): PASS - no new table, entity, artifact, or stored compatibility layer is added
- Behavioral state (`STATE-001`): PASS - the feature removes a legacy active-workflow branch instead of adding a new state family
- UI semantics (`UI-SEM-001`) and shared pattern first (`XCUT-001`): PASS - badge, filter, workflow, and summary semantics stay on existing shared seams rather than a new interpretation layer
- Provider boundary (`PROV-001`) and few layers (`V1-EXP-001`, `LAYER-001`): PASS - no provider seam or extra layer is introduced
- Filament-native UI and planning contract: PASS - Filament v5 remains on Livewire v4, provider registration remains unchanged in `apps/platform/bootstrap/providers.php`, and no panel or asset strategy change is required
- Asset strategy: PASS - no new panel or shared asset registration is planned; existing deploy behavior for `filament:assets` remains unchanged
## Test Governance Check
- **Test purpose / classification by changed surface**: Unit for findings status helpers, badge or filter catalog semantics, and capability-registry cleanup; Feature for findings workflow actions, resource or inbox behavior, policy outcomes, and shared summary consumers; Heavy-Governance for the explicit guard layer that blocks ad-hoc status badge or table drift
- **Affected validation lanes**: fast-feedback, confidence, heavy-governance
- **Why this lane mix is the narrowest sufficient proof**: the core risk is central semantic drift across shared helpers and operator surfaces, not browser choreography or new async behavior. Focused unit and feature coverage prove the canonical status path, while one retained guard layer ensures `acknowledged` does not reappear through shared UI seams
- **Narrowest proving command(s)**:
- `export PATH="/bin:/usr/bin:/usr/local/bin:$PATH" && cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Findings/RemoveAcknowledgedCompatibilityWorkflowTest.php tests/Unit/Support/CustomerHealth/WorkspaceHealthSummaryQueryTest.php tests/Unit/Support/GovernanceInbox/GovernanceInboxSectionBuilderTest.php`
- `export PATH="/bin:/usr/bin:/usr/local/bin:$PATH" && cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Unit/Findings/FindingStatusSemanticsTest.php tests/Unit/Support/Filament/FindingStatusFilterCatalogTest.php tests/Feature/Auth/RemoveAcknowledgedCapabilityAliasTest.php`
- `export PATH="/bin:/usr/bin:/usr/local/bin:$PATH" && cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/PermissionPosture/PermissionPostureFindingGeneratorTest.php tests/Feature/EntraAdminRoles/EntraAdminRolesFindingGeneratorTest.php tests/Feature/Findings/FindingsIntakeQueueTest.php`
- `export PATH="/bin:/usr/bin:/usr/local/bin:$PATH" && cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Guards/NoAdHocStatusBadgesTest.php tests/Feature/Guards/FilamentTableStandardsGuardTest.php`
- **Fixture / helper / factory / seed / context cost risks**: acknowledged-specific fixtures and factory states are likely removal targets; keep any remaining stale-data setup explicit and local instead of spreading a legacy default across helper layers
- **Expensive defaults or shared helper growth introduced?**: no; expected net-neutral to negative because the slice removes compatibility branches and stale test setup
- **Heavy-family additions, promotions, or visibility changes**: no new heavy family is planned; one existing shared guard layer remains explicit because badge and filter drift can otherwise reintroduce removed semantics silently
- **Surface-class relief / special coverage rule**: standard-native-filament and global-context-shell relief are sufficient; no browser lane is required for this cleanup
- **Closing validation and reviewer handoff**: rerun the focused commands above, verify that no productive findings seam, capability alias, badge label, filter option, or summary builder still treats `acknowledged` as current workflow truth, and verify that verification or onboarding acknowledgement domains remain unchanged
- **Budget / baseline / trend follow-up**: expected net-neutral to slightly negative because compatibility-only tests should be removed or consolidated
- **Review-stop questions**: did implementation leave `acknowledged` in a shared status helper, a policy or capability path, a badge or filter catalog, a summary builder, or a generator test family; did it widen into schema removal or non-finding acknowledgement work
- **Escalation path**: reject-or-split
- **Active feature PR close-out entry**: Guardrail
- **Why no dedicated follow-up spec is needed**: this slice is a bounded cleanup; creation-time invariant hardening, backfill-runtime-surface removal, and external support handoff already remain explicit separate follow-up work
## Rollout & Risk Controls
- Implement as replacement, not aliasing. Shared helpers, capability registries, and summary builders should converge directly on canonical statuses in the same slice.
- Treat local stale `acknowledged` rows as pre-production cleanup debt, not a customer compatibility contract. Do not add fallback readers or UI labels to preserve them.
- Preserve scope boundaries aggressively: verification acknowledgement, onboarding verification acknowledgement, restore impact acknowledgement, and non-finding support acknowledgement semantics stay untouched.
- Review stop conditions should fire if implementation tries to drop schema, invent a new compatibility mapper, or widen into findings lifecycle redesign.
- Rollout is code-only and repo-local. No queue, deployment, asset, or migration sequencing is expected.
## Project Structure
### Documentation (this feature)
```text
specs/254-remove-acknowledged-compat/
├── plan.md
├── research.md
├── data-model.md
├── quickstart.md
├── checklists/
│ └── requirements.md
├── contracts/
│ └── findings-acknowledged-compat-removal.contract.yaml
└── tasks.md
```
### Source Code (repository root)
```text
apps/platform/
├── app/
│ ├── Filament/
│ │ ├── Pages/Findings/
│ │ └── Resources/FindingResource/
│ ├── Jobs/Alerts/
│ ├── Models/
│ ├── Policies/
│ ├── Services/
│ │ ├── Auth/
│ │ ├── Baselines/
│ │ ├── EntraAdminRoles/
│ │ ├── Findings/
│ │ ├── PermissionPosture/
│ │ └── TenantReviews/
│ └── Support/
│ ├── Auth/
│ ├── Badges/
│ ├── CustomerHealth/
│ ├── Filament/
│ ├── GovernanceInbox/
│ └── Workspaces/
├── database/
│ └── factories/
└── tests/
├── Feature/
│ ├── Auth/
│ ├── Findings/
│ ├── Guards/
│ ├── PermissionPosture/
│ └── EntraAdminRoles/
└── Unit/
├── Badges/
└── Findings/
```
**Structure Decision**: Laravel monolith. Implementation should stay inside existing findings model, workflow, policy, shared support, Filament resource or page, factory, and test directories rather than creating a new namespace or migration track.
## Complexity Tracking
No constitution violation is expected. If implementation later proves it needs schema removal, a compatibility shim, or a new translation layer, that is a stop condition and should be split or rejected rather than absorbed here.
## Proportionality Review
N/A - this slice removes a legacy active-workflow alias and a stale capability alias. It introduces no new enum, presenter, persistence, contract layer, or taxonomy.
## Phase 0 — Research (output: `research.md`)
- Confirm the exact productive findings seams that still use `acknowledged` and separate them from explicitly out-of-scope non-finding acknowledgement domains.
- Confirm the no-migration posture for existing `acknowledged_*` fields and local stale rows under LEAN-001.
- Confirm which summary or review helpers still encode literal `acknowledged` status expectations and therefore belong in this cleanup instead of a later lifecycle redesign.
- Confirm which existing tests should be deleted, narrowed, or rewritten rather than preserved as compatibility proof.
## Phase 1 — Design & Contracts (outputs: `data-model.md`, `contracts/`, `quickstart.md`)
- `data-model.md` should describe the surviving canonical findings status set, the collapsed open-status query contract, the removal of the acknowledge capability alias, and the unchanged schema posture.
- `contracts/findings-acknowledged-compat-removal.contract.yaml` should capture the cleanup matrix across model helpers, workflow and policy authorization, shared badge or filter catalogs, findings resource behavior, summary consumers, and out-of-scope acknowledgement domains.
- `quickstart.md` should document the intended implementation order, validation commands, and review stop conditions for scope drift.
## Phase 1 — Agent Context Update
After Phase 1 artifacts are generated, update Copilot context from the plan:
- `/Users/ahmeddarrazi/Documents/projects/wt-plattform/.specify/scripts/bash/update-agent-context.sh copilot`
## Phase 2 — Implementation Outline (tasks created in `/speckit.tasks`)
- Collapse `Finding` status helpers and legacy model helpers onto the canonical status set only.
- Remove `TENANT_FINDINGS_ACKNOWLEDGE` and update role mappings, workflow authorization, and policy checks to the surviving capability set.
- Remove `acknowledged` from shared badge and filter catalogs and from findings resource or inbox workflow affordances.
- Update shared summary and generator consumers of `openStatusesForQuery()` or explicit `acknowledged` status lists so counts, previews, and auto-close behavior align with canonical statuses.
- Delete or rewrite acknowledged-compatibility tests and factories, then add focused regression proof for the surviving canonical workflow, summary alignment, and guard coverage.
- Verify that no non-finding acknowledgement domains were touched and that no migration or compatibility shim was introduced.
## Constitution Check (Post-Design)
Re-check target: PASS. The post-design shape must stay subtractive, keep Filament v5 on Livewire v4, leave provider registration unchanged in `apps/platform/bootstrap/providers.php`, keep `FindingResource` global-search-safe through its existing view page, add no new destructive action or asset bundle, and preserve the no-migration, no-compatibility-shim posture.