TenantAtlas/specs/308-decision-register-summary-review-pack/plan.md
ahmido 77c343fb35 feat: implement decision register summary in environment review packs (#363)
## Summary
- add decision register summary output to environment review packs
- update environment review evidence composition and localized summary rendering
- add coverage for executive pack and derived review pack behavior
- include spec artifacts for feature 308

## Testing
- cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/EnvironmentReview/EnvironmentReviewExecutivePackTest.php tests/Feature/ReviewPack/EnvironmentReviewDerivedReviewPackTest.php

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #363
2026-05-15 12:54:41 +00:00

288 lines
20 KiB
Markdown

# Implementation Plan: Decision Register Customer-Safe Summary & Review-Pack Inclusion
**Branch**: `308-decision-register-summary-review-pack` | **Date**: 2026-05-15 | **Spec**: [spec.md](spec.md)
**Input**: Feature specification from `/specs/308-decision-register-summary-review-pack/spec.md`
## Summary
Add a bounded customer-safe Decision Register summary to existing released-review governance-package truth and review-derived Review Pack exports. The implementation should extend current Environment Review and Review Pack composition paths, not create a new decision store, export subsystem, customer approval workflow, or OperationRun lifecycle.
## Technical Context
**Language/Version**: PHP 8.4.15, Laravel 12.52.0
**Primary Dependencies**: Filament 5.2.1, Livewire 4.1.4, Pest 4.3.1, Laravel Sail
**Storage**: PostgreSQL; existing JSONB fields on `environment_reviews.summary`, `review_packs.summary`, and generated files on the existing `exports` disk
**Testing**: Pest 4 feature tests and optional bounded browser smoke
**Validation Lanes**: confidence; browser only if rendered UI changes
**Target Platform**: Laravel Sail locally; Dokploy container deployment for staging/production
**Project Type**: single Laravel app under `apps/platform`
**Performance Goals**: Derived summary composition remains ordinary review/pack work; no new query-heavy global scan
**Constraints**: no migrations expected; no new assets; no new operation type; no customer-safe raw diagnostic leakage
**Scale/Scope**: one managed-environment released-review and review-pack flow at a time
## UI / Surface Guardrail Plan
- **Guardrail scope**: changed customer-safe review/package surfaces and exported artifact content.
- **Native vs custom classification summary**: native/shared existing Filament surfaces plus existing export files.
- **Shared-family relevance**: customer review consumption, governance-package summary, Review Pack artifact truth, evidence/report disclosure, Decision Register context.
- **State layers in scope**: page, detail, artifact content; no shell change.
- **Audience modes in scope**: customer/read-only and operator-MSP.
- **Decision/diagnostic/raw hierarchy plan**: customer-safe decision summary first, existing detail/sections second, raw/support diagnostics absent from default customer paths.
- **Raw/support gating plan**: no raw OperationRun URLs, fingerprints, platform reason families, or provider payloads in customer-safe summary/export.
- **One-primary-action / duplicate-truth control**: preserve existing `Open review`, `Download governance package`, and `Download` actions; summary content must not add peer operator-only actions.
- **Handling modes by drift class or surface**: review-mandatory for customer-safe copy/export changes; report-only for unchanged Decision Register page.
- **Repository-signal treatment**: review-mandatory because this touches customer-safe review consumption and exported artifacts.
- **Special surface test profiles**: shared-detail-family plus standard-native-filament.
- **Required tests or manual smoke**: focused feature tests; existing bounded browser smoke if rendered customer workspace/review detail changes.
- **Exception path and spread control**: none planned.
- **Active feature PR close-out entry**: Guardrail / Smoke Coverage.
## Shared Pattern & System Fit
- **Cross-cutting feature marker**: yes.
- **Systems touched**: `EnvironmentReviewComposer`, `EnvironmentReviewSectionFactory`, `CustomerReviewWorkspace`, `EnvironmentReviewResource`, `ReviewPackService`, `GenerateReviewPackJob`, `ReviewPackResource`, review/review-pack tests.
- **Shared abstractions reused**: existing `governance_package` summary, existing `auditor_ready_executive_export.v1` delivery contract, existing `ReviewPackService::generateFromReview()`, existing `executive-summary.md` generation, existing BADGE-001 rendering if UI labels need badges.
- **New abstraction introduced? why?**: none planned. Private methods in existing classes are acceptable if needed to keep summary derivation readable.
- **Why the existing abstraction was sufficient or insufficient**: Existing review and pack composition paths already own customer-safe summary and export truth; they lack explicit Decision Register follow-through proof. Existing operator register builder is intentionally too operator-focused for direct customer export.
- **Bounded deviation / spread control**: no public framework or new artifact family. Any new summary keys stay nested under existing review/pack summary payloads.
## OperationRun UX Impact
- **Touches OperationRun start/completion/link UX?**: no new start/completion/link UX. Existing review-pack generation run behavior remains.
- **Central contract reused**: existing `ReviewPackService`, `GenerateReviewPackJob`, `OperationRunService`, and existing `OperationRunLinks` usage where already present.
- **Delegated UX behaviors**: existing Review Pack generation / run feedback only.
- **Surface-owned behavior kept local**: customer-safe copy may mention evidence/package availability, not run diagnostics.
- **Queued DB-notification policy**: N/A.
- **Terminal notification path**: existing Review Pack terminal handling remains.
- **Exception path**: none.
## Provider Boundary & Portability Fit
- **Shared provider/platform boundary touched?**: no.
- **Provider-owned seams**: none.
- **Platform-core seams**: review/governance-package summary and review-pack export artifact content.
- **Neutral platform terms / contracts preserved**: governance decision, accepted risk, evidence basis, review, review pack, managed environment, workspace.
- **Retained provider-specific semantics and why**: existing finding/evidence titles may include provider context; this feature does not add provider coupling.
- **Bounded extraction or follow-up path**: none.
## Constitution Check
- Inventory-first: PASS. The feature uses existing evidence/review snapshot truth; it does not query Graph or change inventory behavior.
- Read/write separation: PASS. Customer-safe summary is derived/read-only. Existing review-pack generation remains the only write path and already uses confirmation/authorization semantics where applicable.
- Graph contract path: PASS. No Graph calls or contracts are touched.
- Deterministic capabilities: PASS. Existing review/review-pack capabilities remain.
- RBAC-UX: PASS. Existing workspace/environment membership and review/review-pack policy behavior remain server-side boundaries.
- Workspace isolation: PASS. Summary derivation must scope to the released review workspace.
- Tenant/environment isolation: PASS. Summary derivation must scope to the managed environment under review.
- Run observability: PASS. Existing Review Pack `OperationRun` remains; no new run type or local run UX.
- TEST-GOV-001: PASS with focused feature coverage and optional bounded browser smoke.
- PROP-001 / BLOAT-001: PASS. No new table, enum family, service framework, or export family.
- PERSIST-001: PASS. Existing review and review-pack artifacts receive derived content; no new persisted truth.
- STATE-001: PASS. No new lifecycle/status family.
- UI-SEM-001: PASS. Customer-safe copy stays a derived summary, not a UI framework.
- XCUT-001: PASS. Reuses existing review and review-pack shared paths.
- PROV-001: PASS. No provider seam changes.
- UI-FIL-001: PASS. Existing Filament-native surfaces stay native/shared-primitives first; no ad-hoc CSS or new assets expected.
- DECIDE-AUD-001 / OPSURF-001: PASS. Customer-safe summary excludes raw/support/debug content by default.
## Test Governance Check
- **Test purpose / classification by changed surface**: Feature tests for review summary composition, review-pack ZIP content, customer workspace/review-pack access, redaction, and isolation. Browser smoke only if rendered UI changes.
- **Affected validation lanes**: confidence; browser optional/bounded.
- **Why this lane mix is the narrowest sufficient proof**: Export content can be verified by reading generated ZIP entries; rendered summary and access boundaries can be verified through existing feature tests. A broad suite or new browser family is not justified.
- **Narrowest proving command(s)**:
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/EnvironmentReview/EnvironmentReviewExecutivePackTest.php tests/Feature/EnvironmentReview/EnvironmentReviewCreationTest.php`
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/EnvironmentReviewDerivedReviewPackTest.php tests/Feature/ReviewPack/ReviewPackResourceTest.php`
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Reviews/CustomerReviewWorkspacePageTest.php tests/Feature/Reviews/CustomerReviewWorkspacePackAccessTest.php`
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Browser/Reviews/CustomerReviewWorkspaceSmokeTest.php` if rendered UI changes
- `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent`
- `git diff --check`
- **Fixture / helper / factory / seed / context cost risks**: use existing helpers such as `composeEnvironmentReviewForTest()` and review-pack factories; avoid widening defaults.
- **Expensive defaults or shared helper growth introduced?**: no.
- **Heavy-family additions, promotions, or visibility changes**: none.
- **Surface-class relief / special coverage rule**: standard-native-filament plus shared-detail-family review/export checks.
- **Closing validation and reviewer handoff**: verify summary content, redaction, no raw diagnostics, no cross-scope leakage, unchanged destructive action confirmation, and unchanged asset strategy.
- **Budget / baseline / trend follow-up**: none expected.
- **Review-stop questions**: Are customer-safe summaries derived from the correct source? Are hidden records excluded? Is raw diagnostic detail absent? Did tests stay focused?
- **Escalation path**: none.
- **Active feature PR close-out entry**: Guardrail / Smoke Coverage.
- **Why no dedicated follow-up spec is needed**: This is a bounded productization pass over existing review/pack paths; broader localization, packaging cadence, and artifact lifecycle remain separate candidates.
## Project Structure
### Documentation (this feature)
```text
specs/308-decision-register-summary-review-pack/
|-- spec.md
|-- plan.md
|-- tasks.md
`-- checklists/
`-- requirements.md
```
### Source Code (likely affected later)
```text
apps/platform/app/Services/EnvironmentReviews/
|-- EnvironmentReviewComposer.php
`-- EnvironmentReviewSectionFactory.php
apps/platform/app/Jobs/
`-- GenerateReviewPackJob.php
apps/platform/app/Services/
`-- ReviewPackService.php
apps/platform/app/Filament/Pages/Reviews/
`-- CustomerReviewWorkspace.php
apps/platform/app/Filament/Resources/
|-- EnvironmentReviewResource.php
`-- ReviewPackResource.php
apps/platform/tests/Feature/EnvironmentReview/
apps/platform/tests/Feature/ReviewPack/
apps/platform/tests/Feature/Reviews/
apps/platform/tests/Browser/Reviews/CustomerReviewWorkspaceSmokeTest.php
```
**Structure Decision**: Single Laravel/Filament application under `apps/platform`. Later implementation should modify only existing review, review-pack, and focused test surfaces unless repo inspection proves a smaller private helper is necessary.
## Complexity Tracking
| Violation | Why Needed | Simpler Alternative Rejected Because |
|---|---|---|
| None planned | N/A | N/A |
## Technical Approach
1. Inspect current `EnvironmentReviewComposer::governancePackageSummary()` and `GenerateReviewPackJob::buildExecutiveEntrypoint()` behavior.
2. Add or refine a customer-safe decision-summary shape under existing `EnvironmentReview.summary['governance_package']`.
3. Keep existing `governance_decisions` entries aligned with accepted-risk / exception decision truth and add explicit count/empty/unavailable semantics if missing.
4. Ensure `GenerateReviewPackJob` includes the same summary in `summary.json` and readable markdown in `executive-summary.md`.
5. Update existing customer workspace/review detail presentation only if current surfaces do not expose the summary clearly.
6. Preserve redaction through existing `redactReportPayload()` behavior and add assertions for `include_pii=false`.
7. Add focused tests for positive, none, incomplete, cross-scope, and redaction scenarios.
## Data / Model Implications
- No migration expected.
- No new model expected.
- No new enum/status family expected.
- Existing JSONB summary fields may receive additive keys:
- `governance_package.decision_summary`
- or a clarified `governance_package.governance_decisions` shape
- Existing ZIP entries remain preferred:
- `summary.json`
- `executive-summary.md`
- `sections.json`
If implementation finds a new table, new artifact file family, or new public contract is necessary, stop and update the spec/plan before coding further.
## UI / Filament Implications
- Filament v5.2.1 and Livewire v4.1.4 remain the target.
- No panel provider changes; Laravel 12 provider registration remains in `bootstrap/providers.php`.
- No new globally searchable resource.
- No new destructive actions.
- Existing Review Pack destructive-like actions such as expire/regenerate must keep `->requiresConfirmation()` and existing authorization.
- No new assets; deployment `filament:assets` unchanged.
- If UI changes are needed, prefer existing Filament sections/infolists/table entries and shared badge primitives.
## Livewire Implications
- Existing Filament/Livewire pages may render updated summary content.
- No new Livewire component is expected.
- Avoid server-driven reactivity changes unless current page state already requires it.
## RBAC / Policy Implications
- Reuse current review and review-pack policies/capabilities.
- No new capability constant expected.
- Customer-safe content must derive only after workspace and managed-environment scope are established.
- Non-member/not-entitled remains `404`; member missing capability remains existing `403`.
- Tests must cover at least one hidden/cross-scope omission path.
## Audit / Logging / Evidence Implications
- No new audit action ID expected.
- Existing review pack request/export/download audit and telemetry remain.
- No secrets, raw JSON, fingerprints, internal reason ownership, or OperationRun URLs in customer-safe default content.
- Existing `ReviewPack`, `EnvironmentReview`, `EvidenceSnapshot`, and `OperationRun` links remain traceability for operators.
## Rollout Considerations
- No migration.
- No new env vars.
- Queue workers already process review-pack jobs; no new worker class expected beyond current job changes.
- Staging validation should generate at least one review-derived pack with decisions requiring awareness and one with no decisions.
- Production/Dokploy deploy impact is ordinary app code plus queue restart if job code changes.
## Risk Controls
- Keep customer-safe summary derived and additive.
- Keep operator Decision Register unchanged unless a test proves a small source alignment issue.
- Reject raw OperationRun/proof URL exposure in customer-safe summary.
- Preserve non-certification disclosure in `executive-summary.md`.
- Keep tests focused on current review, pack, and customer workspace families.
## Implementation Phases
### Phase 1 - Discovery and Failing Tests
Confirm current summary/export behavior, then add focused tests for missing customer-safe decision summary, export inclusion, redaction, and cross-scope omission.
### Phase 2 - Summary Composition
Extend existing Environment Review governance-package summary with bounded customer-safe decision summary content.
### Phase 3 - Review Pack Inclusion
Ensure review-derived Review Pack `summary.json` and `executive-summary.md` include matching customer-safe decision summary content.
### Phase 4 - Customer Surface Presentation
Expose the summary on existing customer-safe review surfaces only where needed and only through native/shared Filament patterns.
### Phase 5 - Validation and Close-Out
Run focused Pest commands, optional bounded browser smoke, Pint dirty, and `git diff --check`. Record no new assets, no migrations, and no application implementation outside scope.
## Spec Readiness Gate
- `spec.md`, `plan.md`, `tasks.md`, and `checklists/requirements.md` exist.
- Scope is bounded to existing Decision Register context, Environment Review summary, Customer Review Workspace/review detail, and review-derived Review Pack export.
- No open question blocks implementation.
- No new persisted entity, new status family, new public framework, new OperationRun type, or new asset bundle is planned.
- RBAC, isolation, auditability, OperationRun semantics, evidence/result truth, customer-safe disclosure, and test governance are addressed.
## Implementation Close-Out
- **Implementation status**: Completed for Spec 308.
- **Changed application files**:
- `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewComposer.php`
- `apps/platform/app/Services/Evidence/Sources/FindingsSummarySource.php`
- `apps/platform/app/Jobs/GenerateReviewPackJob.php`
- `apps/platform/resources/views/filament/infolists/entries/environment-review-summary.blade.php`
- `apps/platform/lang/en/localization.php`
- `apps/platform/lang/de/localization.php`
- **Changed tests**:
- `apps/platform/tests/Feature/EnvironmentReview/EnvironmentReviewExecutivePackTest.php`
- `apps/platform/tests/Feature/ReviewPack/EnvironmentReviewDerivedReviewPackTest.php`
- **No-migration status**: No database migration, new model, new enum/status family, new persisted decision source, or new Review Pack status was introduced.
- **No-asset status**: No frontend asset registration was added; existing deployment `filament:assets` expectations are unchanged.
- **OperationRun / audit status**: No new OperationRun type, run-link surface, or audit action ID was added. Existing review-pack generation/download traceability remains unchanged.
- **Validation results**:
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/EnvironmentReview/EnvironmentReviewExecutivePackTest.php tests/Feature/EnvironmentReview/EnvironmentReviewCreationTest.php` - passed, 5 tests / 68 assertions.
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/EnvironmentReviewDerivedReviewPackTest.php tests/Feature/ReviewPack/ReviewPackResourceTest.php` - passed, 22 tests / 142 assertions.
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Reviews/CustomerReviewWorkspacePageTest.php tests/Feature/Reviews/CustomerReviewWorkspacePackAccessTest.php` - passed, 12 tests / 77 assertions.
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Browser/Reviews/CustomerReviewWorkspaceSmokeTest.php` - passed, 1 test / 46 assertions.
- Additional regression lane: `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Unit/EnvironmentReview/EnvironmentReviewComposerTest.php tests/Feature/Evidence/ExceptionValidityEvidenceIntegrationTest.php tests/Feature/ReviewPack/ReviewPackValidRiskAcceptanceTest.php` - passed, 6 tests / 27 assertions.
- Additional RBAC/download lane: `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/ReviewPackDownloadTest.php tests/Feature/ReviewPack/ReviewPackRbacTest.php` - passed, 20 tests / 49 assertions.
- `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent` - passed.
- `git diff --check` - passed.
- **Browser smoke result**: Required because rendered customer review detail UI changed. Existing bounded customer workspace browser smoke passed.
- **Post-implementation analysis**: No confirmed in-scope findings remain after the first implementation/fix iteration.
- **Remaining gaps**: None inside Spec 308 scope. Localization adoption, artifact lifecycle/retention, governance-service packaging cadence, and AI-assisted review drafting remain separate follow-up candidates and were not implemented.