Implemented the output contract and readiness semantics for review packs. Also added spec 348. Includes changes to ChooseEnvironment, CustomerReviewWorkspace, GenerateReviewPackJob and related blade views. Added comprehensive tests. Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de> Reviewed-on: #419
314 lines
18 KiB
Markdown
314 lines
18 KiB
Markdown
# Implementation Plan: Spec 347 - Review Pack Output Contract & Readiness Semantics
|
|
|
|
**Branch**: `347-review-pack-output-contract-readiness-semantics` | **Date**: 2026-06-02 | **Spec**: `specs/347-review-pack-output-contract-readiness-semantics/spec.md`
|
|
**Input**: User-provided Spec 347 draft + repo truth from current review-derived Review Pack generation and Customer Review Workspace readiness behavior.
|
|
|
|
## Summary
|
|
|
|
Harden the current review-derived Review Pack output without rewriting the export pipeline.
|
|
|
|
This slice should:
|
|
|
|
- document the current file contract
|
|
- reconcile review/evidence/section/export/customer-safe semantics
|
|
- tighten ZIP payload consistency where fields are missing or ambiguous
|
|
- add explicit limitations/disclosure output
|
|
- replace unqualified workspace sharing language when repo truth says the package is limited
|
|
|
|
This slice must not:
|
|
|
|
- rebuild Review Pack generation
|
|
- add persistence
|
|
- add a portal
|
|
- change signed-download safety
|
|
- create a new workflow engine
|
|
|
|
## 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 Feature/Livewire tests plus one bounded Pest Browser smoke file
|
|
- **Validation Lanes**: confidence + browser
|
|
- **Target Platform**: `apps/platform` Laravel monolith; Sail-first locally; Dokploy posture unchanged
|
|
- **Project Type**: web application with generated ZIP artifacts
|
|
- **Performance Goals**: no new remote calls during render, no new queue family, deterministic ZIP contract and deterministic derived readiness mapping
|
|
- **Constraints**: no false customer-safe/export-ready/certification claims; no weakened signed-download controls; no revived legacy file layout or query aliases; keep diagnostics secondary
|
|
- **Scale/Scope**: one existing ZIP contract, one existing strategic workspace page, targeted Feature coverage, one Browser smoke, and spec-local contract docs
|
|
|
|
## UI / Surface Guardrail Plan
|
|
|
|
- **Guardrail scope**: material change to an existing strategic customer-safe review surface plus existing output artifact semantics
|
|
- **Affected routes/pages/actions/states/navigation/panel/provider surfaces**:
|
|
- `/admin/reviews/workspace`
|
|
- `apps/platform/app/Filament/Pages/Reviews/CustomerReviewWorkspace.php`
|
|
- `apps/platform/resources/views/filament/pages/reviews/customer-review-workspace.blade.php`
|
|
- review-derived ZIP output generated by `apps/platform/app/Jobs/GenerateReviewPackJob.php`
|
|
- workspace download wording and signed download affordance only; Review Pack Resource detail/header copy stays out of scope unless repo truth reveals a direct contradiction that cannot be fixed on the workspace surface
|
|
- **No-impact class, if applicable**: N/A
|
|
- **Native vs custom classification summary**: native Filament page plus existing Blade composition; existing export ZIP contract; no new route or panel/provider
|
|
- **Shared-family relevance**: evidence/review/export readiness labels, disclosure language, download affordances, proof links
|
|
- **State layers in scope**: page payload, artifact payload, signed-download label/copy on the workspace surface only
|
|
- **Audience modes in scope**: operator-MSP, customer-safe review consumer, support where authorized
|
|
- **Decision/diagnostic/raw hierarchy plan**: readiness and limitations first, proof second, diagnostics third
|
|
- **Raw/support gating plan**: diagnostics collapsed/secondary; raw/support details remain hidden on customer-safe default paths
|
|
- **One-primary-action / duplicate-truth control**: one dominant next action on the workspace decision card; lower sections add proof rather than restating the verdict
|
|
- **Handling modes by drift class or surface**: review-mandatory
|
|
- **Repository-signal treatment**: review-mandatory because this is a strategic output boundary and trust surface
|
|
- **Special surface test profiles**: `global-context-shell` + customer-safe strategic review surface + artifact contract
|
|
- **Required tests or manual smoke**: functional-core + browser smoke
|
|
- **Exception path and spread control**: one bounded readiness mapper is allowed; no generic review-output framework
|
|
- **Active feature PR close-out entry**: Guardrail / Smoke Coverage
|
|
- **UI/Productization coverage decision**: update `docs/ui-ux-enterprise-audit/page-reports/ui-006-customer-review-workspace.md`; do not create a new page-report identity unless implementation proves necessary
|
|
- **Coverage artifacts to update**: existing workspace page report only, unless route/archetype truth changes
|
|
- **Navigation / Filament provider-panel handling**: N/A; no panel/provider change expected
|
|
- **Screenshot or page-report need**: yes, one page-report update plus bounded screenshots because this is a strategic customer-safe surface
|
|
|
|
## Shared Pattern & System Fit
|
|
|
|
- **Cross-cutting feature marker**: yes
|
|
- **Systems touched**:
|
|
- `EnvironmentReviewComposer`
|
|
- `GenerateReviewPackJob`
|
|
- `ReviewPackService`
|
|
- `CustomerReviewWorkspace`
|
|
- existing Review Pack download and artifact proof surfaces
|
|
- **Shared abstractions reused**:
|
|
- current `delivery_bundle` metadata and summary payloads
|
|
- current `governance_package` review summary
|
|
- current `ArtifactTruthPresenter`
|
|
- current download/audit flow
|
|
- **New abstraction introduced? why?**: maybe one narrow readiness mapper or presenter if current page-local heuristics and ZIP payload logic need a single source for derived output-readiness labels
|
|
- **Why the existing abstraction was sufficient or insufficient**: existing structures already carry most facts, but they do not yet define one coherent output-readiness vocabulary or one authoritative contract between ZIP and workspace UI
|
|
- **Bounded deviation / spread control**: keep any new mapper local to review-pack output semantics; do not create a broad governance-output layer
|
|
|
|
## OperationRun UX Impact
|
|
|
|
- **Touches OperationRun start/completion/link UX?**: existing proof linkage and audit/download context only
|
|
- **Central contract reused**: existing operation proof links and current generation lifecycle
|
|
- **Delegated UX behaviors**: unchanged
|
|
- **Surface-owned behavior kept local**: limitations copy and qualified next-action selection
|
|
- **Queued DB-notification policy**: unchanged
|
|
- **Terminal notification path**: unchanged
|
|
- **Exception path**: none
|
|
|
|
## Provider Boundary & Portability Fit
|
|
|
|
- **Shared provider/platform boundary touched?**: no new provider seam
|
|
- **Provider-owned seams**: N/A
|
|
- **Platform-core seams**: review output contract, customer-safe sharing boundary, export readiness semantics
|
|
- **Neutral platform terms / contracts preserved**: review pack, evidence basis, limitations, customer-safe, internal package, output contract
|
|
- **Retained provider-specific semantics and why**: only in existing stored report or review content where already repo-backed
|
|
- **Bounded extraction or follow-up path**: none
|
|
|
|
## Current Repo Truth Summary
|
|
|
|
- `EnvironmentReviewComposer` already derives:
|
|
- `summary.evidence_basis`
|
|
- `summary.section_state_counts`
|
|
- `summary.publish_blockers`
|
|
- `summary.has_ready_export` (initially `false`)
|
|
- `summary.governance_package`
|
|
- `GenerateReviewPackJob` already writes review-derived ZIP files:
|
|
- `metadata.json`
|
|
- `summary.json`
|
|
- `sections.json`
|
|
- `executive-summary.md`
|
|
- one file per section under `sections/`
|
|
- `GenerateReviewPackJob` already updates:
|
|
- `ReviewPack.summary.review_status`
|
|
- `ReviewPack.summary.review_completeness_state`
|
|
- `ReviewPack.summary.delivery_bundle`
|
|
- `ReviewPack.summary.evidence_resolution`
|
|
- `EnvironmentReview.summary.has_ready_export = true` after successful generation
|
|
- `CustomerReviewWorkspace` currently derives share-readiness from:
|
|
- accepted-risk follow-up
|
|
- open findings
|
|
- package availability
|
|
- evidence availability
|
|
- mapped review data
|
|
- not from a dedicated output-readiness contract
|
|
- Current gaps:
|
|
- section-detail files do not currently repeat `section_key`, `required`, or `sort_order`
|
|
- executive summary does not have a dedicated limitations block
|
|
- workspace UI does not surface `include_pii` or redaction state
|
|
- workspace UI does not consume `has_ready_export` as a first-class output-readiness input
|
|
- current ready/share labels are stronger than the current explicit bundle contract
|
|
- existing executive-pack and localization regressions were not yet listed in the original Spec 347 validation matrix even though this slice is expected to change those surfaces
|
|
|
|
## Implementation Approach
|
|
|
|
### Phase 0 - Repo Truth Gate
|
|
|
|
1. Re-read this spec, plan, tasks, repo-truth map, and contract docs before runtime changes.
|
|
2. Re-check current generator and workspace files before editing:
|
|
- `apps/platform/app/Jobs/GenerateReviewPackJob.php`
|
|
- `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewComposer.php`
|
|
- `apps/platform/app/Services/ReviewPackService.php`
|
|
- `apps/platform/app/Filament/Pages/Reviews/CustomerReviewWorkspace.php`
|
|
- `apps/platform/resources/views/filament/pages/reviews/customer-review-workspace.blade.php`
|
|
- `apps/platform/app/Http/Controllers/ReviewPackDownloadController.php`
|
|
3. Keep `specs/347-review-pack-output-contract-readiness-semantics/repo-truth-map.md` current if runtime edits reveal additional truth or limitations.
|
|
4. Preserve repo-truth deviations from the user draft explicitly:
|
|
- section-detail files live under `sections/`
|
|
- current page report identity is `ui-006-customer-review-workspace.md`
|
|
|
|
### Phase 1 - Contract Docs First
|
|
|
|
1. Finalize spec-local contract docs before runtime edits:
|
|
- `contracts/review-pack-output-contract.md`
|
|
- `contracts/readiness-semantics.md`
|
|
- `contracts/customer-safe-output-boundary.md`
|
|
2. Make the contracts explicit about:
|
|
- file layout
|
|
- required fields
|
|
- meaning of `missing`
|
|
- evidence vs export vs customer-safe vs internal-only states
|
|
- PII/redaction visibility
|
|
3. Treat those docs as the runtime review checklist, not as parallel product logic.
|
|
|
|
### Phase 2 - Tests First
|
|
|
|
1. Add focused contract tests:
|
|
- `apps/platform/tests/Feature/ReviewPack/Spec347ReviewPackOutputContractTest.php`
|
|
- `apps/platform/tests/Feature/ReviewPack/Spec347ReviewPackReadinessSemanticsTest.php`
|
|
- `apps/platform/tests/Feature/Filament/Spec347CustomerReviewWorkspaceOutputReadinessTest.php`
|
|
- `apps/platform/tests/Browser/Spec347ReviewPackOutputReadinessSmokeTest.php`
|
|
2. Reuse existing helpers and extend current review-pack/customer-review tests only where proportional.
|
|
3. Re-run existing regressions that already pin the executive entrypoint, workspace wording, browser readiness path, and localization:
|
|
- `apps/platform/tests/Feature/EnvironmentReview/EnvironmentReviewExecutivePackTest.php`
|
|
- `apps/platform/tests/Feature/Localization/CustomerReviewSurfaceLocalizationTest.php`
|
|
- `apps/platform/tests/Feature/Filament/Spec342CustomerReviewWorkspaceConsumptionTest.php`
|
|
- `apps/platform/tests/Browser/Spec342CustomerReviewWorkspaceConsumptionSmokeTest.php`
|
|
4. Lock the following before runtime refactor:
|
|
- required root files
|
|
- required metadata/summary fields
|
|
- section/file consistency
|
|
- limitations/disclosure
|
|
- no false share-ready labels
|
|
- PII visibility
|
|
- preserved signed-download safety
|
|
|
|
### Phase 3 - Derived Output-Readiness Mapper
|
|
|
|
1. Choose the narrowest home for derived output readiness:
|
|
- page-local helper if truly local
|
|
- small support-layer mapper only if both ZIP contract and workspace rendering must share it
|
|
2. Derive, do not persist:
|
|
- readiness label
|
|
- primary reason
|
|
- primary action
|
|
- evidence basis state
|
|
- section completeness summary
|
|
- customer-safe/internal-only/limitations state
|
|
- PII/redaction visibility
|
|
3. Prefer reuse of current review summary and pack summary fields over new fields.
|
|
4. Only add new payload keys where current structures genuinely cannot express the needed contract.
|
|
|
|
### Phase 4 - ZIP Contract Hardening
|
|
|
|
1. Keep the current required root files and current `sections/` detail-file placement unless runtime proof forces a narrower change.
|
|
2. Tighten `metadata.json` so required bundle/review/evidence/options/redaction fields are always present.
|
|
3. Tighten `summary.json` so readiness-related fields are explicit and stable.
|
|
4. Decide the canonical section contract:
|
|
- either add `section_key`, `required`, and `sort_order` to each section-detail file
|
|
- or keep `sections.json` as the canonical section index and document the detail files as thinner subordinate payloads
|
|
5. Preserve the existing delivery contract constant unless a repo-justified contract-version bump is necessary.
|
|
|
|
### Phase 5 - Executive Summary Hardening
|
|
|
|
1. Add an explicit `## Limitations` block whenever output readiness is limited by:
|
|
- evidence completeness
|
|
- required section completeness
|
|
- export readiness
|
|
- PII/internal-only boundary
|
|
2. Keep the existing non-certification disclosure visible.
|
|
3. Avoid raw/internal-only detail in the markdown entrypoint.
|
|
4. Ensure the executive summary explains that a section file may exist even when the section is marked `missing`.
|
|
|
|
### Phase 6 - Customer Review Workspace Remap
|
|
|
|
1. Replace unqualified share-ready language with contract-backed labels when the output contract is incomplete.
|
|
2. Surface:
|
|
- evidence basis state
|
|
- section completeness summary
|
|
- PII/redaction visibility
|
|
- qualified customer-safe/internal-only/limitations state
|
|
3. Keep one dominant next action:
|
|
- review limitations
|
|
- open review
|
|
- qualified download
|
|
4. Keep diagnostics collapsed and secondary.
|
|
5. Do not redesign the page beyond bounded readiness/disclosure hardening.
|
|
|
|
### Phase 7 - Copy, Audit, And Browser Proof
|
|
|
|
1. Update `apps/platform/lang/en/localization.php` and `apps/platform/lang/de/localization.php` only for the new qualified readiness vocabulary needed by repo truth.
|
|
2. Update `docs/ui-ux-enterprise-audit/page-reports/ui-006-customer-review-workspace.md` with:
|
|
- output contract summary
|
|
- readiness label mapping
|
|
- limitations and PII visibility expectations
|
|
- deferred follow-ups
|
|
Review Pack Resource detail/header coverage is not part of this slice unless a minimal contradiction fix becomes unavoidable.
|
|
3. Capture screenshots under `specs/347-review-pack-output-contract-readiness-semantics/artifacts/screenshots/`.
|
|
4. Use one bounded browser smoke proving:
|
|
- limitations-bearing package
|
|
- qualified download label
|
|
- no unqualified share-ready claim
|
|
- PII warning where repo-backed
|
|
|
|
### Phase 8 - Validation And Close-Out
|
|
|
|
1. Run the narrowest focused Feature and Browser commands.
|
|
2. Re-run overlapping regressions for `ReviewPack` and `CustomerReviewWorkspace`.
|
|
3. Run `pint --dirty` and `git diff --check`.
|
|
4. Record unrelated failures honestly if broader regressions are not green.
|
|
5. Do not widen scope into portal, lifecycle-governance, or localization-wide cleanup.
|
|
|
|
## Deployment / Ops Impact
|
|
|
|
- **Env vars**: none expected
|
|
- **Migrations**: none expected
|
|
- **Queues/scheduler**: no new queue family or scheduler change expected
|
|
- **Storage/volumes**: existing exports disk only; no storage topology change expected
|
|
- **Filament assets**: none expected; if any registered Filament assets unexpectedly appear, deployment must include `cd apps/platform && php artisan filament:assets`
|
|
|
|
## Filament / Laravel Guardrails
|
|
|
|
- **Livewire v4 compliance**: required; no Livewire v3 APIs
|
|
- **Panel provider registration**: remains `apps/platform/bootstrap/providers.php`
|
|
- **Global search**: no resource global-search change is expected
|
|
- **Destructive/high-impact actions**: no new destructive action is planned. Existing regenerate/expire actions remain governed by current confirmation, authorization, and audit rules if touched at all.
|
|
- **Asset strategy**: no new panel asset strategy expected
|
|
|
|
## Test Governance Check
|
|
|
|
- **Test purpose / classification by changed surface**: Feature for ZIP contract and workspace state mapping; Browser for first-screen readiness wording and qualified download proof
|
|
- **Affected validation lanes**: confidence + browser
|
|
- **Why this lane mix is the narrowest sufficient proof**: file contracts and workspace rendering are deterministic and can be proven with focused tests; one browser path covers the strategic customer-safe trust surface
|
|
- **Narrowest proving command(s)**:
|
|
|
|
```bash
|
|
cd apps/platform
|
|
./vendor/bin/sail artisan test tests/Feature/ReviewPack/Spec347ReviewPackOutputContractTest.php tests/Feature/ReviewPack/Spec347ReviewPackReadinessSemanticsTest.php tests/Feature/Filament/Spec347CustomerReviewWorkspaceOutputReadinessTest.php --compact
|
|
./vendor/bin/sail artisan test tests/Feature/EnvironmentReview/EnvironmentReviewExecutivePackTest.php tests/Feature/Localization/CustomerReviewSurfaceLocalizationTest.php tests/Feature/Filament/Spec342CustomerReviewWorkspaceConsumptionTest.php --compact
|
|
./vendor/bin/sail php vendor/bin/pest tests/Browser/Spec347ReviewPackOutputReadinessSmokeTest.php tests/Browser/Spec342CustomerReviewWorkspaceConsumptionSmokeTest.php --compact
|
|
./vendor/bin/sail artisan test --compact --filter=ReviewPack
|
|
./vendor/bin/sail artisan test --compact --filter=CustomerReviewWorkspace
|
|
./vendor/bin/sail pint --dirty
|
|
git diff --check
|
|
```
|
|
|
|
- **Fixture / helper / factory / seed / context cost risks**: reuse existing review/evidence/review-pack helpers; do not add a heavy default browser or feature fixture layer
|
|
- **Expensive defaults or shared helper growth introduced?**: none expected
|
|
- **Heavy-family additions, promotions, or visibility changes**: one explicit browser smoke only
|
|
- **Surface-class relief / special coverage rule**: no relief; this is a strategic customer-safe trust surface
|
|
- **Closing validation and reviewer handoff**: confirm no false-ready claims, explicit limitations, explicit PII/redaction visibility, preserved signed-download safety, and no new persistence/framework
|
|
- **Budget / baseline / trend follow-up**: none expected beyond one explicit browser smoke
|
|
- **Review-stop questions**:
|
|
- Did the implementation introduce a new persisted readiness truth?
|
|
- Did it invent a second output-readiness dialect instead of aligning ZIP and workspace?
|
|
- Did it weaken download safety?
|
|
- Did it overclaim customer-safe or certification semantics?
|
|
- **Escalation path**: `document-in-feature` for unreachable states; `follow-up-spec` only for broader artifact lifecycle issues
|