a6ff903093
Some checks failed
PR Fast Feedback / fast-feedback (pull_request) Failing after 3m45s
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.
248 lines
17 KiB
Markdown
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`
|