TenantAtlas/specs/351-review-output-resolve-actions-v1/plan.md
ahmido d4e4d2d109 feat: review output resolve actions v1 (spec 351) (#422)
Implemented the first version of review output resolve actions. Included a ReviewOutputResolveActionMapper, commands to seed browser fixtures, updated CustomerReviewWorkspace, EnvironmentReviewResource, UI enforcement, and related views. Also added extensive unit, feature, and browser tests, and updated the design coverage matrix.

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #422
2026-06-04 00:55:02 +00:00

248 lines
17 KiB
Markdown

# Implementation Plan: Spec 351 - Review Output Resolve Actions v1
**Branch**: `351-review-output-resolve-actions-v1` | **Date**: 2026-06-03 | **Spec**: `specs/351-review-output-resolve-actions-v1/spec.md`
**Input**: Direct user-provided Spec 351 draft plus repo truth from the current review-output guidance path and existing review lifecycle actions.
## Summary
Productize Spec 350's review-output-first contract into one bounded review-output resolve-action layer that can surface a real next step where the repo already supports one:
1. choose the strongest safe next action
2. explain why it is recommended
3. execute it only through existing source-owned Filament actions and services
4. degrade honestly to navigation or disclosure when execution is unavailable
This slice stays review-output-only and touches only the current review-output consumers:
- `CustomerReviewWorkspace`
- Environment Review detail
It must not:
- broaden into provider readiness, governance inbox, or dashboard reuse
- invent a new workflow engine or action runtime
- create fake remediation or fake draft discovery
- add persistence, schema, or new lifecycle states
## Technical Context
- **Language/Version**: PHP 8.4.15, Laravel 12.52.x
- **Primary Dependencies**: Filament 5.2.x, Livewire 4.1.x, Pest 4, Tailwind CSS 4
- **Storage**: PostgreSQL; no schema change expected
- **Testing**: Pest Unit + Feature/Livewire + one bounded Browser smoke
- **Validation Lanes**: fast-feedback + confidence + browser
- **Target Platform**: `apps/platform` Laravel monolith, Sail-first locally
- **Project Type**: server-rendered Filament web application
- **Performance Goals**: derived-only action selection, no new remote calls during render, no new queue family, no duplicate lifecycle dispatch
- **Constraints**: no fake executable actions, no direct service calls from Blade, no duplicate primary CTAs on detail, no provider/dashboard scope growth
- **Scale/Scope**: one deterministic mapper, one review-output adapter integration, one workspace-side executable action bridge, one non-duplicative detail alignment
## UI / Surface Guardrail Plan
- **Guardrail scope**: changed strategic workspace surface plus changed review-detail guidance semantics
- **Affected routes/pages/actions/states/navigation/panel/provider surfaces**:
- `/admin/reviews/workspace`
- existing Environment Review detail route(s)
- existing workspace-mounted page actions and Environment Review header lifecycle actions
- **No-impact class, if applicable**: N/A
- **Native vs custom classification summary**: native Filament page/resource surfaces plus existing custom Blade rendering
- **Shared-family relevance**: review-output guidance, lifecycle actions, evidence/proof follow-up links
- **State layers in scope**: page, detail, URL-query, derived action-state envelope
- **Audience modes in scope**: operator-MSP, manager, readonly reviewer, customer-workspace detail context
- **Decision/diagnostic/raw hierarchy plan**: dominant next action first; limitations and evidence second; technical detail third
- **Raw/support gating plan**: unchanged; raw/support detail stays on source-owned surfaces
- **One-primary-action / duplicate-truth control**: workspace gets one dominant resolve action; detail aligns to one dominant lifecycle action without creating a second equal-weight rail
- **Handling modes by drift class or surface**: review-mandatory
- **Repository-signal treatment**: review-mandatory because the slice makes high-impact review actions more visible
- **Special surface test profiles**: `global-context-shell` + `shared-detail-family`
- **Required tests or manual smoke**: Unit + Feature + one bounded Browser smoke
- **Exception path and spread control**: if workspace execution requires more than mounting source-owned Filament actions, stop and keep the action as fallback navigation/disclosure
- **Active feature PR close-out entry**: Guardrail / Confirmation / Smoke Coverage
- **UI/Productization coverage decision**: update `ui-006` and `ui-040` only; no new route archetype or registry row is required
- **Coverage artifacts to update**: `docs/ui-ux-enterprise-audit/page-reports/ui-006-customer-review-workspace.md`, `docs/ui-ux-enterprise-audit/page-reports/ui-040-environment-review-detail.md`
- **No-impact rationale**: N/A
- **Navigation / Filament provider-panel handling**: no route or provider change
- **Screenshot or page-report need**: yes; this is a first-screen CTA trust change on existing strategic surfaces
## Shared Pattern & System Fit
- **Cross-cutting feature marker**: yes
- **Systems touched**:
- `App\Support\ReviewPacks\ReviewPackOutputResolutionGuidance`
- `App\Support\ResolutionGuidance\ResolutionAction`
- `App\Support\ResolutionGuidance\ResolutionCase`
- `App\Support\ResolutionGuidance\Adapters\ReviewPackOutputResolutionAdapter`
- `App\Filament\Pages\Reviews\CustomerReviewWorkspace`
- `App\Filament\Resources\EnvironmentReviewResource`
- `App\Filament\Resources\EnvironmentReviewResource\Pages\ViewEnvironmentReview`
- `App\Support\Ui\GovernanceActions\GovernanceActionCatalog`
- **Shared abstractions reused**:
- current review-output readiness derivation
- existing `ResolutionCase` / `ResolutionAction` contract
- existing Filament page-action mounting pattern on `CustomerReviewWorkspace`
- existing lifecycle actions and services on `ViewEnvironmentReview`
- current scoped route helpers and `OperationRunLinks`
- **New abstraction introduced? why?**: yes, one `ReviewOutputResolveActionMapper` is required because the repo has multiple real action sources and fallbacks, but current review-output guidance remains link-first and not execution-aware
- **Why the existing abstraction was sufficient or insufficient**: Spec 350 standardized the case shape, but not the deterministic ranking of real resolve actions or the surface-safe execution bridge for those actions
- **Bounded deviation / spread control**: any action execution metadata added to `ResolutionAction` must stay optional, review-output-only, and derived-only
## OperationRun UX Impact
- **Touches OperationRun start/completion/link UX?**: yes, indirectly
- **Central contract reused**:
- `EnvironmentReviewService::refresh()`
- `EnvironmentReviewLifecycleService::createNextReview()`
- existing `OperationRunLinks`
- **Delegated UX behaviors**: queueing, dedupe, run creation, and notifications remain owned by the current services and shared UX paths
- **Surface-owned behavior kept local**: action ranking, explanation text, and honest fallback selection
- **Queued DB-notification policy**: unchanged
- **Terminal notification path**: unchanged
- **Exception path**: none
## Provider Boundary & Portability Fit
N/A - the slice stays on review, evidence, pack, and operation surfaces only.
## Current Repo Truth Summary
- `ReviewPackOutputResolutionGuidance` already derives rich output state, limitation, and link/disclosure copy from `ReviewPackOutputReadiness`.
- `ReviewPackOutputResolutionAdapter` already wraps that guidance into `ResolutionCase`, but today it can only promote safe navigation/download/disclosure actions.
- `ViewEnvironmentReview` already exposes repo-real lifecycle actions:
- `refresh_review` backed by `EnvironmentReviewService::refresh()`
- `publish_review` backed by `EnvironmentReviewLifecycleService::publish()`
- `create_next_review` backed by `EnvironmentReviewLifecycleService::createNextReview()`
- `create_next_review` is capability-gated and audited, but it is **not** currently confirmation-gated even though it mutates review lifecycle state and starts the next review cycle.
- `EnvironmentReviewLifecycleService::createNextReview()` supersedes the published review and returns a mutable successor review; the repo has no generic workspace-level "open existing draft review" helper unless a concrete target review is already known.
- `CustomerReviewWorkspace` already mounts real Filament page actions from custom Blade via `mountAction()` and includes `<x-filament-actions::modals />`, so workspace-side execution of bounded actions is feasible without inventing a second runtime.
- Environment Review detail already has dominant header lifecycle actions, and customer-workspace detail mode intentionally suppresses repeated action rails inside the guidance card.
- `EvidenceSnapshotResource\Pages\ViewEvidenceSnapshot` already exposes `refresh_evidence` as a repo-real action, but review-output surfaces currently only link to evidence detail rather than executing evidence refresh directly.
- `CustomerReviewWorkspace` is viewable by readonly actors, so executable resolve actions on the workspace must be capability-aware and honest when unavailable.
## Implementation Approach
### Phase 0 - Repo Truth Gate
1. Re-read `spec.md`, `plan.md`, `tasks.md`, `repo-truth-map.md`, `contracts/review-output-resolve-action-map.md`, and `checklists/requirements.md`.
2. Re-verify:
- `apps/platform/app/Support/ReviewPacks/ReviewPackOutputResolutionGuidance.php`
- `apps/platform/app/Support/ResolutionGuidance/Adapters/ReviewPackOutputResolutionAdapter.php`
- `apps/platform/app/Filament/Pages/Reviews/CustomerReviewWorkspace.php`
- `apps/platform/app/Filament/Resources/EnvironmentReviewResource.php`
- `apps/platform/app/Filament/Resources/EnvironmentReviewResource/Pages/ViewEnvironmentReview.php`
- `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewService.php`
- `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewLifecycleService.php`
- `apps/platform/app/Filament/Resources/EvidenceSnapshotResource/Pages/ViewEvidenceSnapshot.php`
3. Keep the repo-truth docs current if the narrowest safe action map changes during implementation.
### Phase 1 - Tests First
1. Add `apps/platform/tests/Unit/ResolutionGuidance/Spec351ReviewOutputResolveActionMapperTest.php`.
2. Cover deterministic selection for:
- published blocked review with known successor review
- published blocked review without known successor review
- published blocked review without safe executable action
- mutable review with stale/missing evidence
- ready mutable review
- internal-only / PII / export-not-ready / no-action fallback
3. Add workspace/detail feature tests:
- `apps/platform/tests/Feature/Filament/Spec351CustomerReviewWorkspaceResolveActionTest.php`
- `apps/platform/tests/Feature/EnvironmentReview/Spec351EnvironmentReviewResolveActionTest.php`
4. Add one browser smoke:
- `apps/platform/tests/Browser/Spec351ReviewOutputResolveActionsSmokeTest.php`, covering at least one blocked path and one ready-state path
5. Reuse Spec 347/349/350 coverage instead of duplicating readiness/output assertions wholesale.
### Phase 2 - Core Mapper And Contract Extension
1. Create `apps/platform/app/Support/ResolutionGuidance/ReviewOutputResolveActionMapper.php`.
2. Feed it:
- the current `EnvironmentReview`
- current review-output guidance/readiness truth
- known linked targets such as evidence, review detail, operation proof, download URL
- surface context (`customer_review_workspace`, `environment_review_detail`, `environment_review_detail.customer_workspace`)
- actor/capability availability where action execution must differ by viewer
3. Deterministic ranking order:
- known successor review navigation when a concrete target exists
- `create_next_review` for blocked/limited published reviews
- `refresh_review` for mutable reviews with stale or incomplete inputs
- `publish_review` for ready mutable reviews
- evidence/proof/disclosure fallback when no safe execution path exists
4. Extend `ResolutionAction` only if needed so a case can carry missing source-owned execution-routing metadata such as `action_name` and `execution_surface`, while reusing existing `disabled_reason`, capability, confirmation, audit-event, and `OperationRun` metadata.
5. Make the mapper the canonical selector of `resolution_case.primary_action` and `secondary_actions` for the two in-scope surfaces. `ReviewPackOutputResolutionGuidance` remains the source for readiness, limitations, and explanatory copy; its raw action fields are compatibility data and must not drive CTA ranking on workspace or detail.
6. Keep the contract derived-only and request-scoped.
### Phase 3 - Workspace-Side Source-Owned Action Execution
1. Add or reuse `CustomerReviewWorkspace` page actions for:
- `refresh_review`
- `publish_review`
- `create_next_review`
2. Reuse existing services, `GovernanceActionCatalog` where already repo-real, `UiEnforcement`, capability checks, notifications, and audit behavior.
3. Mount those actions from the workspace decision card via `mountAction()` and existing Filament modals.
4. Do not call lifecycle services directly from Blade.
5. If a current actor or surface cannot safely execute the dominant action, downgrade to the strongest truthful navigation/disclosure fallback instead of showing a fake enabled button.
6. If `create_next_review` becomes a dominant workspace CTA, add `requiresConfirmation()` to the source-owned action before exposing it as executable; otherwise degrade it to truthful navigation/disclosure fallback. No exception path is allowed for executable use.
### Phase 4 - Adapter Integration And Workspace Behavior
1. Update `ReviewPackOutputResolutionAdapter` so the dominant action comes from the new mapper while preserving current case/title/reason/impact/source/evidence data.
2. Preserve current findings and accepted-risk follow-up overrides on `CustomerReviewWorkspace`; those remain higher-priority than the base review-output case.
3. Render exactly one dominant primary CTA in the workspace decision card and move all other mapped actions into a clearly secondary supporting-action group.
4. Keep current technical details and grouped limitations disclosure intact unless a narrower improvement is required for clarity.
5. Keep the review-consumption flow on the workspace as a supporting reference below the primary decision surfaces instead of a peer decision card.
6. Do not regress qualified download wording or honest internal-only wording.
### Phase 5 - Environment Review Detail Alignment
1. Align `EnvironmentReviewResource::outputGuidanceState()` to the same resolve-action semantics.
2. Preserve current `customer_workspace` detail-mode suppression and context note.
3. In normal detail mode, do **not** create two equal-weight primary action rails when the header already carries the same lifecycle action.
4. Satisfy the detail requirement by aligning the guidance card `next step` semantics and copy to the existing dominant header action. Do not move lifecycle CTA ownership out of `ViewEnvironmentReview` in this slice.
5. Only offer successor-review-open navigation on detail when a concrete successor review target is actually known.
### Phase 6 - Copy, Audit, And Browser Proof
1. Update only the required copy keys in:
- `apps/platform/lang/en/localization.php`
- `apps/platform/lang/de/localization.php`
2. Clarify acknowledgement copy so it never implies acknowledgement alone makes an output customer-ready.
3. Split the workspace review-pack state presentation into package-exists, internal-export, and customer-sharing semantics without adding new persistence or workflow state.
4. If repo truth keeps the current labels (`Create next review`, `Refresh review`) instead of the user-draft wording (`Create next review draft`, `Refresh review inputs`), document that decision in the implementation close-out.
5. Update:
- `docs/ui-ux-enterprise-audit/page-reports/ui-006-customer-review-workspace.md`
- `docs/ui-ux-enterprise-audit/page-reports/ui-040-environment-review-detail.md`
6. Capture screenshots under `specs/351-review-output-resolve-actions-v1/artifacts/screenshots/`.
7. When a persistent ready-path demo is needed in local/testing, seed it via a bounded Artisan browser-fixture command instead of mutating product code or widening browser smoke scope.
### Phase 7 - Validation And Close-Out
1. Run focused Spec 351 Unit, Feature, and Browser coverage.
2. Re-run filtered regressions for Specs 347, 349, 350, and `CustomerReviewWorkspace`.
3. Run `pint --dirty` and `git diff --check`.
4. Report any out-of-scope failures separately without widening implementation scope.
## Validation Plan
```bash
cd apps/platform
./vendor/bin/sail artisan test tests/Unit/ResolutionGuidance/Spec351ReviewOutputResolveActionMapperTest.php --compact
./vendor/bin/sail artisan test tests/Feature/Filament/Spec351CustomerReviewWorkspaceResolveActionTest.php tests/Feature/EnvironmentReview/Spec351EnvironmentReviewResolveActionTest.php --compact
./vendor/bin/sail artisan test tests/Feature/Console/TenantpilotSeedReviewOutputBrowserFixtureCommandTest.php --compact
./vendor/bin/sail php vendor/bin/pest tests/Browser/Spec351ReviewOutputResolveActionsSmokeTest.php --compact
./vendor/bin/sail artisan tenantpilot:review-output:seed-browser-fixture --no-interaction
./vendor/bin/sail artisan test --compact --filter=Spec347
./vendor/bin/sail artisan test --compact --filter=Spec349
./vendor/bin/sail artisan test --compact --filter=Spec350
./vendor/bin/sail artisan test --compact --filter=CustomerReviewWorkspace
./vendor/bin/sail pint --dirty
git diff --check
```
## Deployment Impact
- **Env vars**: none expected
- **Migrations**: none
- **Queues / scheduler**: no new queue family; existing review refresh / next-review queue behavior reused
- **Storage**: none
- **Assets**: no new Filament asset registration expected; `filament:assets` is not newly required by this slice
- **Panel providers / global search**: unchanged; Filament v5 / Livewire v4 posture remains, and provider registration stays in `apps/platform/bootstrap/providers.php`