From 77c68d2d90bd93a3a77fc4961779fc09b6adfb99 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Sat, 6 Jun 2026 16:41:10 +0200 Subject: [PATCH] feat: implement review compose reconciliation adapter (spec 359) --- .../Resources/OperationRunResource.php | 6 + .../app/Jobs/ComposeEnvironmentReviewJob.php | 80 +++- apps/platform/app/Models/OperationRun.php | 31 ++ .../app/Services/AdapterRunReconciler.php | 97 ++++- .../EnvironmentReviewService.php | 99 ++++- .../app/Services/OperationRunService.php | 34 +- .../app/Support/OperationRunLinks.php | 21 +- .../EnvironmentReviewComposeDecision.php | 334 +++++++++++++++ .../Reconciliation/ReconciliationResult.php | 218 ++++++++++ .../Support/OpsUx/OperationUxPresenter.php | 85 ++++ .../ArtifactTruthPresenter.php | 26 +- ...59ReviewComposeReconciliationSmokeTest.php | 234 +++++++++++ .../Spec359ReviewComposeIdempotencyTest.php | 73 ++++ ...Spec359ReviewComposeReconciliationTest.php | 220 ++++++++++ .../RecentOperationsSummaryWidgetTest.php | 42 +- ...tionLifecycleFreshnessPresentationTest.php | 42 ++ .../OperationRunNotificationTest.php | 47 +++ ...9OperationRunAdapterReconciliationTest.php | 125 ++++++ .../System/Spec114/OpsFailuresViewTest.php | 44 ++ ...AdapterRunReconcilerSupportedTypesTest.php | 18 + ...359EnvironmentReviewComposeAdapterTest.php | 161 ++++++++ .../Spec359ReconciliationResultTest.php | 62 +++ .../checklists/requirements.md | 64 +++ .../plan.md | 273 +++++++++++++ .../spec.md | 379 ++++++++++++++++++ .../tasks.md | 209 ++++++++++ 26 files changed, 2979 insertions(+), 45 deletions(-) create mode 100644 apps/platform/app/Support/Operations/Reconciliation/EnvironmentReviewComposeDecision.php create mode 100644 apps/platform/app/Support/Operations/Reconciliation/ReconciliationResult.php create mode 100644 apps/platform/tests/Browser/Spec359ReviewComposeReconciliationSmokeTest.php create mode 100644 apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeIdempotencyTest.php create mode 100644 apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeReconciliationTest.php create mode 100644 apps/platform/tests/Feature/Operations/Spec359OperationRunAdapterReconciliationTest.php create mode 100644 apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359AdapterRunReconcilerSupportedTypesTest.php create mode 100644 apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359EnvironmentReviewComposeAdapterTest.php create mode 100644 apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359ReconciliationResultTest.php create mode 100644 specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/checklists/requirements.md create mode 100644 specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/plan.md create mode 100644 specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/spec.md create mode 100644 specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/tasks.md diff --git a/apps/platform/app/Filament/Resources/OperationRunResource.php b/apps/platform/app/Filament/Resources/OperationRunResource.php index 4130421d..3c6a8866 100644 --- a/apps/platform/app/Filament/Resources/OperationRunResource.php +++ b/apps/platform/app/Filament/Resources/OperationRunResource.php @@ -1384,6 +1384,12 @@ private static function reconciliationHeadline(OperationRun $record): ?string return null; } + $reviewComposeHeadline = \App\Support\OpsUx\OperationUxPresenter::reviewComposeReconciliationCopy($record)['headline'] ?? null; + + if (is_string($reviewComposeHeadline) && trim($reviewComposeHeadline) !== '') { + return trim($reviewComposeHeadline); + } + return 'TenantPilot force-resolved this run after normal lifecycle truth was lost.'; } diff --git a/apps/platform/app/Jobs/ComposeEnvironmentReviewJob.php b/apps/platform/app/Jobs/ComposeEnvironmentReviewJob.php index 3c418efe..9ab1d801 100644 --- a/apps/platform/app/Jobs/ComposeEnvironmentReviewJob.php +++ b/apps/platform/app/Jobs/ComposeEnvironmentReviewJob.php @@ -5,14 +5,20 @@ namespace App\Jobs; use App\Jobs\Concerns\BridgesFailedOperationRun; -use App\Models\OperationRun; use App\Models\EnvironmentReview; +use App\Models\OperationRun; +use App\Services\AdapterRunReconciler; use App\Services\OperationRunService; use App\Services\EnvironmentReviews\EnvironmentReviewService; +use App\Support\OpsUx\RunFailureSanitizer; use App\Support\OperationRunOutcome; use App\Support\OperationRunStatus; +use App\Support\Operations\LifecycleReconciliationReason; +use App\Support\Operations\Reconciliation\EnvironmentReviewComposeDecision; +use App\Support\Operations\Reconciliation\ReconciliationResult; use App\Support\EnvironmentReviewStatus; use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Database\QueryException; use Illuminate\Foundation\Queue\Queueable; use Throwable; @@ -30,7 +36,11 @@ public function __construct( public int $operationRunId, ) {} - public function handle(EnvironmentReviewService $service, OperationRunService $operationRuns): void + public function handle( + EnvironmentReviewService $service, + OperationRunService $operationRuns, + AdapterRunReconciler $adapterRunReconciler, + ): void { $review = EnvironmentReview::query()->with(['tenant', 'evidenceSnapshot.items'])->find($this->environmentReviewId); $operationRun = OperationRun::query()->find($this->operationRunId); @@ -39,6 +49,16 @@ public function handle(EnvironmentReviewService $service, OperationRunService $o return; } + if ((string) $operationRun->status === OperationRunStatus::Completed->value) { + return; + } + + $adapterChange = $adapterRunReconciler->reconcileOperationRun($operationRun); + + if (($adapterChange['applied'] ?? false) === true) { + return; + } + $operationRuns->updateRun($operationRun, OperationRunStatus::Running->value, OperationRunOutcome::Pending->value); $review->update(['status' => EnvironmentReviewStatus::Draft->value]); @@ -60,12 +80,51 @@ public function handle(EnvironmentReviewService $service, OperationRunService $o ], ); } catch (Throwable $throwable) { - $review->update([ - 'status' => EnvironmentReviewStatus::Failed->value, - 'summary' => array_merge(is_array($review->summary) ? $review->summary : [], [ - 'error' => $throwable->getMessage(), - ]), - ]); + $adapterChange = $adapterRunReconciler->reconcileOperationRun($operationRun); + + if (($adapterChange['applied'] ?? false) === true) { + return; + } + + if ($throwable instanceof QueryException && $this->isUniqueViolation($throwable)) { + $result = ReconciliationResult::attentionRequired( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'TenantPilot found matching review activity, but it could not be resolved automatically.', + evidence: [ + 'adapter' => EnvironmentReviewComposeDecision::ADAPTER, + 'exception_class' => class_basename($throwable), + 'workspace_id' => (int) $operationRun->workspace_id, + 'managed_environment_id' => (int) $operationRun->managed_environment_id, + 'fingerprint' => (string) data_get($operationRun->context, 'review_fingerprint', ''), + ], + ); + + $operationRuns->updateRunWithReconciliation( + run: $operationRun, + status: (string) $result->status, + outcome: (string) $result->outcome, + summaryCounts: $result->summaryCounts, + failures: $result->failures, + reasonCode: $result->reasonCode, + reasonMessage: $result->reasonMessage, + source: 'adapter_reconciler', + evidence: $result->evidence, + adapter: EnvironmentReviewComposeDecision::ADAPTER, + decision: $result->decision, + related: $result->related, + ); + + return; + } + + if ($review->isMutable() && (int) ($review->operation_run_id ?? 0) === (int) $operationRun->getKey()) { + $review->update([ + 'status' => EnvironmentReviewStatus::Failed->value, + 'summary' => array_merge(is_array($review->summary) ? $review->summary : [], [ + 'error' => RunFailureSanitizer::sanitizeMessage($throwable->getMessage()), + ]), + ]); + } $operationRuns->updateRun( $operationRun, @@ -82,4 +141,9 @@ public function handle(EnvironmentReviewService $service, OperationRunService $o throw $throwable; } } + + private function isUniqueViolation(QueryException $exception): bool + { + return in_array(($exception->errorInfo[0] ?? null), ['23505', '23000'], true); + } } diff --git a/apps/platform/app/Models/OperationRun.php b/apps/platform/app/Models/OperationRun.php index a03bdaf0..8594dc21 100644 --- a/apps/platform/app/Models/OperationRun.php +++ b/apps/platform/app/Models/OperationRun.php @@ -400,6 +400,37 @@ public function reconciliation(): array return is_array($reconciliation) ? $reconciliation : []; } + public function reconciliationDecision(): ?string + { + $decision = $this->reconciliation()['decision'] ?? null; + + return is_string($decision) && trim($decision) !== '' ? trim($decision) : null; + } + + public function reconciliationAdapter(): ?string + { + $adapter = $this->reconciliation()['adapter'] ?? null; + + return is_string($adapter) && trim($adapter) !== '' ? trim($adapter) : null; + } + + /** + * @return array + */ + public function reconciliationRelated(): array + { + $related = $this->reconciliation()['related'] ?? null; + + return is_array($related) ? $related : []; + } + + public function reconciledRelatedReviewId(): ?int + { + $reviewId = data_get($this->reconciliationRelated(), 'review.id'); + + return is_numeric($reviewId) ? (int) $reviewId : null; + } + public function isLifecycleReconciled(): bool { return $this->reconciliation() !== []; diff --git a/apps/platform/app/Services/AdapterRunReconciler.php b/apps/platform/app/Services/AdapterRunReconciler.php index cae33055..f7e14152 100644 --- a/apps/platform/app/Services/AdapterRunReconciler.php +++ b/apps/platform/app/Services/AdapterRunReconciler.php @@ -6,6 +6,7 @@ use App\Models\OperationRun; use App\Models\RestoreRun; +use App\Support\Operations\Reconciliation\EnvironmentReviewComposeDecision; use App\Support\OperationRunOutcome; use App\Support\OperationRunStatus; use App\Support\Operations\LifecycleReconciliationReason; @@ -15,6 +16,10 @@ final class AdapterRunReconciler { + public function __construct( + private readonly EnvironmentReviewComposeDecision $environmentReviewComposeDecision, + ) {} + /** * @return array */ @@ -22,6 +27,7 @@ public function supportedTypes(): array { return [ 'restore.execute', + 'environment.review.compose', ]; } @@ -46,7 +52,6 @@ public function reconcile(array $options = []): array $query = OperationRun::query() ->whereIn('type', $type ? [$type] : $this->supportedTypes()) ->whereIn('status', [OperationRunStatus::Queued->value, OperationRunStatus::Running->value]) - ->whereNotNull('context->restore_run_id') ->where(function (Builder $q) use ($cutoff): void { $q ->where(function (Builder $q2) use ($cutoff): void { @@ -70,7 +75,7 @@ public function reconcile(array $options = []): array $skipped = 0; foreach ($candidates as $run) { - $change = $this->reconcileOne($run, $dryRun); + $change = $this->reconcileOperationRun($run, $dryRun); if ($change === null) { $skipped++; @@ -96,12 +101,20 @@ public function reconcile(array $options = []): array /** * @return array|null */ - private function reconcileOne(OperationRun $run, bool $dryRun): ?array + public function reconcileOperationRun(OperationRun $run, bool $dryRun = false): ?array { - if ($run->type !== 'restore.execute') { - return null; - } + return match ((string) $run->type) { + 'restore.execute' => $this->reconcileRestoreRun($run, $dryRun), + 'environment.review.compose' => $this->reconcileReviewComposeRun($run, $dryRun), + default => null, + }; + } + /** + * @return array|null + */ + private function reconcileRestoreRun(OperationRun $run, bool $dryRun): ?array + { $context = is_array($run->context) ? $run->context : []; $restoreRunId = $context['restore_run_id'] ?? null; @@ -165,6 +178,7 @@ private function reconcileOne(OperationRun $run, bool $dryRun): ?array 'restore_run_id' => (int) $restoreRun->getKey(), 'restore_status' => $restoreStatus?->value, ], + adapter: 'restore_run', ); $run->refresh(); @@ -189,6 +203,77 @@ private function reconcileOne(OperationRun $run, bool $dryRun): ?array ]; } + /** + * @return array|null + */ + private function reconcileReviewComposeRun(OperationRun $run, bool $dryRun): ?array + { + $result = $this->environmentReviewComposeDecision->evaluate($run); + + if (! $result->shouldFinalizeRun()) { + if (! $dryRun) { + return null; + } + + return array_merge($result->toArray(), [ + 'applied' => false, + 'operation_run_id' => (int) $run->getKey(), + 'type' => (string) $run->type, + 'before' => [ + 'status' => (string) $run->status, + 'outcome' => (string) $run->outcome, + ], + ]); + } + + $before = [ + 'status' => (string) $run->status, + 'outcome' => (string) $run->outcome, + ]; + $after = [ + 'status' => (string) $result->status, + 'outcome' => (string) $result->outcome, + ]; + + if ($dryRun) { + return array_merge($result->toArray(), [ + 'applied' => false, + 'operation_run_id' => (int) $run->getKey(), + 'type' => (string) $run->type, + 'before' => $before, + 'after' => $after, + ]); + } + + /** @var OperationRunService $runs */ + $runs = app(OperationRunService::class); + + $runs->updateRunWithReconciliation( + run: $run, + status: (string) $result->status, + outcome: (string) $result->outcome, + summaryCounts: $result->summaryCounts, + failures: $result->failures, + reasonCode: $result->reasonCode, + reasonMessage: $result->reasonMessage, + source: 'adapter_reconciler', + evidence: $result->evidence, + adapter: EnvironmentReviewComposeDecision::ADAPTER, + decision: $result->decision, + related: $result->related, + ); + + $run->refresh(); + + return array_merge($result->toArray(), [ + 'applied' => true, + 'operation_run_id' => (int) $run->getKey(), + 'type' => (string) $run->type, + 'before' => $before, + 'after' => $after, + ]); + } + private function isTerminalRestoreStatus(?RestoreRunStatus $status): bool { if (! $status instanceof RestoreRunStatus) { diff --git a/apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewService.php b/apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewService.php index b5bf8eff..cd16200b 100644 --- a/apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewService.php +++ b/apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewService.php @@ -15,6 +15,7 @@ use App\Support\Audit\AuditActionId; use App\Support\OperationRunType; use App\Support\EnvironmentReviewStatus; +use Illuminate\Database\QueryException; use Illuminate\Support\Facades\DB; use InvalidArgumentException; @@ -177,28 +178,6 @@ private function queueComposition( initiator: $user, ); - $review ??= EnvironmentReview::query()->create([ - 'managed_environment_id' => (int) $tenant->getKey(), - 'workspace_id' => (int) $tenant->workspace_id, - 'evidence_snapshot_id' => (int) $snapshot->getKey(), - 'operation_run_id' => (int) $operationRun->getKey(), - 'initiated_by_user_id' => (int) $user->getKey(), - 'fingerprint' => $fingerprint, - 'status' => EnvironmentReviewStatus::Draft->value, - 'completeness_state' => (string) $snapshot->completeness_state, - 'summary' => [ - 'evidence_basis' => [ - 'snapshot_id' => (int) $snapshot->getKey(), - 'snapshot_fingerprint' => (string) $snapshot->fingerprint, - 'snapshot_completeness_state' => (string) $snapshot->completeness_state, - 'snapshot_generated_at' => $snapshot->generated_at?->toIso8601String(), - ], - 'publish_blockers' => [], - 'has_ready_export' => false, - 'last_requested_at' => now()->toIso8601String(), - ], - ]); - if ($existingReview instanceof EnvironmentReview) { $existingReview->forceFill([ 'evidence_snapshot_id' => (int) $snapshot->getKey(), @@ -210,6 +189,18 @@ private function queueComposition( $review = $existingReview->refresh(); } + if (! $review instanceof EnvironmentReview) { + $review = $this->findReviewForOperationRun($operationRun, $tenant, $fingerprint); + } + + $review ??= $this->createQueuedReview( + tenant: $tenant, + snapshot: $snapshot, + user: $user, + operationRun: $operationRun, + fingerprint: $fingerprint, + ); + if ($operationRun->wasRecentlyCreated) { $this->operationRuns->dispatchOrFail($operationRun, function () use ($review, $operationRun): void { ComposeEnvironmentReviewJob::dispatch( @@ -251,6 +242,70 @@ private function findExistingMutableReview(ManagedEnvironment $tenant, string $f ->first(); } + private function findReviewForOperationRun( + OperationRun $operationRun, + ManagedEnvironment $tenant, + string $fingerprint, + ): ?EnvironmentReview { + return EnvironmentReview::query() + ->forWorkspace((int) $tenant->workspace_id) + ->forTenant((int) $tenant->getKey()) + ->where('operation_run_id', (int) $operationRun->getKey()) + ->where('fingerprint', $fingerprint) + ->latest('id') + ->first(); + } + + private function createQueuedReview( + ManagedEnvironment $tenant, + EvidenceSnapshot $snapshot, + User $user, + OperationRun $operationRun, + string $fingerprint, + ): EnvironmentReview { + try { + return EnvironmentReview::query()->create([ + 'managed_environment_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'operation_run_id' => (int) $operationRun->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'fingerprint' => $fingerprint, + 'status' => EnvironmentReviewStatus::Draft->value, + 'completeness_state' => (string) $snapshot->completeness_state, + 'summary' => [ + 'evidence_basis' => [ + 'snapshot_id' => (int) $snapshot->getKey(), + 'snapshot_fingerprint' => (string) $snapshot->fingerprint, + 'snapshot_completeness_state' => (string) $snapshot->completeness_state, + 'snapshot_generated_at' => $snapshot->generated_at?->toIso8601String(), + ], + 'publish_blockers' => [], + 'has_ready_export' => false, + 'last_requested_at' => now()->toIso8601String(), + ], + ]); + } catch (QueryException $exception) { + if (! $this->isUniqueViolation($exception)) { + throw $exception; + } + + $existing = $this->findExistingMutableReview($tenant, $fingerprint) + ?? $this->findReviewForOperationRun($operationRun, $tenant, $fingerprint); + + if ($existing instanceof EnvironmentReview) { + return $existing->load(['tenant', 'evidenceSnapshot', 'sections', 'operationRun', 'initiator', 'publisher']); + } + + throw $exception; + } + } + + private function isUniqueViolation(QueryException $exception): bool + { + return in_array(($exception->errorInfo[0] ?? null), ['23505', '23000'], true); + } + /** * @return array{label: string, operation_count: int, failed_count: int, partial_count: int} */ diff --git a/apps/platform/app/Services/OperationRunService.php b/apps/platform/app/Services/OperationRunService.php index 13465b45..c6322cc1 100644 --- a/apps/platform/app/Services/OperationRunService.php +++ b/apps/platform/app/Services/OperationRunService.php @@ -857,6 +857,9 @@ public function updateRunWithReconciliation( string $reasonMessage, string $source = 'scheduled_reconciler', array $evidence = [], + ?string $adapter = null, + ?string $decision = null, + array $related = [], ): OperationRun { /** @var OperationRun $updated */ $updated = DB::transaction(function () use ( @@ -869,6 +872,9 @@ public function updateRunWithReconciliation( $reasonMessage, $source, $evidence, + $adapter, + $decision, + $related, ): OperationRun { $locked = OperationRun::query() ->whereKey($run->getKey()) @@ -890,6 +896,11 @@ public function updateRunWithReconciliation( reasonMessage: $reasonMessage, source: $source, evidence: $evidence, + previousStatus: (string) $locked->status, + previousOutcome: (string) $locked->outcome, + adapter: $adapter, + decision: $decision, + related: $related, ); $translatedContext = $this->withReasonTranslationContext( @@ -1280,15 +1291,32 @@ private function reconciliationMetadata( string $reasonMessage, string $source, array $evidence, + string $previousStatus, + string $previousOutcome, + ?string $adapter = null, + ?string $decision = null, + array $related = [], ): array { - return [ - 'reconciled_at' => now()->toIso8601String(), + $timestamp = now()->toIso8601String(); + + return array_filter([ + 'reconciled_at' => $timestamp, + 'timestamp' => $timestamp, 'reason' => RunFailureSanitizer::normalizeReasonCode($reasonCode), 'reason_code' => RunFailureSanitizer::normalizeReasonCode($reasonCode), 'reason_message' => $this->sanitizeMessage($reasonMessage), 'source' => $this->sanitizeFailureCode($source), + 'adapter' => is_string($adapter) && trim($adapter) !== '' ? trim($adapter) : null, + 'decision' => is_string($decision) && trim($decision) !== '' ? trim($decision) : null, + 'previous_status' => $previousStatus, + 'previous_outcome' => $previousOutcome, + 'previous' => [ + 'status' => $previousStatus, + 'outcome' => $previousOutcome, + ], + 'related' => $related, 'evidence' => $evidence, - ]; + ], static fn (mixed $value): bool => $value !== null && $value !== []); } private function bridgeReasonForThrowable(Throwable $exception): LifecycleReconciliationReason diff --git a/apps/platform/app/Support/OperationRunLinks.php b/apps/platform/app/Support/OperationRunLinks.php index c531e0c3..12818dcc 100644 --- a/apps/platform/app/Support/OperationRunLinks.php +++ b/apps/platform/app/Support/OperationRunLinks.php @@ -230,10 +230,23 @@ public static function related(OperationRun $run, ?ManagedEnvironment $tenant): } if ($canonicalType === 'environment.review.compose') { - $review = EnvironmentReview::query() - ->where('operation_run_id', (int) $run->getKey()) - ->latest('id') - ->first(); + $review = null; + $relatedReviewId = $run->reconciledRelatedReviewId(); + + if ($relatedReviewId !== null) { + $review = EnvironmentReview::query() + ->whereKey($relatedReviewId) + ->where('workspace_id', (int) $run->workspace_id) + ->where('managed_environment_id', (int) $run->managed_environment_id) + ->first(); + } + + if (! $review instanceof EnvironmentReview) { + $review = EnvironmentReview::query() + ->where('operation_run_id', (int) $run->getKey()) + ->latest('id') + ->first(); + } if ($review instanceof EnvironmentReview) { $links['ManagedEnvironment Review'] = EnvironmentReviewResource::environmentScopedUrl('view', ['record' => $review], $tenant); diff --git a/apps/platform/app/Support/Operations/Reconciliation/EnvironmentReviewComposeDecision.php b/apps/platform/app/Support/Operations/Reconciliation/EnvironmentReviewComposeDecision.php new file mode 100644 index 00000000..e8544943 --- /dev/null +++ b/apps/platform/app/Support/Operations/Reconciliation/EnvironmentReviewComposeDecision.php @@ -0,0 +1,334 @@ +type !== 'environment.review.compose') { + return ReconciliationResult::unsupported( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'This adapter only supports environment review composition runs.', + evidence: [ + 'adapter' => self::ADAPTER, + 'type' => (string) $run->type, + ], + ); + } + + $context = is_array($run->context) ? $run->context : []; + $workspaceId = (int) ($run->workspace_id ?? $context['workspace_id'] ?? 0); + $tenantId = (int) ($run->managed_environment_id ?? $context['managed_environment_id'] ?? 0); + $fingerprint = is_string($context['review_fingerprint'] ?? null) + ? trim((string) $context['review_fingerprint']) + : ''; + $explicitReviewId = is_numeric($context['review_id'] ?? null) + ? (int) $context['review_id'] + : null; + + $evidence = [ + 'adapter' => self::ADAPTER, + 'operation_run_id' => (int) $run->getKey(), + 'workspace_id' => $workspaceId, + 'managed_environment_id' => $tenantId, + 'fingerprint' => $fingerprint, + 'explicit_review_id' => $explicitReviewId, + ]; + + if ($workspaceId <= 0 || $tenantId <= 0 || $fingerprint === '') { + return ReconciliationResult::attentionRequired( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'The run no longer carries enough review scope to reconcile it safely.', + evidence: $evidence, + ); + } + + $candidates = EnvironmentReview::query() + ->forWorkspace($workspaceId) + ->forTenant($tenantId) + ->where('fingerprint', $fingerprint) + ->orderByDesc('id') + ->get(); + + $evidence['considered_review_ids'] = $candidates->modelKeys(); + $evidence['considered_reviews'] = $candidates + ->map(fn (EnvironmentReview $review): array => $this->reviewReference($review)) + ->values() + ->all(); + + $explicitReview = $explicitReviewId !== null + ? $candidates->firstWhere('id', $explicitReviewId) + : null; + + if ($explicitReviewId !== null && ! $explicitReview instanceof EnvironmentReview) { + return ReconciliationResult::attentionRequired( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'The originally queued review no longer matches the current review truth safely.', + evidence: $evidence + ['explicit_review_missing' => true], + ); + } + + if ($explicitReview instanceof EnvironmentReview) { + $lineageDecision = $this->evaluateExplicitReviewLineage($explicitReview, $candidates, $evidence); + + if ($lineageDecision instanceof ReconciliationResult) { + return $lineageDecision; + } + } + + $usable = $candidates->filter(fn (EnvironmentReview $review): bool => $this->isUsableReview($review))->values(); + + if ($usable->count() === 1) { + /** @var EnvironmentReview $review */ + $review = $usable->first(); + + return ReconciliationResult::reconciledSucceeded( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: $review->status === EnvironmentReviewStatus::Published->value + ? 'A matching published review was already available for this run.' + : 'A matching ready review was already available for this run.', + evidence: $evidence + ['chosen_review_id' => (int) $review->getKey()], + related: $this->relatedReviewMetadata($review), + summaryCounts: $this->summaryCounts($review), + ); + } + + if ($usable->count() > 1) { + return ReconciliationResult::attentionRequired( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'Multiple matching reviews are available, so this run needs manual review.', + evidence: $evidence + ['ambiguous_review_ids' => $usable->modelKeys()], + ); + } + + if ($candidates->isEmpty()) { + return ReconciliationResult::notReconciled( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'No matching review truth is available yet for this run.', + evidence: $evidence, + ); + } + + if ($candidates->contains(fn (EnvironmentReview $review): bool => $this->isBlockingState($review))) { + $blockingReview = $candidates->first(fn (EnvironmentReview $review): bool => $this->isBlockingState($review)); + + if ($blockingReview instanceof EnvironmentReview && (int) ($blockingReview->operation_run_id ?? 0) === (int) $run->getKey()) { + return ReconciliationResult::notReconciled( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'The queued review is still composing and does not prove final review truth yet.', + evidence: $evidence + [ + 'chosen_review_id' => (int) $blockingReview->getKey(), + ], + related: $this->relatedReviewMetadata($blockingReview), + ); + } + + return ReconciliationResult::blocked( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'A matching review exists, but it is not ready yet.', + evidence: $evidence + [ + 'chosen_review_id' => $blockingReview instanceof EnvironmentReview ? (int) $blockingReview->getKey() : null, + ], + related: $blockingReview instanceof EnvironmentReview ? $this->relatedReviewMetadata($blockingReview) : [], + summaryCounts: $blockingReview instanceof EnvironmentReview ? $this->summaryCounts($blockingReview) : [], + ); + } + + return ReconciliationResult::attentionRequired( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'Review lineage needs operator confirmation before this run can be trusted.', + evidence: $evidence, + ); + } + + /** + * @param Collection $candidates + * @param array $evidence + */ + private function evaluateExplicitReviewLineage( + EnvironmentReview $review, + Collection $candidates, + array $evidence, + ): ?ReconciliationResult { + if ($this->isUsableReview($review)) { + return ReconciliationResult::reconciledSucceeded( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: $review->status === EnvironmentReviewStatus::Published->value + ? 'The queued review was already published before the run finished updating.' + : 'The queued review was already ready before the run finished updating.', + evidence: $evidence + ['chosen_review_id' => (int) $review->getKey()], + related: $this->relatedReviewMetadata($review), + summaryCounts: $this->summaryCounts($review), + ); + } + + $successorId = is_numeric($review->superseded_by_review_id) + ? (int) $review->superseded_by_review_id + : null; + + if ($successorId === null) { + if ($this->isBlockingState($review)) { + if ($candidates->contains(fn (EnvironmentReview $candidate): bool => $this->isUsableReview($candidate))) { + return null; + } + + if ((int) ($review->operation_run_id ?? 0) === (int) ($evidence['operation_run_id'] ?? 0)) { + return ReconciliationResult::notReconciled( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'The queued review is still composing and does not prove final review truth yet.', + evidence: $evidence + ['chosen_review_id' => (int) $review->getKey()], + related: $this->relatedReviewMetadata($review), + ); + } + + return ReconciliationResult::blocked( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'The queued review still exists, but it is not ready yet.', + evidence: $evidence + ['chosen_review_id' => (int) $review->getKey()], + related: $this->relatedReviewMetadata($review), + summaryCounts: $this->summaryCounts($review), + ); + } + + if ($review->status === EnvironmentReviewStatus::Superseded->value) { + return ReconciliationResult::attentionRequired( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'This run points at superseded review lineage that cannot be resolved safely.', + evidence: $evidence + ['lineage_review_id' => (int) $review->getKey()], + related: $this->relatedReviewMetadata($review), + ); + } + + return null; + } + + /** @var EnvironmentReview|null $successor */ + $successor = $candidates->firstWhere('id', $successorId); + + if (! $successor instanceof EnvironmentReview) { + return ReconciliationResult::attentionRequired( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'A successor review was recorded, but the matching review truth could not be resolved safely.', + evidence: $evidence + [ + 'lineage_review_id' => (int) $review->getKey(), + 'lineage_successor_review_id' => $successorId, + ], + related: $this->relatedReviewMetadata($review), + ); + } + + if ($this->isUsableReview($successor)) { + return ReconciliationResult::reconciledSucceeded( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'A ready successor review was already available for this run.', + evidence: $evidence + [ + 'lineage_review_id' => (int) $review->getKey(), + 'chosen_review_id' => (int) $successor->getKey(), + ], + related: $this->relatedReviewMetadata($successor) + [ + 'lineage_review_id' => (int) $review->getKey(), + ], + summaryCounts: $this->summaryCounts($successor), + ); + } + + if ($this->isBlockingState($successor)) { + return ReconciliationResult::blocked( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'A successor review exists, but it is not ready yet.', + evidence: $evidence + [ + 'lineage_review_id' => (int) $review->getKey(), + 'chosen_review_id' => (int) $successor->getKey(), + ], + related: $this->relatedReviewMetadata($successor) + [ + 'lineage_review_id' => (int) $review->getKey(), + ], + summaryCounts: $this->summaryCounts($successor), + ); + } + + return ReconciliationResult::attentionRequired( + reasonCode: LifecycleReconciliationReason::AdapterOutOfSync->value, + reasonMessage: 'The successor review needs operator review before this run can be trusted.', + evidence: $evidence + [ + 'lineage_review_id' => (int) $review->getKey(), + 'chosen_review_id' => (int) $successor->getKey(), + ], + related: $this->relatedReviewMetadata($successor) + [ + 'lineage_review_id' => (int) $review->getKey(), + ], + ); + } + + private function isUsableReview(EnvironmentReview $review): bool + { + return in_array((string) $review->status, [ + EnvironmentReviewStatus::Ready->value, + EnvironmentReviewStatus::Published->value, + ], true); + } + + private function isBlockingState(EnvironmentReview $review): bool + { + return in_array((string) $review->status, [ + EnvironmentReviewStatus::Draft->value, + EnvironmentReviewStatus::Failed->value, + ], true); + } + + /** + * @return array + */ + private function relatedReviewMetadata(EnvironmentReview $review): array + { + return array_filter([ + 'review' => [ + 'id' => (int) $review->getKey(), + 'status' => (string) $review->status, + 'fingerprint' => (string) $review->fingerprint, + 'operation_run_id' => is_numeric($review->operation_run_id) ? (int) $review->operation_run_id : null, + 'superseded_by_review_id' => is_numeric($review->superseded_by_review_id) ? (int) $review->superseded_by_review_id : null, + ], + ], static fn (mixed $value): bool => $value !== null && $value !== []); + } + + /** + * @return array + */ + private function reviewReference(EnvironmentReview $review): array + { + return [ + 'id' => (int) $review->getKey(), + 'status' => (string) $review->status, + 'operation_run_id' => is_numeric($review->operation_run_id) ? (int) $review->operation_run_id : null, + 'superseded_by_review_id' => is_numeric($review->superseded_by_review_id) ? (int) $review->superseded_by_review_id : null, + ]; + } + + /** + * @return array + */ + private function summaryCounts(EnvironmentReview $review): array + { + $summary = is_array($review->summary) ? $review->summary : []; + $counts = [ + 'finding_count' => is_numeric($summary['finding_count'] ?? null) ? (int) $summary['finding_count'] : null, + 'report_count' => is_numeric($summary['report_count'] ?? null) ? (int) $summary['report_count'] : null, + 'operation_count' => is_numeric($summary['operation_count'] ?? null) ? (int) $summary['operation_count'] : null, + 'errors_recorded' => 0, + ]; + + return array_filter($counts, static fn (mixed $value): bool => is_int($value)); + } +} diff --git a/apps/platform/app/Support/Operations/Reconciliation/ReconciliationResult.php b/apps/platform/app/Support/Operations/Reconciliation/ReconciliationResult.php new file mode 100644 index 00000000..b035c1a0 --- /dev/null +++ b/apps/platform/app/Support/Operations/Reconciliation/ReconciliationResult.php @@ -0,0 +1,218 @@ + $evidence + * @param array $related + * @param array $summaryCounts + * @param array $failures + */ + private function __construct( + public string $decision, + public ?string $status, + public ?string $outcome, + public string $reasonCode, + public string $reasonMessage, + public array $evidence, + public array $related, + public array $summaryCounts, + public array $failures, + public bool $safeForAutoCompletion, + ) {} + + /** + * @param array $evidence + * @param array $related + * @param array $summaryCounts + */ + public static function reconciledSucceeded( + string $reasonCode, + string $reasonMessage, + array $evidence = [], + array $related = [], + array $summaryCounts = [], + ): self { + return new self( + decision: 'reconciled_succeeded', + status: OperationRunStatus::Completed->value, + outcome: OperationRunOutcome::Succeeded->value, + reasonCode: $reasonCode, + reasonMessage: $reasonMessage, + evidence: $evidence, + related: $related, + summaryCounts: $summaryCounts, + failures: [], + safeForAutoCompletion: true, + ); + } + + /** + * @param array $evidence + * @param array $related + * @param array $summaryCounts + */ + public static function blocked( + string $reasonCode, + string $reasonMessage, + array $evidence = [], + array $related = [], + array $summaryCounts = [], + ): self { + return new self( + decision: 'blocked', + status: OperationRunStatus::Completed->value, + outcome: OperationRunOutcome::Blocked->value, + reasonCode: $reasonCode, + reasonMessage: $reasonMessage, + evidence: $evidence, + related: $related, + summaryCounts: $summaryCounts, + failures: [[ + 'code' => $reasonCode, + 'reason_code' => $reasonCode, + 'message' => $reasonMessage, + ]], + safeForAutoCompletion: true, + ); + } + + /** + * @param array $evidence + * @param array $related + * @param array $summaryCounts + */ + public static function attentionRequired( + string $reasonCode, + string $reasonMessage, + array $evidence = [], + array $related = [], + array $summaryCounts = [], + ): self { + return new self( + decision: 'attention_required', + status: OperationRunStatus::Completed->value, + outcome: OperationRunOutcome::Failed->value, + reasonCode: $reasonCode, + reasonMessage: $reasonMessage, + evidence: $evidence, + related: $related, + summaryCounts: $summaryCounts, + failures: [[ + 'code' => $reasonCode, + 'reason_code' => $reasonCode, + 'message' => $reasonMessage, + ]], + safeForAutoCompletion: true, + ); + } + + /** + * @param array $evidence + * @param array $related + * @param array $summaryCounts + */ + public static function failedUnrecoverable( + string $reasonCode, + string $reasonMessage, + array $evidence = [], + array $related = [], + array $summaryCounts = [], + ): self { + return new self( + decision: 'failed_unrecoverable', + status: OperationRunStatus::Completed->value, + outcome: OperationRunOutcome::Failed->value, + reasonCode: $reasonCode, + reasonMessage: $reasonMessage, + evidence: $evidence, + related: $related, + summaryCounts: $summaryCounts, + failures: [[ + 'code' => $reasonCode, + 'reason_code' => $reasonCode, + 'message' => $reasonMessage, + ]], + safeForAutoCompletion: true, + ); + } + + /** + * @param array $evidence + * @param array $related + */ + public static function notReconciled( + string $reasonCode, + string $reasonMessage, + array $evidence = [], + array $related = [], + ): self { + return new self( + decision: 'not_reconciled', + status: null, + outcome: null, + reasonCode: $reasonCode, + reasonMessage: $reasonMessage, + evidence: $evidence, + related: $related, + summaryCounts: [], + failures: [], + safeForAutoCompletion: false, + ); + } + + /** + * @param array $evidence + * @param array $related + */ + public static function unsupported( + string $reasonCode, + string $reasonMessage, + array $evidence = [], + array $related = [], + ): self { + return new self( + decision: 'unsupported', + status: null, + outcome: null, + reasonCode: $reasonCode, + reasonMessage: $reasonMessage, + evidence: $evidence, + related: $related, + summaryCounts: [], + failures: [], + safeForAutoCompletion: false, + ); + } + + public function shouldFinalizeRun(): bool + { + return $this->status !== null && $this->outcome !== null; + } + + /** + * @return array + */ + public function toArray(): array + { + return array_filter([ + 'decision' => $this->decision, + 'status' => $this->status, + 'outcome' => $this->outcome, + 'reason_code' => $this->reasonCode, + 'reason_message' => $this->reasonMessage, + 'safe_for_auto_completion' => $this->safeForAutoCompletion, + 'summary_counts' => $this->summaryCounts, + 'failures' => $this->failures, + 'evidence' => $this->evidence, + 'related' => $this->related, + ], static fn (mixed $value): bool => $value !== null && $value !== []); + } +} diff --git a/apps/platform/app/Support/OpsUx/OperationUxPresenter.php b/apps/platform/app/Support/OpsUx/OperationUxPresenter.php index 31e17978..f5004edd 100644 --- a/apps/platform/app/Support/OpsUx/OperationUxPresenter.php +++ b/apps/platform/app/Support/OpsUx/OperationUxPresenter.php @@ -199,12 +199,17 @@ private static function buildSurfaceGuidance(OperationRun $run): ?string $operatorExplanationGuidance = self::operatorExplanationGuidance($run); $nextStepLabel = self::firstNextStepLabel($run); $freshnessState = self::freshnessState($run); + $reconciliationCopy = self::reviewComposeReconciliationCopy($run); if ($freshnessState->isLikelyStale()) { return RunDurationInsights::stuckGuidance($run) ?? 'Past the lifecycle window. Review worker health and logs before retrying.'; } + if (is_string($reconciliationCopy['guidance'] ?? null) && trim((string) $reconciliationCopy['guidance']) !== '') { + return trim((string) $reconciliationCopy['guidance']); + } + if ($freshnessState->isReconciledFailed()) { return $operatorExplanationGuidance ?? $reasonGuidance @@ -264,6 +269,12 @@ public static function surfaceFailureDetailFresh(OperationRun $run): ?string private static function buildSurfaceFailureDetail(OperationRun $run): ?string { + $reconciliationCopy = self::reviewComposeReconciliationCopy($run); + + if (is_string($reconciliationCopy['detail'] ?? null) && trim((string) $reconciliationCopy['detail']) !== '') { + return trim((string) $reconciliationCopy['detail']); + } + $operatorExplanation = self::governanceOperatorExplanation($run); if (is_string($operatorExplanation?->dominantCauseExplanation) && trim($operatorExplanation->dominantCauseExplanation) !== '') { @@ -470,6 +481,14 @@ private static function terminalPresentation(OperationRun $run): array $uxStatus = OperationStatusNormalizer::toUxStatus($run->status, $run->outcome); $reasonEnvelope = self::reasonEnvelope($run); $freshnessState = self::freshnessState($run); + $reconciliationCopy = self::reviewComposeReconciliationCopy($run); + + if (is_array($reconciliationCopy['terminal'] ?? null)) { + /** @var array{titleSuffix:string,body:string,status:string} $terminal */ + $terminal = $reconciliationCopy['terminal']; + + return $terminal; + } if ($freshnessState->isReconciledFailed()) { return [ @@ -788,6 +807,72 @@ private static function operatorExplanationGuidance(OperationRun $run): ?string : 'Next step: '.$text.'.'; } + /** + * @return array{ + * guidance?: string, + * detail?: string, + * headline?: string, + * terminal?: array{titleSuffix:string,body:string,status:string} + * }|null + */ + public static function reviewComposeReconciliationCopy(OperationRun $run): ?array + { + if ((string) $run->type !== 'environment.review.compose' || ! $run->isLifecycleReconciled()) { + return null; + } + + if ($run->reconciliationAdapter() !== 'environment_review_compose') { + return null; + } + + $reviewId = $run->reconciledRelatedReviewId(); + $reviewLabel = $reviewId !== null ? sprintf('review #%d', $reviewId) : 'the matching review'; + + return match ($run->reconciliationDecision()) { + 'reconciled_succeeded' => [ + 'headline' => 'A matching review was already available.', + 'detail' => sprintf('TenantPilot confirmed %s and closed this run from existing review truth.', $reviewLabel), + 'guidance' => 'No action needed. A matching review was already available.', + 'terminal' => [ + 'titleSuffix' => 'was reconciled from matching review', + 'body' => 'A matching review was already available.', + 'status' => 'success', + ], + ], + 'blocked' => [ + 'headline' => 'Matching review still needs follow-up.', + 'detail' => sprintf('TenantPilot found %s, but it is not ready yet.', $reviewLabel), + 'guidance' => 'Review the matching review before retrying.', + 'terminal' => [ + 'titleSuffix' => 'needs review follow-up', + 'body' => 'A matching review exists, but it is not ready yet.', + 'status' => 'warning', + ], + ], + 'attention_required' => [ + 'headline' => 'Review truth needs operator confirmation.', + 'detail' => 'TenantPilot found matching review activity, but it could not resolve the final review truth safely.', + 'guidance' => 'Inspect the recorded review lineage before retrying.', + 'terminal' => [ + 'titleSuffix' => 'needs operator review', + 'body' => 'Matching review activity needs operator review before retrying.', + 'status' => 'warning', + ], + ], + 'failed_unrecoverable' => [ + 'headline' => 'Review reconciliation could not recover this run.', + 'detail' => 'TenantPilot could not recover this run from the available review truth.', + 'guidance' => 'Review the recorded evidence before retrying.', + 'terminal' => [ + 'titleSuffix' => 'could not be recovered automatically', + 'body' => 'The available review truth could not recover this run automatically.', + 'status' => 'danger', + ], + ], + default => null, + }; + } + private static function resolveGovernanceOperatorExplanation(OperationRun $run, bool $fresh = false): ?OperatorExplanationPattern { if (! $run->supportsOperatorExplanation()) { diff --git a/apps/platform/app/Support/Ui/GovernanceArtifactTruth/ArtifactTruthPresenter.php b/apps/platform/app/Support/Ui/GovernanceArtifactTruth/ArtifactTruthPresenter.php index cb51b9ae..ded7e7b2 100644 --- a/apps/platform/app/Support/Ui/GovernanceArtifactTruth/ArtifactTruthPresenter.php +++ b/apps/platform/app/Support/Ui/GovernanceArtifactTruth/ArtifactTruthPresenter.php @@ -1132,12 +1132,36 @@ private function resolveArtifactForRun(OperationRun $run): BaselineSnapshot|Evid ? BaselineSnapshot::query()->with('baselineProfile')->find($run->relatedArtifactId()) : null, 'evidence_snapshot' => EvidenceSnapshot::query()->with('tenant')->where('operation_run_id', (int) $run->getKey())->latest('id')->first(), - 'environment_review' => EnvironmentReview::query()->with(['tenant', 'currentExportReviewPack'])->where('operation_run_id', (int) $run->getKey())->latest('id')->first(), + 'environment_review' => $this->resolveEnvironmentReviewForRun($run), 'review_pack' => ReviewPack::query()->with(['tenant', 'environmentReview'])->where('operation_run_id', (int) $run->getKey())->latest('id')->first(), default => null, }; } + private function resolveEnvironmentReviewForRun(OperationRun $run): ?EnvironmentReview + { + $relatedReviewId = $run->reconciledRelatedReviewId(); + + if ($relatedReviewId !== null) { + $review = EnvironmentReview::query() + ->with(['tenant', 'currentExportReviewPack']) + ->whereKey($relatedReviewId) + ->where('workspace_id', (int) $run->workspace_id) + ->where('managed_environment_id', (int) $run->managed_environment_id) + ->first(); + + if ($review instanceof EnvironmentReview) { + return $review; + } + } + + return EnvironmentReview::query() + ->with(['tenant', 'currentExportReviewPack']) + ->where('operation_run_id', (int) $run->getKey()) + ->latest('id') + ->first(); + } + private function resolveEnvelope( Model $record, string $variant, diff --git a/apps/platform/tests/Browser/Spec359ReviewComposeReconciliationSmokeTest.php b/apps/platform/tests/Browser/Spec359ReviewComposeReconciliationSmokeTest.php new file mode 100644 index 00000000..17a07ba7 --- /dev/null +++ b/apps/platform/tests/Browser/Spec359ReviewComposeReconciliationSmokeTest.php @@ -0,0 +1,234 @@ +browser()->timeout(60_000); + +it('Spec359 smokes attention-required review-compose guidance on the operations hub', function (): void { + [$user, $environment] = createUserWithTenant(role: 'owner', workspaceRole: 'manager'); + + spec359AuthenticateBrowser($this, $user, $environment); + + $run = spec359BrowserCreateReconciledReviewComposeRun( + environment: $environment, + user: $user, + decision: 'attention_required', + outcome: OperationRunOutcome::Failed->value, + reasonCode: 'review.compose.ambiguous_truth', + reasonMessage: 'Multiple matching reviews are available, so this run needs manual review.', + failures: [ + [ + 'code' => 'review.compose.ambiguous_truth', + 'message' => 'Multiple matching reviews are available, so this run needs manual review.', + ], + ], + ); + + visit(OperationRunLinks::index($environment)) + ->resize(1440, 1100) + ->waitForText('Operations Hub') + ->assertSee('Inspect the recorded review lineage before retrying.') + ->assertSee('Failed') + ->assertSee('Completed') + ->assertSee($environment->name) + ->assertDontSee('SQLSTATE') + ->assertDontSee('duplicate key') + ->assertDontSee('environment_reviews_fingerprint_mutable_unique') + ->assertDontSee('worker crash') + ->assertNoJavaScriptErrors() + ->assertNoConsoleLogs(); + + visit(OperationRunLinks::tenantlessView($run)) + ->waitForText('Monitoring detail') + ->assertSee('Automatically reconciled') + ->assertSee('TenantPilot found matching review activity, but it could not resolve the final review truth safely.') + ->assertSee('Inspect the recorded review lineage before retrying.') + ->assertDontSee('SQLSTATE') + ->assertDontSee('duplicate key') + ->assertNoJavaScriptErrors() + ->assertNoConsoleLogs(); +}); + +it('Spec359 smokes matching-review reconciliation wording on the tenantless run detail', function (): void { + [$user, $environment] = createUserWithTenant(role: 'owner', workspaceRole: 'manager'); + + spec359AuthenticateBrowser($this, $user, $environment); + + $review = spec359BrowserCreatePublishedMatchingReview($environment, $user, 'spec359-browser-reused'); + + $run = spec359BrowserCreateReconciledReviewComposeRun( + environment: $environment, + user: $user, + decision: 'reconciled_succeeded', + outcome: OperationRunOutcome::Succeeded->value, + reasonCode: 'review.compose.matching_review_available', + reasonMessage: 'A matching review was already available for this run.', + relatedReview: $review, + evidence: [ + 'fingerprint' => 'spec359-browser-reused', + ], + ); + + visit(OperationRunLinks::tenantlessView($run)) + ->resize(1440, 1100) + ->waitForText('Monitoring detail') + ->assertSee(OperationRunLinks::identifier($run)) + ->assertSee('A matching review was already available.') + ->assertSee('No action needed. A matching review was already available.') + ->assertSee('Automatically reconciled') + ->assertSee(sprintf('TenantPilot confirmed review #%d and closed this run from existing review truth.', (int) $review->getKey())) + ->assertDontSee('SQLSTATE') + ->assertDontSee('duplicate key') + ->assertDontSee('environment_reviews_fingerprint_mutable_unique') + ->assertNoJavaScriptErrors() + ->assertNoConsoleLogs(); +}); + +it('Spec359 smokes duplicate-recovered review-compose wording without leaking raw constraint text', function (): void { + [$user, $environment] = createUserWithTenant(role: 'owner', workspaceRole: 'manager'); + + spec359AuthenticateBrowser($this, $user, $environment); + + $review = spec359BrowserCreatePublishedMatchingReview($environment, $user, 'spec359-browser-duplicate'); + + $run = spec359BrowserCreateReconciledReviewComposeRun( + environment: $environment, + user: $user, + decision: 'reconciled_succeeded', + outcome: OperationRunOutcome::Succeeded->value, + reasonCode: 'review.compose.duplicate_recovered', + reasonMessage: 'A duplicate compose attempt was recovered from matching review truth.', + relatedReview: $review, + evidence: [ + 'fingerprint' => 'spec359-browser-duplicate', + 'duplicate_recovered' => true, + ], + ); + + visit(OperationRunLinks::tenantlessView($run)) + ->resize(1440, 1100) + ->waitForText('Monitoring detail') + ->assertSee('A matching review was already available.') + ->assertSee('No action needed. A matching review was already available.') + ->assertSee(sprintf('TenantPilot confirmed review #%d and closed this run from existing review truth.', (int) $review->getKey())) + ->assertDontSee('duplicate key') + ->assertDontSee('environment_reviews_fingerprint_mutable_unique') + ->assertDontSee('SQLSTATE') + ->assertDontSee('23505') + ->assertNoJavaScriptErrors() + ->assertNoConsoleLogs(); +}); + +function spec359AuthenticateBrowser(mixed $test, User $user, ManagedEnvironment $environment): void +{ + $workspaceId = (int) $environment->workspace_id; + + $test->actingAs($user)->withSession([ + WorkspaceContext::SESSION_KEY => $workspaceId, + WorkspaceContext::LAST_ENVIRONMENT_IDS_SESSION_KEY => [ + (string) $workspaceId => (int) $environment->getKey(), + ], + ]); + + session()->put(WorkspaceContext::SESSION_KEY, $workspaceId); + session()->put(WorkspaceContext::LAST_ENVIRONMENT_IDS_SESSION_KEY, [ + (string) $workspaceId => (int) $environment->getKey(), + ]); + + setAdminPanelContext($environment); +} + +function spec359BrowserCreatePublishedMatchingReview( + ManagedEnvironment $environment, + User $user, + string $fingerprint, +): EnvironmentReview { + $snapshot = seedEnvironmentReviewEvidence($environment, operationRunCount: 1); + + $publishedRun = OperationRun::factory()->forTenant($environment)->create([ + 'user_id' => (int) $user->getKey(), + 'initiator_name' => $user->name, + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Completed->value, + 'outcome' => OperationRunOutcome::Succeeded->value, + 'completed_at' => now()->subMinutes(5), + 'context' => [ + 'workspace_id' => (int) $environment->workspace_id, + 'managed_environment_id' => (int) $environment->getKey(), + 'review_fingerprint' => $fingerprint, + ], + ]); + + return EnvironmentReview::factory()->published()->create([ + 'workspace_id' => (int) $environment->workspace_id, + 'managed_environment_id' => (int) $environment->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'operation_run_id' => (int) $publishedRun->getKey(), + 'fingerprint' => $fingerprint, + 'status' => EnvironmentReviewStatus::Published->value, + 'published_by_user_id' => (int) $user->getKey(), + ]); +} + +function spec359BrowserCreateReconciledReviewComposeRun( + ManagedEnvironment $environment, + User $user, + string $decision, + string $outcome, + string $reasonCode, + string $reasonMessage, + ?EnvironmentReview $relatedReview = null, + array $failures = [], + array $evidence = [], +): OperationRun { + $fingerprint = (string) ($evidence['fingerprint'] ?? 'spec359-browser-run'); + + $run = OperationRun::factory()->forTenant($environment)->create([ + 'user_id' => (int) $user->getKey(), + 'initiator_name' => $user->name, + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Queued->value, + 'outcome' => OperationRunOutcome::Pending->value, + 'context' => [ + 'workspace_id' => (int) $environment->workspace_id, + 'managed_environment_id' => (int) $environment->getKey(), + 'review_fingerprint' => $fingerprint, + ], + ]); + + return app(OperationRunService::class)->updateRunWithReconciliation( + run: $run, + status: OperationRunStatus::Completed->value, + outcome: $outcome, + summaryCounts: ['finding_count' => 2], + failures: $failures, + reasonCode: $reasonCode, + reasonMessage: $reasonMessage, + source: 'adapter_reconciler', + evidence: $evidence, + adapter: 'environment_review_compose', + decision: $decision, + related: $relatedReview instanceof EnvironmentReview + ? [ + 'review' => [ + 'id' => (int) $relatedReview->getKey(), + ], + ] + : [], + )->fresh(); +} diff --git a/apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeIdempotencyTest.php b/apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeIdempotencyTest.php new file mode 100644 index 00000000..d54e1d81 --- /dev/null +++ b/apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeIdempotencyTest.php @@ -0,0 +1,73 @@ +create($tenant, $snapshot, $user); + $second = $service->create($tenant, $snapshot, $user); + + expect((int) $second->getKey())->toBe((int) $first->getKey()) + ->and((int) $second->operation_run_id)->toBe((int) $first->operation_run_id); + + Queue::assertPushed(ComposeEnvironmentReviewJob::class, 1); +}); + +it('reuses the existing mutable review for repeated Spec359 refresh triggers', function (): void { + Queue::fake(); + + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $snapshot = seedEnvironmentReviewEvidence($tenant); + + /** @var EnvironmentReviewService $service */ + $service = app(EnvironmentReviewService::class); + + $review = $service->create($tenant, $snapshot, $user)->refresh(); + $reused = $service->refresh($review, $user, $snapshot); + + expect((int) $reused->getKey())->toBe((int) $review->getKey()) + ->and((int) $reused->operation_run_id)->toBe((int) $review->operation_run_id); + + Queue::assertPushed(ComposeEnvironmentReviewJob::class, 1); +}); + +it('enforces the Spec359 mutable fingerprint uniqueness contract on PostgreSQL', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $snapshot = seedEnvironmentReviewEvidence($tenant); + $fingerprint = app(EnvironmentReviewFingerprint::class)->forSnapshot($tenant, $snapshot); + + EnvironmentReview::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'fingerprint' => $fingerprint, + 'status' => EnvironmentReviewStatus::Draft->value, + ]); + + expect(fn () => EnvironmentReview::factory()->ready()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'fingerprint' => $fingerprint, + ]))->toThrow(QueryException::class); +})->group('pgsql'); diff --git a/apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeReconciliationTest.php b/apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeReconciliationTest.php new file mode 100644 index 00000000..77f540db --- /dev/null +++ b/apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeReconciliationTest.php @@ -0,0 +1,220 @@ +forSnapshot($tenant, $snapshot); + + $run = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'initiator_name' => $user->name, + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Queued->value, + 'outcome' => OperationRunOutcome::Pending->value, + 'context' => [ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'review_fingerprint' => $fingerprint, + ], + ]); + + $review = EnvironmentReview::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'operation_run_id' => (int) $run->getKey(), + 'fingerprint' => $fingerprint, + 'status' => EnvironmentReviewStatus::Draft->value, + ]); + + $run->update([ + 'context' => array_replace(is_array($run->context) ? $run->context : [], [ + 'review_id' => (int) $review->getKey(), + ]), + ]); + + $publishedRun = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Completed->value, + 'outcome' => OperationRunOutcome::Succeeded->value, + ]); + + EnvironmentReview::factory()->published()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'operation_run_id' => (int) $publishedRun->getKey(), + 'fingerprint' => $fingerprint, + ]); + + app()->call([new ComposeEnvironmentReviewJob((int) $review->getKey(), (int) $run->getKey()), 'handle']); + + $run->refresh(); + $review->refresh(); + + expect($run->status)->toBe(OperationRunStatus::Completed->value) + ->and($run->outcome)->toBe(OperationRunOutcome::Succeeded->value) + ->and($run->reconciliationDecision())->toBe('reconciled_succeeded') + ->and($review->status)->toBe(EnvironmentReviewStatus::Draft->value); +}); + +it('blocks a late Spec359 job when a successor review exists but is not ready', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $snapshot = seedEnvironmentReviewEvidence($tenant); + $fingerprint = app(EnvironmentReviewFingerprint::class)->forSnapshot($tenant, $snapshot); + + $run = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'initiator_name' => $user->name, + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Queued->value, + 'outcome' => OperationRunOutcome::Pending->value, + 'context' => [ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'review_fingerprint' => $fingerprint, + ], + ]); + + $successor = EnvironmentReview::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'fingerprint' => $fingerprint, + 'status' => EnvironmentReviewStatus::Draft->value, + ]); + + $review = EnvironmentReview::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'operation_run_id' => (int) $run->getKey(), + 'fingerprint' => $fingerprint, + 'status' => EnvironmentReviewStatus::Superseded->value, + 'superseded_by_review_id' => (int) $successor->getKey(), + ]); + + $run->update([ + 'context' => array_replace(is_array($run->context) ? $run->context : [], [ + 'review_id' => (int) $review->getKey(), + ]), + ]); + + app()->call([new ComposeEnvironmentReviewJob((int) $review->getKey(), (int) $run->getKey()), 'handle']); + + $run->refresh(); + $review->refresh(); + + expect($run->status)->toBe(OperationRunStatus::Completed->value) + ->and($run->outcome)->toBe(OperationRunOutcome::Blocked->value) + ->and($run->reconciliationDecision())->toBe('blocked') + ->and($review->status)->toBe(EnvironmentReviewStatus::Superseded->value); +}); + +it('records calm Spec359 attention copy for ambiguous matching review truth', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $snapshot = seedEnvironmentReviewEvidence($tenant); + $fingerprint = app(EnvironmentReviewFingerprint::class)->forSnapshot($tenant, $snapshot); + + $run = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'initiator_name' => $user->name, + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Queued->value, + 'outcome' => OperationRunOutcome::Pending->value, + 'context' => [ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'review_fingerprint' => $fingerprint, + ], + ]); + + $review = EnvironmentReview::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'operation_run_id' => (int) $run->getKey(), + 'fingerprint' => $fingerprint, + 'status' => EnvironmentReviewStatus::Draft->value, + ]); + + $run->update([ + 'context' => array_replace(is_array($run->context) ? $run->context : [], [ + 'review_id' => (int) $review->getKey(), + ]), + ]); + + $firstPublishedRun = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Completed->value, + 'outcome' => OperationRunOutcome::Succeeded->value, + ]); + $secondPublishedRun = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Completed->value, + 'outcome' => OperationRunOutcome::Succeeded->value, + ]); + + EnvironmentReview::factory()->published()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'operation_run_id' => (int) $firstPublishedRun->getKey(), + 'fingerprint' => $fingerprint, + ]); + + EnvironmentReview::factory()->published()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'operation_run_id' => (int) $secondPublishedRun->getKey(), + 'fingerprint' => $fingerprint, + ]); + + app(AdapterRunReconciler::class)->reconcileOperationRun($run, false); + + $run->refresh(); + + expect($run->status)->toBe(OperationRunStatus::Completed->value) + ->and($run->outcome)->toBe(OperationRunOutcome::Failed->value) + ->and($run->reconciliationDecision())->toBe('attention_required') + ->and(data_get($run->failure_summary, '0.message'))->toBe('Multiple matching reviews are available, so this run needs manual review.') + ->and((string) data_get($run->failure_summary, '0.message'))->not->toContain('SQLSTATE') + ->and((string) data_get($run->failure_summary, '0.message'))->not->toContain('duplicate key'); +}); diff --git a/apps/platform/tests/Feature/Filament/RecentOperationsSummaryWidgetTest.php b/apps/platform/tests/Feature/Filament/RecentOperationsSummaryWidgetTest.php index 079a6457..d0e22b1c 100644 --- a/apps/platform/tests/Feature/Filament/RecentOperationsSummaryWidgetTest.php +++ b/apps/platform/tests/Feature/Filament/RecentOperationsSummaryWidgetTest.php @@ -84,6 +84,46 @@ ->test(RecentOperations::class) ->assertSee('Likely stale') ->assertSee('Automatically reconciled') - ->assertSee('Review worker health and logs before retrying from the start surface.') ->assertSee('Review worker health and logs before retrying this operation.'); }); + +it('renders Spec359 matching-review reconciliation copy on shared recent-operations consumers', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + + $this->actingAs($user); + + OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Completed->value, + 'outcome' => OperationRunOutcome::Succeeded->value, + 'context' => [ + 'reconciliation' => [ + 'reconciled_at' => now()->toIso8601String(), + 'reason' => 'run.adapter_out_of_sync', + 'reason_code' => 'run.adapter_out_of_sync', + 'source' => 'adapter_reconciler', + 'adapter' => 'environment_review_compose', + 'decision' => 'reconciled_succeeded', + 'related' => [ + 'review' => [ + 'id' => 321, + ], + ], + ], + ], + ]); + + Livewire::actingAs($user) + ->test(RecentOperationsSummary::class, ['record' => $tenant]) + ->assertSee('A matching review was already available.') + ->assertSee('No action needed. A matching review was already available.'); + + setAdminPanelContext($tenant); + + Livewire::actingAs($user) + ->test(RecentOperations::class) + ->assertSee('A matching review was already available.') + ->assertSee('No action needed. A matching review was already available.'); +}); diff --git a/apps/platform/tests/Feature/Monitoring/OperationLifecycleFreshnessPresentationTest.php b/apps/platform/tests/Feature/Monitoring/OperationLifecycleFreshnessPresentationTest.php index 17e799d0..f6b57dd2 100644 --- a/apps/platform/tests/Feature/Monitoring/OperationLifecycleFreshnessPresentationTest.php +++ b/apps/platform/tests/Feature/Monitoring/OperationLifecycleFreshnessPresentationTest.php @@ -118,3 +118,45 @@ ->assertOk() ->assertSee('Reconciled failed'); }); + +it('shows Spec359 matching-review reconciliation copy on operations hub and tenantless detail surfaces', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + + $run = OperationRun::factory()->create([ + 'managed_environment_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'type' => 'environment.review.compose', + 'status' => 'completed', + 'outcome' => 'succeeded', + 'context' => [ + 'reason_code' => 'run.adapter_out_of_sync', + 'reconciliation' => [ + 'reconciled_at' => now()->toIso8601String(), + 'reason' => 'run.adapter_out_of_sync', + 'reason_code' => 'run.adapter_out_of_sync', + 'source' => 'adapter_reconciler', + 'adapter' => 'environment_review_compose', + 'decision' => 'reconciled_succeeded', + 'related' => [ + 'review' => [ + 'id' => 44, + ], + ], + ], + ], + ]); + + $this->actingAs($user) + ->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) + ->get(\App\Support\OperationRunLinks::index()) + ->assertOk() + ->assertSee('Automatically reconciled') + ->assertSee('A matching review was already available.'); + + $this->actingAs($user) + ->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) + ->get(\App\Support\OperationRunLinks::tenantlessView($run)) + ->assertOk() + ->assertSee('A matching review was already available.') + ->assertSee('No action needed. A matching review was already available.'); +}); diff --git a/apps/platform/tests/Feature/Notifications/OperationRunNotificationTest.php b/apps/platform/tests/Feature/Notifications/OperationRunNotificationTest.php index 96d9fd8b..d95825fd 100644 --- a/apps/platform/tests/Feature/Notifications/OperationRunNotificationTest.php +++ b/apps/platform/tests/Feature/Notifications/OperationRunNotificationTest.php @@ -194,6 +194,53 @@ function spec230ExpectedNotificationIcon(string $status): string $this->get(data_get($notification->data, 'actions.0.url'))->assertSuccessful(); }); +it('emits calm Spec359 terminal notification copy for matching-review reconciliation', function () { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $this->actingAs($user); + + $run = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'initiator_name' => $user->name, + 'type' => 'environment.review.compose', + 'status' => 'queued', + 'outcome' => 'pending', + 'context' => [ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'review_fingerprint' => 'spec359-notification', + ], + ]); + + app(OperationRunService::class)->updateRunWithReconciliation( + run: $run, + status: 'completed', + outcome: 'succeeded', + summaryCounts: ['finding_count' => 2], + failures: [], + reasonCode: 'run.adapter_out_of_sync', + reasonMessage: 'A matching review was already available for this run.', + source: 'adapter_reconciler', + evidence: ['fingerprint' => 'spec359-notification'], + adapter: 'environment_review_compose', + decision: 'reconciled_succeeded', + related: [ + 'review' => [ + 'id' => 91, + ], + ], + ); + + $notification = $user->notifications()->latest('id')->first(); + + expect($notification)->not->toBeNull() + ->and((string) data_get($notification?->data, 'title'))->toContain('was reconciled from matching review') + ->and((string) data_get($notification?->data, 'body'))->toContain('A matching review was already available.') + ->and((string) data_get($notification?->data, 'body'))->not->toContain('infrastructure failure') + ->and(array_values(data_get($notification?->data, 'supporting_lines', [])))->toContain('No action needed. A matching review was already available.'); +}); + it('uses a tenantless view link for completed tenantless runs', function () { [$user, $tenant] = createUserWithTenant(role: 'owner'); $this->actingAs($user); diff --git a/apps/platform/tests/Feature/Operations/Spec359OperationRunAdapterReconciliationTest.php b/apps/platform/tests/Feature/Operations/Spec359OperationRunAdapterReconciliationTest.php new file mode 100644 index 00000000..d5ecf2f6 --- /dev/null +++ b/apps/platform/tests/Feature/Operations/Spec359OperationRunAdapterReconciliationTest.php @@ -0,0 +1,125 @@ +forSnapshot($tenant, $snapshot); + + $run = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'initiator_name' => $user->name, + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Queued->value, + 'outcome' => OperationRunOutcome::Pending->value, + 'created_at' => now()->subMinutes(30), + 'context' => [ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'review_fingerprint' => $fingerprint, + ], + ]); + + $publishedRun = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Completed->value, + 'outcome' => OperationRunOutcome::Succeeded->value, + ]); + + $review = EnvironmentReview::factory()->ready()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'operation_run_id' => (int) $publishedRun->getKey(), + 'fingerprint' => $fingerprint, + 'summary' => [ + 'finding_count' => 4, + 'report_count' => 2, + 'operation_count' => 3, + ], + ]); + + $change = app(AdapterRunReconciler::class)->reconcileOperationRun($run, false); + + expect($change['applied'] ?? null)->toBeTrue(); + + $run->refresh(); + + expect($run->status)->toBe(OperationRunStatus::Completed->value) + ->and($run->outcome)->toBe(OperationRunOutcome::Succeeded->value) + ->and($run->reconciliationDecision())->toBe('reconciled_succeeded') + ->and($run->reconciliationAdapter())->toBe('environment_review_compose') + ->and($run->reconciledRelatedReviewId())->toBe((int) $review->getKey()); + + $truth = app(ArtifactTruthPresenter::class)->forOperationRun($run->fresh()); + + expect($truth->relatedArtifactUrl)->toBe( + EnvironmentReviewResource::environmentScopedUrl('view', ['record' => $review], $tenant), + ); +}); + +it('shows calm Spec359 matching-review copy on the tenantless run detail surface', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $snapshot = seedEnvironmentReviewEvidence($tenant); + $fingerprint = app(EnvironmentReviewFingerprint::class)->forSnapshot($tenant, $snapshot); + + $run = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'initiator_name' => $user->name, + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Completed->value, + 'outcome' => OperationRunOutcome::Succeeded->value, + 'context' => [ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'review_fingerprint' => $fingerprint, + 'reconciliation' => [ + 'reconciled_at' => now()->toIso8601String(), + 'reason' => 'run.adapter_out_of_sync', + 'reason_code' => 'run.adapter_out_of_sync', + 'source' => 'adapter_reconciler', + 'adapter' => 'environment_review_compose', + 'decision' => 'reconciled_succeeded', + 'related' => [ + 'review' => [ + 'id' => 88, + ], + ], + ], + ], + ]); + + Filament::setTenant(null, true); + $this->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]); + session([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]); + + Livewire::actingAs($user) + ->test(TenantlessOperationRunViewer::class, ['run' => $run]) + ->assertSee('A matching review was already available.') + ->assertSee('No action needed. A matching review was already available.'); +}); diff --git a/apps/platform/tests/Feature/System/Spec114/OpsFailuresViewTest.php b/apps/platform/tests/Feature/System/Spec114/OpsFailuresViewTest.php index 1d8a1fc9..49804e18 100644 --- a/apps/platform/tests/Feature/System/Spec114/OpsFailuresViewTest.php +++ b/apps/platform/tests/Feature/System/Spec114/OpsFailuresViewTest.php @@ -145,3 +145,47 @@ ->assertSee('Review composition') ->assertSee('Review pack generation'); }); + +it('shows Spec359 attention copy for review-compose failures on the system failures page', function () { + $tenant = ManagedEnvironment::factory()->create(); + + $run = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'type' => 'environment.review.compose', + 'status' => OperationRunStatus::Completed->value, + 'outcome' => OperationRunOutcome::Failed->value, + 'context' => [ + 'reconciliation' => [ + 'reconciled_at' => now()->toIso8601String(), + 'reason' => 'run.adapter_out_of_sync', + 'reason_code' => 'run.adapter_out_of_sync', + 'source' => 'adapter_reconciler', + 'adapter' => 'environment_review_compose', + 'decision' => 'attention_required', + 'related' => [ + 'review' => [ + 'id' => 654, + ], + ], + ], + ], + 'failure_summary' => [ + ['code' => 'run.adapter_out_of_sync', 'message' => 'TenantPilot found matching review activity, but it could not be resolved automatically.'], + ], + ]); + + $platformUser = PlatformUser::factory()->create([ + 'capabilities' => [ + PlatformCapabilities::ACCESS_SYSTEM_PANEL, + PlatformCapabilities::OPERATIONS_VIEW, + ], + 'is_active' => true, + ]); + + $this->actingAs($platformUser, 'platform') + ->get('/system/ops/failures') + ->assertSuccessful() + ->assertSee(SystemOperationRunLinks::view($run)) + ->assertSee('Inspect the recorded review lineage before retrying.'); +}); diff --git a/apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359AdapterRunReconcilerSupportedTypesTest.php b/apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359AdapterRunReconcilerSupportedTypesTest.php new file mode 100644 index 00000000..c776ff25 --- /dev/null +++ b/apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359AdapterRunReconcilerSupportedTypesTest.php @@ -0,0 +1,18 @@ +supportedTypes())->toBe([ + 'restore.execute', + 'environment.review.compose', + ]); +}); + +it('rejects unsupported adapter filters for Spec359', function (): void { + expect(fn () => app(AdapterRunReconciler::class)->reconcile([ + 'type' => 'policy.sync', + ]))->toThrow(InvalidArgumentException::class, 'Unsupported adapter run type: policy.sync'); +}); diff --git a/apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359EnvironmentReviewComposeAdapterTest.php b/apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359EnvironmentReviewComposeAdapterTest.php new file mode 100644 index 00000000..06bcf494 --- /dev/null +++ b/apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359EnvironmentReviewComposeAdapterTest.php @@ -0,0 +1,161 @@ +create(array_replace([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'initiator_name' => $user->name, + 'type' => 'environment.review.compose', + 'status' => 'queued', + 'outcome' => 'pending', + 'context' => [ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'review_fingerprint' => $fingerprint, + ], + ], $overrides)); + + return [$run, $tenant, $user, $snapshot]; +} + +it('returns reconciled_succeeded for a same-scope ready review in Spec359', function (): void { + [$run, $tenant, $user, $snapshot] = spec359ReviewComposeFixture(); + $otherRun = OperationRun::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'type' => 'environment.review.compose', + ]); + + $review = EnvironmentReview::factory()->ready()->create([ + 'workspace_id' => (int) $run->workspace_id, + 'managed_environment_id' => (int) $run->managed_environment_id, + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'operation_run_id' => (int) $otherRun->getKey(), + 'fingerprint' => (string) data_get($run->context, 'review_fingerprint'), + 'summary' => [ + 'finding_count' => 4, + 'report_count' => 2, + 'operation_count' => 7, + ], + ]); + + $result = app(EnvironmentReviewComposeDecision::class)->evaluate($run); + + expect($result->decision)->toBe('reconciled_succeeded') + ->and(data_get($result->related, 'review.id'))->toBe((int) $review->getKey()) + ->and($result->summaryCounts)->toMatchArray([ + 'finding_count' => 4, + 'report_count' => 2, + 'operation_count' => 7, + ]); +}); + +it('returns not_reconciled for the current in-progress draft review in Spec359', function (): void { + [$run, $tenant, $user, $snapshot] = spec359ReviewComposeFixture(); + + $review = EnvironmentReview::factory()->create([ + 'workspace_id' => (int) $run->workspace_id, + 'managed_environment_id' => (int) $run->managed_environment_id, + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'operation_run_id' => (int) $run->getKey(), + 'fingerprint' => (string) data_get($run->context, 'review_fingerprint'), + 'status' => EnvironmentReviewStatus::Draft->value, + ]); + + $run->update([ + 'context' => array_replace(is_array($run->context) ? $run->context : [], [ + 'review_id' => (int) $review->getKey(), + ]), + ]); + + $result = app(EnvironmentReviewComposeDecision::class)->evaluate($run->fresh()); + + expect($result->decision)->toBe('not_reconciled') + ->and(data_get($result->related, 'review.id'))->toBe((int) $review->getKey()); +}); + +it('returns blocked when an explicit successor review is still draft in Spec359', function (): void { + [$run, $tenant, $user, $snapshot] = spec359ReviewComposeFixture(); + + $successor = EnvironmentReview::factory()->create([ + 'workspace_id' => (int) $run->workspace_id, + 'managed_environment_id' => (int) $run->managed_environment_id, + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'fingerprint' => (string) data_get($run->context, 'review_fingerprint'), + 'status' => EnvironmentReviewStatus::Draft->value, + ]); + + $review = EnvironmentReview::factory()->create([ + 'workspace_id' => (int) $run->workspace_id, + 'managed_environment_id' => (int) $run->managed_environment_id, + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'operation_run_id' => (int) $run->getKey(), + 'fingerprint' => (string) data_get($run->context, 'review_fingerprint'), + 'status' => EnvironmentReviewStatus::Superseded->value, + 'superseded_by_review_id' => (int) $successor->getKey(), + ]); + + $run->update([ + 'context' => array_replace(is_array($run->context) ? $run->context : [], [ + 'review_id' => (int) $review->getKey(), + ]), + ]); + + $result = app(EnvironmentReviewComposeDecision::class)->evaluate($run->fresh()); + + expect($result->decision)->toBe('blocked') + ->and(data_get($result->related, 'review.id'))->toBe((int) $successor->getKey()); +}); + +it('fails closed when multiple usable same-scope reviews exist in Spec359', function (): void { + [$run, $tenant, $user, $snapshot] = spec359ReviewComposeFixture(); + $fingerprint = (string) data_get($run->context, 'review_fingerprint'); + + EnvironmentReview::factory()->ready()->create([ + 'workspace_id' => (int) $run->workspace_id, + 'managed_environment_id' => (int) $run->managed_environment_id, + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'fingerprint' => $fingerprint, + ]); + + EnvironmentReview::factory()->published()->create([ + 'workspace_id' => (int) $run->workspace_id, + 'managed_environment_id' => (int) $run->managed_environment_id, + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + 'initiated_by_user_id' => (int) $user->getKey(), + 'fingerprint' => $fingerprint, + ]); + + $result = app(EnvironmentReviewComposeDecision::class)->evaluate($run); + + expect($result->decision)->toBe('attention_required') + ->and($result->shouldFinalizeRun())->toBeTrue(); +}); diff --git a/apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359ReconciliationResultTest.php b/apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359ReconciliationResultTest.php new file mode 100644 index 00000000..6155f280 --- /dev/null +++ b/apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359ReconciliationResultTest.php @@ -0,0 +1,62 @@ +value; + + $result = ReconciliationResult::reconciledSucceeded( + reasonCode: $reasonCode, + reasonMessage: 'A matching review was already available.', + evidence: ['fingerprint' => 'abc'], + related: ['review' => ['id' => 55]], + summaryCounts: ['finding_count' => 3], + ); + + expect($result->decision)->toBe('reconciled_succeeded') + ->and($result->shouldFinalizeRun())->toBeTrue() + ->and($result->safeForAutoCompletion)->toBeTrue() + ->and($result->toArray())->toMatchArray([ + 'decision' => 'reconciled_succeeded', + 'reason_code' => $reasonCode, + 'reason_message' => 'A matching review was already available.', + 'evidence' => ['fingerprint' => 'abc'], + 'related' => ['review' => ['id' => 55]], + 'summary_counts' => ['finding_count' => 3], + ]); +}); + +it('keeps unsupported and not_reconciled results non-terminal for Spec359', function (): void { + $reasonCode = LifecycleReconciliationReason::AdapterOutOfSync->value; + + $unsupported = ReconciliationResult::unsupported($reasonCode, 'Unsupported adapter type.'); + $notReconciled = ReconciliationResult::notReconciled($reasonCode, 'No matching review truth yet.'); + + expect($unsupported->shouldFinalizeRun())->toBeFalse() + ->and($unsupported->safeForAutoCompletion)->toBeFalse() + ->and($unsupported->decision)->toBe('unsupported') + ->and($notReconciled->shouldFinalizeRun())->toBeFalse() + ->and($notReconciled->safeForAutoCompletion)->toBeFalse() + ->and($notReconciled->decision)->toBe('not_reconciled'); +}); + +it('keeps blocked and attention-required Spec359 decisions terminal and failure-backed', function (): void { + $reasonCode = LifecycleReconciliationReason::AdapterOutOfSync->value; + + $blocked = ReconciliationResult::blocked($reasonCode, 'A matching review exists, but it is not ready yet.'); + $attention = ReconciliationResult::attentionRequired($reasonCode, 'Matching review activity needs operator review.'); + $failed = ReconciliationResult::failedUnrecoverable($reasonCode, 'The run could not be recovered automatically.'); + + expect($blocked->shouldFinalizeRun())->toBeTrue() + ->and($blocked->decision)->toBe('blocked') + ->and($blocked->failures)->toHaveCount(1) + ->and($attention->shouldFinalizeRun())->toBeTrue() + ->and($attention->decision)->toBe('attention_required') + ->and($attention->failures)->toHaveCount(1) + ->and($failed->shouldFinalizeRun())->toBeTrue() + ->and($failed->decision)->toBe('failed_unrecoverable') + ->and($failed->failures)->toHaveCount(1); +}); diff --git a/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/checklists/requirements.md b/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/checklists/requirements.md new file mode 100644 index 00000000..181c11af --- /dev/null +++ b/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/checklists/requirements.md @@ -0,0 +1,64 @@ +# Requirements Checklist: Spec 359 - OperationRun Reconciliation Adapter Framework & Review Compose Adapter + +**Purpose**: Preparation analysis for Spec 359 readiness +**Created**: 2026-06-06 +**Feature**: `specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/spec.md` + +## Candidate Selection And Guardrails + +- [x] CHK001 The candidate source is explicit: direct user-provided draft plus repo-verified current code truth. +- [x] CHK002 No `specs/359-*` package existed before this prep. +- [x] CHK003 Related existing specs were checked for completed/prepared/runtime signals and are treated as context only: 357, 358, and 311. +- [x] CHK004 The active candidate queue's `no safe automatic next-best-prep target` note is respected; this package is an intentional manual promotion rather than an auto-selected queue item. +- [x] CHK005 Repo-truth deviations from the user draft are recorded in `spec.md`, especially the existing restore adapter precedent and current reconciliation metadata seam. + +## Required Prep Artifacts + +- [x] CHK006 `spec.md` exists and contains no template placeholders. +- [x] CHK007 `plan.md` exists and is repo-aware. +- [x] CHK008 `tasks.md` exists and is ordered, small, and verifiable. +- [x] CHK009 This checklist exists. + +## Spec Quality + +- [x] CHK010 Spec Candidate Check is completed and scored above the approval threshold. +- [x] CHK011 The spec keeps `OperationRun` persistence unchanged and explicitly forbids a new status/reconciled column. +- [x] CHK012 The spec explains why a narrow adapter contract is justified now despite the constitution's anti-abstraction bias. +- [x] CHK013 The spec keeps scope bounded to review-compose truth, duplicate recovery, and existing monitoring/review-start surfaces. +- [x] CHK014 The proportionality review rejects a universal reconciliation engine, schema expansion, and cross-domain adapter sprawl. + +## Plan / Task Alignment + +- [x] CHK015 The plan identifies the actual repo surfaces likely to change, including the current restore adapter precedent and current review-compose seams. +- [x] CHK016 The plan keeps Filament v5 / Livewire v4 posture and provider-registration location visible. +- [x] CHK017 The plan explicitly requires a PGSQL lane for duplicate-index and lock behavior. +- [x] CHK018 The tasks start with repo truth and failing tests before runtime edits. +- [x] CHK019 The tasks include explicit anti-creep guardrails against new persistence, universal adapters, and destructive cleanup. + +## UI / Monitoring / Review Coverage + +- [x] CHK020 UI Surface Impact is completed and does not claim a new page family. +- [x] CHK021 The changed surfaces are correctly classified as existing monitoring/detail and review-start feedback follow-through, not a new strategic customer surface. +- [x] CHK022 No new page-report identity or route-inventory expansion is required unless implementation proves a materially new visible hierarchy. +- [x] CHK023 Audience-aware disclosure and no-raw-SQL wording boundaries are explicit. + +## Test Governance + +- [x] CHK024 The declared test families are the narrowest honest proof: Unit + Feature + one bounded Browser smoke, with PGSQL lane coverage for duplicate-index truth. +- [x] CHK025 No heavy-governance family is introduced. +- [x] CHK026 Planned validation commands are explicit and include Spec 358 plus review/report regressions. + +## Readiness Gate Outcome + +- [x] CHK027 Candidate Selection Gate passes. +- [x] CHK028 Spec Readiness Gate passes. +- [x] CHK029 Runtime implementation has not started in this preparation step. +- [x] CHK030 Recommended next step is implementation, not more prep. + +## Review Outcome + +- [x] Outcome class: acceptable-special-case +- [x] Workflow outcome: keep +- [x] Final note location: active feature PR close-out entry `Guardrail / Smoke Coverage` +- [x] Preparation analyze result: pass via repo-based artifact review checklist; no standalone local `speckit.analyze` command was available in this repo surface +- [x] Tooling note: Spec Kit branch/spec creation succeeded via `create-new-feature.sh`, `setup-plan.sh` generated the plan file, and `tasks.md` plus this checklist were authored manually because this repo surface does not expose standalone local tasks/analyze generators diff --git a/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/plan.md b/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/plan.md new file mode 100644 index 00000000..cbea5fe1 --- /dev/null +++ b/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/plan.md @@ -0,0 +1,273 @@ +# Implementation Plan: OperationRun Reconciliation Adapter Framework & Review Compose Adapter + +**Branch**: `359-operationrun-reconciliation-adapter-framework-review-compose-adapter` | **Date**: 2026-06-06 | **Spec**: `specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/spec.md` +**Input**: Feature specification from `specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/spec.md` + +## Summary + +Implement a narrow business-truth reconciliation seam for `OperationRun` that reuses existing service-owned lifecycle writes and existing review-compose start/monitoring UX. The slice introduces one typed adapter contract plus one `environment.review.compose` adapter, proves same-scope `EnvironmentReview` truth, recovers duplicate fingerprint collisions deterministically, and keeps restore-specific reconciliation behavior intact as existing context rather than reopening it as a second feature. + +This plan must stay smaller than a universal reconciliation engine. It extends current repo seams only where current review-compose truth cannot be proven safely by the existing scheduled stale-run reconciler or the hard-coded restore adapter. + +## Technical Context + +**Language/Version**: PHP 8.4.15, Laravel 12.52, Filament 5.2.1, Livewire 4.1.4 +**Primary Dependencies**: existing `OperationRunService`, `OperationRun`, `OperationCatalog`, current restore adapter reconciler, current review-compose services/jobs/resources, Pest 4.3 +**Storage**: PostgreSQL `operation_runs` and `environment_reviews`; no schema change planned +**Testing**: Pest Unit + Feature + one bounded Browser smoke; PGSQL lane required for duplicate-index/locking proof +**Validation Lanes**: fast-feedback, confidence, pgsql, browser +**Target Platform**: Laravel monolith in `apps/platform` +**Project Type**: web application +**Performance Goals**: reuse current DB-only monitoring posture, avoid new render-time Graph work, and keep adapter evaluation to scoped local DB reads only +**Constraints**: no new `OperationRun` status/outcome field, no new review status family, no new panel/provider/asset change, no provider-boundary widening, no destructive cleanup, and no broad restore/report/evidence adapter expansion +**Scale/Scope**: one new review-compose path on the existing adapter reconciliation seam plus the smallest local decision/result helper needed to make adapter reconciliation auditable and repeatable + +## Likely Affected Repo Surfaces + +- Existing reconciliation and lifecycle seams: + - `apps/platform/app/Services/OperationRunService.php` + - `apps/platform/app/Models/OperationRun.php` + - `apps/platform/app/Services/Operations/OperationLifecycleReconciler.php` + - `apps/platform/app/Services/AdapterRunReconciler.php` + - `apps/platform/app/Support/OperationCatalog.php` + - `apps/platform/app/Support/OperationRunType.php` + - `apps/platform/app/Support/OperationRunStatus.php` + - `apps/platform/app/Support/OperationRunOutcome.php` +- Existing review-compose runtime seams: + - `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewService.php` + - `apps/platform/app/Jobs/ComposeEnvironmentReviewJob.php` + - `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewComposer.php` + - `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewFingerprint.php` + - `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewLifecycleService.php` + - `apps/platform/app/Models/EnvironmentReview.php` +- Existing operator-facing surfaces likely to consume richer metadata: + - `apps/platform/app/Support/OpsUx/OperationUxPresenter.php` + - `apps/platform/app/Support/Ui/GovernanceArtifactTruth/ArtifactTruthPresenter.php` + - `apps/platform/app/Support/OperationRunLinks.php` + - `apps/platform/app/Filament/Pages/Monitoring/Operations.php` + - `apps/platform/app/Filament/Pages/Operations/TenantlessOperationRunViewer.php` + - `apps/platform/app/Filament/Resources/OperationRunResource.php` + - `apps/platform/app/Filament/Resources/EnvironmentReviewResource.php` + - `apps/platform/app/Filament/Widgets/Dashboard/RecentOperations.php` + - `apps/platform/app/Filament/System/Pages/Ops/Runs.php` + - `apps/platform/app/Support/EnvironmentDashboard/EnvironmentDashboardSummaryBuilder.php` + - `apps/platform/lang/en/localization.php` + - `apps/platform/lang/de/localization.php` +- Focused tests: + - `apps/platform/tests/Unit/Support/Operations/Reconciliation/` + - `apps/platform/tests/Feature/Operations/` + - `apps/platform/tests/Feature/EnvironmentReview/` + - `apps/platform/tests/Feature/Monitoring/` + - `apps/platform/tests/Browser/` + +## Domain And Data Implications + +- `operation_runs` remains the only persisted run-lifecycle truth. Reconciliation remains a context extension, not a new status model. +- `environment_reviews` remains the only domain-truth source for review availability. The adapter proves truth from existing review rows only. +- The partial unique index `environment_reviews_fingerprint_mutable_unique` remains the core duplicate-collision constraint. The feature must adapt to it, not replace it. +- Existing mutable/superseded semantics remain authoritative: + - mutable: `draft`, `ready`, `failed` + - historical/terminal lineage: `published`, `archived`, `superseded` +- No migration is planned. If implementation appears to need one, that is a scope alarm and should stop for review. + +## UI / Surface Guardrail Plan + +- **Guardrail scope**: existing operator-facing monitoring surfaces plus existing review-start feedback +- **Affected routes/pages/actions/states/navigation/panel/provider surfaces**: + - `/admin/workspaces/{workspace}/operations` + - `/admin/workspaces/{workspace}/operations/{run}` + - `EnvironmentReviewResource` create/refresh notifications and open-link feedback + - tenant dashboard recent-operations widget + - system operations list + - environment dashboard recent/attention summaries +- **No-impact class, if applicable**: non-material existing-surface wording/state handling only +- **Native vs custom classification summary**: native Filament surfaces plus existing action notifications +- **Shared-family relevance**: monitoring-state messaging, reconciliation diagnostics, run-start feedback +- **State layers in scope**: page, detail, action feedback +- **Audience modes in scope**: operator-MSP, support-platform +- **Decision/diagnostic/raw hierarchy plan**: decision-first reconciliation result, diagnostics second, raw evidence third +- **Raw/support gating plan**: existing detail/raw context remains secondary and must never become the primary explanation +- **One-primary-action / duplicate-truth control**: keep the existing open-run/open-review actions authoritative and avoid a second local “duplicate recovered” surface +- **Handling modes by drift class or surface**: + - review-compose succeeded from domain truth: `review-mandatory` + - blocked/attention without safe proof: `review-mandatory` + - any raw SQL/constraint leak: `hard-stop-candidate` +- **Repository-signal treatment**: review-mandatory +- **Special surface test profiles**: monitoring-state-page, shared-detail-family +- **Required tests or manual smoke**: Unit + Feature + PGSQL + one bounded Browser smoke +- **Exception path and spread control**: any move toward new operator-center UX, report/evidence adapter copy, or customer-facing surface changes is `reject-or-split` +- **Active feature PR close-out entry**: Guardrail / Smoke Coverage +- **UI/Productization coverage decision**: existing surface follow-through only +- **Coverage artifacts to update**: none by default; update existing page reports only if final visible hierarchy materially changes +- **No-impact rationale**: no route or page identity change +- **Navigation / Filament provider-panel handling**: unchanged; provider registration stays in `apps/platform/bootstrap/providers.php` +- **Screenshot or page-report need**: no new page-report identity; one bounded browser smoke screenshot is sufficient only if implementation materially changes visible wording hierarchy + +## Shared Pattern & System Fit + +- **Cross-cutting feature marker**: yes +- **Systems touched**: + - scheduled reconciliation path + - restore-specific adapter reconciliation precedent + - review-compose start/complete flow + - monitoring list/detail surfaces + - review-create notifications +- **Shared abstractions reused**: + - `OperationRunService::updateRunWithReconciliation()` + - `OperationRun::reconciliation()` + - `OperationUxPresenter` + - `OperationRunLinks` + - `ArtifactTruthPresenter` + - `EnvironmentReviewResource::executeCreateReview()` +- **New abstraction introduced? why?**: yes; one local review-compose decision/result helper is introduced because the current restore-only adapter path is too hard-coded to absorb review fingerprint/scope/supersession proof safely. +- **Why the existing abstraction was sufficient or insufficient**: current abstractions already own run lifecycle and generic stale truth, but `AdapterRunReconciler` currently only knows `restore.execute`; this slice extends that seam rather than adding a parallel registry. +- **Bounded deviation / spread control**: keep the new helper local to adapter reconciliation and explicitly bounded to review-compose plus existing restore context + +## OperationRun UX Impact + +- **Touches OperationRun start/completion/link UX?**: yes +- **Central contract reused**: current service-owned lifecycle plus current review-create queued-toast/open-run path +- **Delegated UX behaviors**: + - create/refresh keeps the current queued toast and `Open operation` link + - adapter completion writes terminal truth through `OperationRunService` + - operations list/detail continue to read one reconciliation metadata shape +- **Surface-owned behavior kept local**: only review-create body copy and any additional review-available/recovered wording +- **Queued DB-notification policy**: unchanged +- **Terminal notification path**: unchanged central lifecycle mechanism +- **Exception path**: none + +## Provider Boundary & Portability Fit + +- **Shared provider/platform boundary touched?**: no +- **Provider-owned seams**: N/A +- **Platform-core seams**: `OperationRun` lifecycle truth and `EnvironmentReview` artifact truth only +- **Neutral platform terms / contracts preserved**: `operation`, `reconciliation`, `review`, `workspace`, `managed environment` +- **Retained provider-specific semantics and why**: none +- **Bounded extraction or follow-up path**: none + +## Authorization, Audit, And Observability + +- Existing workspace-first and managed-environment entitlement checks remain unchanged. +- Adapter lookup must not widen who can see or use a review; it only determines which local record may prove the run. +- `OperationRunService` remains the owner of terminal audit emission. +- Reconciliation evidence must stay bounded to safe identifiers and state: + - allowed: workspace ID, managed environment ID, review ID, review status, fingerprint hash, considered review IDs, timestamps + - forbidden: raw Graph payloads, signed URLs, tokens, raw SQL errors, full snapshot blobs +- Optional operational logging may emit adapter name, run ID, type, decision, and related review ID only. + +## Constitution Check + +- Inventory-first: unchanged; `EnvironmentReview` remains the local artifact truth and still derives from anchored evidence snapshots. +- Read/write separation: adapter reconciliation mutates only `OperationRun`, not the provider or remote state. +- Graph contract path: unchanged; no Graph calls are introduced. +- Deterministic capabilities: unchanged; current capability checks remain authoritative. +- Workspace and tenant isolation: unchanged and still mandatory for both runs and reviews. +- Run observability: strengthened; late or duplicate review-compose work remains visible on `OperationRun` instead of being silently inferred elsewhere. +- Proportionality / anti-bloat: pass only if the adapter contract stays narrow and does not become a universal business-reconciliation engine. +- No premature abstraction: this is allowed only because queue correctness and auditability already justify a typed seam, and the repo already ships one restore-specific adapter precedent. +- Persisted truth: no new tables or persisted status families. +- Shared pattern first: reuse current run lifecycle, current monitoring/detail surfaces, and current review-start feedback. +- Filament v5 / Livewire v4 posture: unchanged; no panel/provider or asset work is allowed. + +## Test Governance Check + +- **Test purpose / classification by changed surface**: + - Unit: reconciliation decisions, supported-type handling, adapter same-scope proof + - Feature: service/job/idempotency/reconciliation metadata flow + - Browser: visible operations/review wording smoke +- **Affected validation lanes**: fast-feedback, confidence, pgsql, browser +- **Why this lane mix is the narrowest sufficient proof**: duplicate fingerprint recovery depends on a PGSQL partial unique index and scope-safe DB truth, while visible monitoring wording needs only one bounded browser smoke rather than a broad productization run. +- **Narrowest proving command(s)**: + - `cd apps/platform && ./vendor/bin/sail artisan test --compact --filter=Spec359` + - `cd apps/platform && ./vendor/bin/sail php vendor/bin/pest -c phpunit.pgsql.xml --filter=Spec359` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact --filter=Spec358` + - `git diff --check` +- **Fixture / helper / factory / seed / context cost risks**: reuse existing review/evidence/run factories; do not widen default tenant/workspace setup +- **Expensive defaults or shared helper growth introduced?**: none planned +- **Heavy-family additions, promotions, or visibility changes**: none +- **Surface-class relief / special coverage rule**: monitoring-state-page plus shared-detail-family; one explicit browser smoke only +- **Closing validation and reviewer handoff**: reviewers should compare ready-review reuse, ambiguous duplicate collisions, repeated create/refresh triggers, and visible operations detail wording before merge +- **Budget / baseline / trend follow-up**: none expected beyond feature-local tests +- **Review-stop questions**: + - Did implementation add a second persisted status shape? + - Did implementation widen into report/evidence/restore business reconciliation? + - Can any duplicate/superseded case still remain queued/running forever? + - Does any visible copy leak SQL/constraint wording? +- **Escalation path**: `reject-or-split` for universal reconciliation scope growth +- **Active feature PR close-out entry**: Guardrail / Smoke Coverage +- **Why no dedicated follow-up spec is needed**: this slice is already the follow-through over Spec 358 and must remain bounded to review-compose truth only + +## Implementation Phases + +### Phase 1 — Lock repo truth and convergence boundaries + +- Reconfirm the current scheduled reconciler, restore-specific adapter reconciler, and review-compose seams. +- Decide whether the existing restore adapter stays as-is or is bridged behind the new contract without behavioral change. +- Freeze the current visible operations/review feedback vocabulary that must remain consistent. + +### Phase 2 — Add the narrow reconciliation contract + +- Extend the existing adapter reconciliation seam with one local review-compose decision/result helper and explicit supported-type handling. +- Extend or wrap `OperationRunService::updateRunWithReconciliation()` so richer metadata can be merged idempotently. +- Keep all persistence shapes unchanged. + +### Phase 3 — Implement review-compose proof and recovery + +- Add a same-scope review-compose adapter that proves ready/published review truth and fails closed on ambiguity. +- Integrate duplicate recovery and pre/post compose checks into `ComposeEnvironmentReviewJob`. +- Tighten `EnvironmentReviewService` repeated-trigger behavior so active review truth or active run truth is reused rather than multiplied. + +### Phase 4 — Retrofit existing operator surfaces + +- Keep current monitoring list/detail surfaces authoritative for reconciled review-compose explanation. +- Reuse current review-start feedback and current `Open operation` / `View review` patterns. +- Add bounded EN/DE copy only where the existing language is too generic or too technical. + +### Phase 5 — Validate and hand off + +- Add Unit + Feature + PGSQL + Browser proof. +- Re-run Spec 358 plus the named review/report regressions. +- Record final guardrail, smoke, and bounded-scope notes in the feature close-out entry. + +## Risks and Mitigations + +- **Framework creep**: mitigate by forbidding new adapter families and by treating restore/report/evidence follow-through as out of scope. +- **Scope-safety mistakes**: mitigate with explicit cross-workspace, cross-environment, and wrong-fingerprint negative tests. +- **PGSQL-only blind spots**: mitigate with an explicit `phpunit.pgsql.xml` lane for duplicate-index and lock behavior. +- **Visible copy drift**: mitigate by keeping all user-facing wording on current operations/review paths and adding one bounded browser smoke. + +## Project Structure + +### Documentation + +```text +specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/ +├── spec.md +├── plan.md +├── tasks.md +└── checklists/ + └── requirements.md +``` + +### Likely Runtime Structure + +```text +apps/platform/app/Services/ +├── OperationRunService.php +├── AdapterRunReconciler.php +└── EnvironmentReviews/ + ├── EnvironmentReviewService.php + ├── EnvironmentReviewComposer.php + ├── EnvironmentReviewFingerprint.php + └── EnvironmentReviewLifecycleService.php + +apps/platform/app/Jobs/ +└── ComposeEnvironmentReviewJob.php + +apps/platform/app/Filament/ +├── Pages/Monitoring/Operations.php +├── Pages/Operations/TenantlessOperationRunViewer.php +└── Resources/ + ├── OperationRunResource.php + └── EnvironmentReviewResource.php +``` diff --git a/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/spec.md b/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/spec.md new file mode 100644 index 00000000..7f23c455 --- /dev/null +++ b/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/spec.md @@ -0,0 +1,379 @@ +# Feature Specification: OperationRun Reconciliation Adapter Framework & Review Compose Adapter + +**Feature Branch**: `359-operationrun-reconciliation-adapter-framework-review-compose-adapter` +**Created**: 2026-06-06 +**Status**: Draft +**Input**: User-provided Spec 359 draft, reconciled against current repo truth + +## Spec Candidate Check *(mandatory — SPEC-GATE-001)* + +- **Problem**: `environment.review.compose` runs can remain queued/running or later appear stale on monitoring surfaces even when matching review truth already exists or a duplicate-fingerprint collision proves the original compose path is no longer the source of truth. +- **Today's failure**: the repo can leave ghost `OperationRun` records after late compose jobs or `environment_reviews_fingerprint_mutable_unique` collisions, forcing operators to reconcile review truth mentally across runs, reviews, and duplicate errors. +- **User-visible improvement**: review-compose work will end in one auditable, scope-safe outcome: matching review reused, still in progress, or attention required, without pretending generic queue truth is business completion. +- **Smallest enterprise-capable version**: extend the existing `AdapterRunReconciler` seam with one additional `environment.review.compose` path plus one local decision/result helper that can prove same-scope review truth and complete the `OperationRun` through the existing service-owned lifecycle path. +- **Explicit non-goals**: no new `operation_runs` status column, no new `environment_reviews` schema, no report/evidence/sync/backup/alert adapters, no universal operator-center redesign, no destructive cleanup, and no speculative multi-provider framework. +- **Permanent complexity imported**: one bounded review-compose decision/result helper, richer reconciliation metadata, and focused Unit/Feature/Browser/PGSQL coverage. +- **Why now**: Spec 358 fixed generic queue truth. The remaining repo-real trust gap is domain-truth reconciliation for review composition, especially around duplicate fingerprint collisions and late jobs. +- **Why not local**: a job-local duplicate catch or a create-review fast-path tweak would not fix late/superseded runs, monitoring detail, audit metadata, or idempotent repeat-trigger behavior consistently. +- **Approval class**: Core Enterprise +- **Red flags triggered**: new abstraction and new non-persisted decision/reason family. Defense: queue/job correctness plus auditability justify one bounded second adapter path here, but not a new registry layer. +- **Score**: Nutzen: 2 | Dringlichkeit: 2 | Scope: 2 | Komplexität: 1 | Produktnähe: 2 | Wiederverwendung: 2 | **Gesamt: 11/12** +- **Decision**: approve + +## Repo Truth Reconciliation + +The user draft is directionally correct, but current repo truth narrows the implementation shape: + +1. The repo already has two reconciliation families: + - scheduled stale-run reconciliation via `App\Services\Operations\OperationLifecycleReconciler` + - restore-specific adapter reconciliation via `App\Services\AdapterRunReconciler` + Spec 359 is therefore a follow-through over existing reconciliation seams, not a greenfield concept. +2. `App\Services\OperationRunService::updateRunWithReconciliation()` and `App\Models\OperationRun::reconciliation()` already own current reconciliation writes/reads. Spec 359 must extend or wrap that seam instead of introducing a second persisted reconciliation contract. +3. `App\Services\EnvironmentReviews\EnvironmentReviewService::create()` already short-circuits to an existing mutable review before dispatch, and `App\Filament\Resources\EnvironmentReviewResource` already exposes `review_already_available`. The remaining gap is late jobs, superseded review lineages, duplicate fingerprint collisions, and stale `OperationRun` records after runtime drift. +4. `EnvironmentReview` mutability is already constrained by the partial unique index `environment_reviews_fingerprint_mutable_unique` on `draft|ready|failed`. No new review state or new schema is justified for this slice. +5. Current monitoring surfaces already render generic `Automatically reconciled` lifecycle language. Spec 359 may add review-compose-specific explanation keys, but it must stay inside the current monitoring/detail families instead of inventing a second operator language. +6. `docs/product/spec-candidates.md` currently records `no safe automatic next-best-prep target`. This package is therefore an explicit manual promotion from the user and current repo truth, not an automatic queue selection. +7. Existing restore reconciliation must not be broken. Implementation may either preserve the current restore-specific reconciler or bridge it behind the new contract, but restore business logic expansion remains out of scope. + +## Spec Scope Fields *(mandatory)* + +- **Scope**: workspace, tenant, canonical-view +- **Primary Routes**: + - `/admin/workspaces/{workspace}/operations` + - `/admin/workspaces/{workspace}/operations/{run}` + - existing environment-scoped review create/refresh entry points via `App\Filament\Resources\EnvironmentReviewResource` +- **Data Ownership**: + - `operation_runs` remain the only persisted lifecycle/reconciliation owner for run state + - `environment_reviews` remain the only domain-truth source for review availability + - related `environment_review_sections` and `review_packs` are read-side context only when the chosen review already proves usable truth +- **RBAC**: + - existing workspace and managed-environment entitlement checks remain authoritative + - non-members remain `404` + - review creation/refresh remains capability-gated by `Capabilities::ENVIRONMENT_REVIEW_MANAGE` + - operation-run inspection remains governed by current workspace-first `OperationRun` access boundaries + +For canonical-view specs, the spec MUST define: + +- **Default filter behavior when tenant-context is active**: the operations hub stays workspace-scoped with explicit environment filters. Adapter decisions must use the run's recorded workspace/environment scope and must not infer broader tenant context from session, remembered tenant, or current page state. +- **Explicit entitlement checks preventing cross-tenant leakage**: no adapter may reconcile to an `EnvironmentReview` outside the run's workspace and managed environment. Any reused review link or related-model metadata must stay current-scope only. + +## UI Surface Impact *(mandatory — UI-COV-001)* + +- [ ] No UI surface impact +- [x] Existing page changed +- [ ] New page/route added +- [ ] Navigation changed +- [ ] Filament panel/provider surface changed +- [ ] New modal/drawer/wizard/action added +- [ ] New table/form/state added +- [ ] Customer-facing surface changed +- [ ] Dangerous action changed +- [x] Status/evidence/review presentation changed +- [ ] Workspace/environment context presentation changed + +## UI/Productization Coverage *(mandatory when UI Surface Impact is not "No UI surface impact")* + +- **Route/page/surface**: + - workspace operations hub (`App\Filament\Pages\Monitoring\Operations`) + - canonical run detail (`App\Filament\Pages\Operations\TenantlessOperationRunViewer`) + - existing review-create/review-refresh feedback path in `App\Filament\Resources\EnvironmentReviewResource` + - tenant dashboard recent-operations widget (`App\Filament\Widgets\Dashboard\RecentOperations`) + - system operations list (`App\Filament\System\Pages\Ops\Runs`) + - environment dashboard recent/attention summaries (`App\Support\EnvironmentDashboard\EnvironmentDashboardSummaryBuilder`) +- **Current or new page archetype**: existing monitoring family plus existing review-initiation action feedback +- **Design depth**: Domain Pattern Surface +- **Repo-truth level**: repo-verified +- **Existing pattern reused**: current monitoring reconciliation banners, current `review_already_available` / `open_operation` feedback, current review-output action affordances +- **New pattern required**: none; extend existing monitoring and review-start feedback only +- **Screenshot required**: no; one bounded browser smoke is sufficient unless implementation proves a materially new visible hierarchy +- **Page audit required**: no new page-report identity is required; existing anchors remain `docs/ui-ux-enterprise-audit/page-reports/ui-003-operations.md`, `ui-011-reviews.md`, and `ui-040-environment-review-detail.md` +- **Customer-safe review required**: no; changed copy remains operator-facing +- **Dangerous-action review required**: no; no destructive or high-risk confirmation flow changes +- **Coverage files updated or explicitly not needed**: + - [ ] `docs/ui-ux-enterprise-audit/route-inventory.md` + - [ ] `docs/ui-ux-enterprise-audit/design-coverage-matrix.md` + - [x] `docs/ui-ux-enterprise-audit/page-reports/...` + - [ ] `docs/ui-ux-enterprise-audit/strategic-surfaces.md` + - [ ] `docs/ui-ux-enterprise-audit/grouped-follow-up-candidates.md` + - [ ] `docs/ui-ux-enterprise-audit/unresolved-pages.md` + - [ ] `N/A - no reachable UI surface impact` +- **No-impact rationale when applicable**: N/A + +## Cross-Cutting / Shared Pattern Reuse *(mandatory)* + +- **Cross-cutting feature?**: yes +- **Interaction class(es)**: status messaging, operation lifecycle explanation, action links, queued/already-available feedback, reconciliation diagnostics +- **Systems touched**: + - `OperationRunService` reconciliation metadata + - `OperationRun::reconciliation()` + - current monitoring list/detail presenters + - `EnvironmentReviewResource` create/refresh feedback + - shared review artifact and link readers + - current localization families under `localization.review` and operation-run detail copy +- **Existing pattern(s) to extend**: current reconciliation metadata path, current operations monitoring/detail language, current `review_already_available` notification pattern, and current shared review artifact/link resolution +- **Shared contract / presenter / builder / renderer to reuse**: `OperationRunService::updateRunWithReconciliation()`, `OperationRun::reconciliation()`, `OperationUxPresenter`, `OperationRunLinks`, `ArtifactTruthPresenter`, and existing `EnvironmentReviewResource::executeCreateReview()` affordances +- **Why the existing shared path is sufficient or insufficient**: existing shared paths already own run status, links, and generic stale truth, but the restore-specific reconciler is hard-coded and cannot safely prove review fingerprint/scope/supersession truth without new feature-local branching. +- **Allowed deviation and why**: none by default; any new adapter copy or metadata must feed the existing presenter/resource seams rather than local exception prose in jobs or pages. +- **Consistency impact**: the same review-compose reconciliation outcome must read consistently on start feedback, operations list/detail, and technical diagnostics. +- **Review focus**: no parallel local reconciliation language, no direct model mutation outside services, and no raw duplicate-key/SQL wording leaking into customer-visible or operator-primary copy. + +## OperationRun UX Impact *(mandatory)* + +- **Touches OperationRun start/completion/link UX?**: yes +- **Shared OperationRun UX contract/layer reused**: current `OperationRunService`, `OperationRunLinks`, `OperationUxPresenter`, and existing review-create feedback path +- **Delegated start/completion UX behaviors**: + - review creation keeps the current queued toast plus `Open operation` link + - adapter reconciliation writes terminal truth through the current service-owned lifecycle path + - operations list/detail continue to consume the canonical `OperationRun` reconciliation metadata +- **Local surface-owned behavior that remains**: the review-create form input and any review-specific success/blocked body copy +- **Queued DB-notification policy**: unchanged; remain on the current shared contract +- **Terminal notification path**: central lifecycle mechanism remains authoritative +- **Exception required?**: none + +## Provider Boundary / Platform Core Check *(mandatory)* + +- **Shared provider/platform boundary touched?**: no +- **Boundary classification**: N/A +- **Seams affected**: N/A +- **Neutral platform terms preserved or introduced**: `operation`, `reconciliation`, `review`, `workspace`, `managed environment`, `artifact truth` +- **Provider-specific semantics retained and why**: none +- **Why this does not deepen provider coupling accidentally**: the adapter proves platform-owned `EnvironmentReview` truth from existing local records only; it does not add new Microsoft- or Graph-shaped platform-core semantics. +- **Follow-up path**: none + +## UI / Surface Guardrail Impact *(mandatory)* + +| Surface / Change | Operator-facing surface change? | Native vs Custom | Shared-Family Relevance | State Layers Touched | Exception Needed? | Low-Impact / `N/A` Note | +|---|---|---|---|---|---|---| +| Operations hub reconciliation wording | yes | Native Filament page | shared monitoring family | page, table row | no | no new route or action family | +| Tenantless run detail reconciliation explanation | yes | Native Filament page | shared monitoring detail family | detail | no | explanation only; diagnostics remain secondary | +| Review create / refresh feedback | yes | Native Filament action + notification | shared run-start feedback family | action feedback | no | reuses existing notification/action pattern | + +## Decision-First Surface Role *(mandatory)* + +| Surface | Decision Role | Human-in-the-loop Moment | Immediately Visible for First Decision | On-Demand Detail / Evidence | Why This Is Primary or Why Not | Workflow Alignment | Attention-load Reduction | +|---|---|---|---|---|---|---|---| +| Operations hub | Primary Decision Surface | Decide whether a stale review-compose run is actually completed, blocked, or needs attention | lifecycle state, review-availability result, one open action | full diagnostics and related review evidence | primary because it is the canonical operations triage queue | aligns to current monitoring workflow | removes mental cross-checking across review and run surfaces | +| Tenantless run detail | Tertiary Evidence / Diagnostics Surface | Confirm why the run was reconciled or blocked before reading raw context | one reconciled/blocked explanation with related review hint | raw context, evidence, lineage, failures | tertiary because the run is already selected | preserves current detail role | removes duplicate/raw-first explanations | +| Review create / refresh feedback | Secondary Context Surface | Decide whether to open the reused review, open the run, or do nothing | already-available vs queued background feedback | monitoring detail or review detail after click | secondary because it supports, not owns, the workflow | follows existing review-start pattern | avoids double-dispatch and duplicate click confusion | + +## Audience-Aware Disclosure *(mandatory)* + +| Surface | Audience Modes In Scope | Decision-First Default-Visible Content | Operator Diagnostics | Support / Raw Evidence | One Dominant Next Action | Hidden / Gated By Default | Duplicate-Truth Prevention | +|---|---|---|---|---|---|---|---| +| Operations hub | operator-MSP, support-platform | reconciled/blocked review-compose summary | related review ID and reason code secondarily | raw payloads remain on detail | `Open operation` | raw SQL/constraint detail stays hidden | list row states the result once | +| Tenantless run detail | operator-MSP, support-platform | calm reconciliation result and related review truth | evidence, related review metadata, reason code | low-level payloads after summary | existing back/open actions | raw duplicate error bodies stay secondary | banner and diagnostics must not restate conflicting truth | +| Review create / refresh feedback | operator-MSP | review reused vs background compose still running | none beyond open link | raw detail stays on operations/review pages | `View review` or `Open operation` | technical duplicate details hidden | reuse one notification family, not separate local wording | + +## UI/UX Surface Classification *(mandatory)* + +| Surface | Action Surface Class | Surface Type | Likely Next Operator Action | Primary Inspect/Open Model | Row Click | Secondary Actions Placement | Destructive Actions Placement | Canonical Collection Route | Canonical Detail Route | Scope Signals | Canonical Noun | Critical Truth Visible by Default | Exception Type / Justification | +|---|---|---|---|---|---|---|---|---|---|---|---|---|---| +| Operations hub | List / Table / Monitoring | Read-only monitoring registry | open the run that now needs review or no action | full-row open | required | existing table controls only | none | `/admin/workspaces/{workspace}/operations` | `/admin/workspaces/{workspace}/operations/{run}` | workspace scope plus explicit filters | Operation run | whether review truth already exists or still needs attention | none | +| Tenantless run detail | Record / Detail / Monitoring | Diagnostics-first detail | confirm the reconciled or blocked explanation | canonical detail page | N/A | existing header links only | none | `/admin/workspaces/{workspace}/operations` | `/admin/workspaces/{workspace}/operations/{run}` | workspace scope and entitled environment | Operation run | related review truth before raw diagnostics | none | +| Review create / refresh feedback | Action Feedback | Action result surface | open the review or open the run | action notification link | N/A | existing notification action set | none | current environment review surface | linked review or operation detail | tenant context on the current page | Review composition | already available vs queued/recovered state | none | + +## Operator Surface Contract *(mandatory)* + +| Surface | Primary Persona | Decision / Operator Action Supported | Surface Type | Primary Operator Question | Default-visible Information | Diagnostics-only Information | Status Dimensions Used | Mutation Scope | Primary Actions | Dangerous Actions | +|---|---|---|---|---|---|---|---|---|---|---| +| Operations hub | workspace operator | decide whether a review-compose run is done, blocked, or needs follow-up | monitoring registry | Do I still need to wait for compose, or is a matching review already the truth? | lifecycle outcome, related review availability, one open action | raw evidence, reason ownership, full context | lifecycle, reconciliation truth, artifact availability | none | open row | none | +| Tenantless run detail | workspace operator | confirm the exact reconciliation decision before technical follow-up | detail surface | Why did this run complete from review truth or remain blocked? | top-level reconciled/blocked explanation, related review summary | evidence array, considered review IDs, failures | lifecycle, reconciliation truth, artifact readiness | none | existing navigation links | none | + +## Proportionality Review *(mandatory when structural complexity is introduced)* + +- **New source of truth?**: no +- **New persisted entity/table/artifact?**: no +- **New abstraction?**: yes +- **New enum/state/reason family?**: yes, bounded non-persisted reconciliation decision/reason modeling +- **New cross-domain UI framework/taxonomy?**: no +- **Current operator problem**: current runtime can prove generic stale truth but cannot safely prove that review-compose business truth already exists, so ghost runs survive duplicate collisions and late jobs. +- **Existing structure is insufficient because**: the current scheduled reconciler only fails stale runs generically, and the current restore-only adapter reconciler is hard-coded to `restore.execute`; neither can prove review fingerprint, scope, mutable/superseded lineage, or safe auto-success for review composition. +- **Narrowest correct implementation**: extend the existing adapter reconciliation seam with one local review-compose decision/result helper and one review-compose path that reuses existing `OperationRunService` reconciliation writes. +- **Ownership cost**: one new bounded support layer, richer reconciliation metadata, and targeted test coverage across unit, feature, PGSQL, and one bounded browser smoke. +- **Alternative intentionally rejected**: job-local duplicate catch plus ad hoc review lookups was rejected because it would duplicate logic across job, service, and monitoring surfaces and would not produce one audit-visible reconciliation contract. +- **Release truth**: current-release queue correctness and auditability, not speculative future multi-artifact preparation. + +### Compatibility posture + +This feature assumes the current pre-production environment. Backward-compatibility aliases or migration shims are not justified unless implementation proves the existing restore adapter needs a bounded bridge to the new contract. No legacy write path should be added for historical rows. + +## Testing / Lane / Runtime Impact *(mandatory for runtime behavior changes)* + +- **Test purpose / classification**: Unit + Feature + Browser +- **Validation lane(s)**: fast-feedback, confidence, pgsql, browser +- **Why this classification and these lanes are sufficient**: Unit coverage proves result/supported-type/adapter decisions; Feature coverage proves service/job/idempotency behavior; the PGSQL lane is required for partial unique-index and locking truth around duplicate fingerprint recovery; one bounded Browser smoke proves the visible operations/review wording without widening into broad UI productization. +- **New or expanded test families**: + - `apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359ReconciliationResultTest.php` + - `apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359AdapterRunReconcilerSupportedTypesTest.php` + - `apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359EnvironmentReviewComposeAdapterTest.php` + - `apps/platform/tests/Feature/Operations/Spec359OperationRunAdapterReconciliationTest.php` + - `apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeReconciliationTest.php` + - `apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeIdempotencyTest.php` + - `apps/platform/tests/Browser/Spec359ReviewComposeReconciliationSmokeTest.php` +- **Fixture / helper cost impact**: low to moderate; reuse existing workspace/tenant/review factories and existing operations monitoring fixtures +- **Heavy-family visibility / justification**: no heavy-governance family added; the browser addition is explicit and bounded to one smoke file +- **Special surface test profile**: monitoring-state-page plus shared-detail-family +- **Standard-native relief or required special coverage**: required special coverage for PGSQL duplicate-index proof and one bounded browser smoke; otherwise ordinary Unit/Feature proof is sufficient +- **Reviewer handoff**: reviewers must confirm that duplicate or superseded review-compose cases end as succeeded/blocked/attention deterministically, never as endless queued/running ghosts, and that customer-facing or operator-primary copy never leaks SQL/constraint details +- **Budget / baseline / trend impact**: minor feature-local growth only +- **Escalation needed**: `reject-or-split` if implementation broadens into report/evidence/restore adapter scope, new persisted status, or a universal business reconciliation engine +- **Active feature PR close-out entry**: Guardrail / Smoke Coverage +- **Planned validation commands**: + - `cd apps/platform && ./vendor/bin/sail artisan test --compact --filter=Spec359` + - `cd apps/platform && ./vendor/bin/sail php vendor/bin/pest -c phpunit.pgsql.xml --filter=Spec359` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact --filter=Spec358` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Browser/CustomerReviewWorkspaceSmokeTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Browser/Spec357ReportProfilesSmokeTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/Spec357RenderedReportProfileTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/ReviewPackDownloadTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/EnvironmentReviewDerivedReviewPackTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/EnvironmentReviewExecutivePackTest.php` + - `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent` + - `git diff --check` + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Reuse proven review truth for a stale review-compose run (Priority: P1) + +As an operator, I need a stale or late `environment.review.compose` run to complete from existing review truth when a same-scope ready or published review already proves the result is available. + +**Why this priority**: this is the primary trust gap exposed after Spec 358 removed contradictory generic queue wording. + +**Independent Test**: seed a queued or running review-compose run plus a same-scope ready review and verify that the adapter completes the run as succeeded with audit-visible reconciliation metadata. + +**Acceptance Scenarios**: + +1. **Given** a queued or running `environment.review.compose` run and a same-workspace, same-environment, same-fingerprint ready review, **When** adapter reconciliation runs, **Then** the run becomes `completed/succeeded` with related review metadata and no new review is created. +2. **Given** a matching published review already exists for the same scope, **When** the adapter evaluates the run, **Then** the run may complete from that publishable truth without reopening or mutating the published review. + +--- + +### User Story 2 - Recover duplicate fingerprint and superseded review lineages deterministically (Priority: P1) + +As an operator, I need duplicate fingerprint collisions and late jobs against older review rows to end in a deterministic succeeded/blocked/attention outcome instead of an endless active run. + +**Why this priority**: duplicate fingerprint recovery is the observed runtime failure that creates ghost runs and unsafe manual interpretation. + +**Independent Test**: simulate duplicate-key and superseded-successor cases in PGSQL-aware tests and verify that recoverable cases complete from matching review truth while ambiguous or incomplete cases fail closed. + +**Acceptance Scenarios**: + +1. **Given** compose attempts to create a mutable review and the DB raises the mutable fingerprint unique constraint while a matching ready successor review already exists, **When** recovery runs, **Then** the `OperationRun` becomes `completed/succeeded` from adapter truth. +2. **Given** a late job points at an older review while a matching successor review is still draft/failed/in-progress, **When** the adapter evaluates the lineage, **Then** the run becomes blocked or attention-required rather than falsely succeeded. + +--- + +### User Story 3 - Keep review-start dispatch idempotent for repeated triggers (Priority: P2) + +As an operator, I need repeated create/refresh clicks for the same review fingerprint to reuse the correct review or active run instead of creating competing mutable reviews and duplicate run noise. + +**Why this priority**: repeat-trigger safety is the operator-facing entry point for avoiding the runtime collision in the first place. + +**Independent Test**: trigger the same review composition rapidly and verify that only one active mutable review truth survives for the fingerprint while later requests reuse existing truth or surface the correct active run. + +**Acceptance Scenarios**: + +1. **Given** a matching mutable review already exists for the fingerprint, **When** create/refresh is triggered again, **Then** the existing review is reused and the current already-available feedback remains truthful. +2. **Given** an active compose run already exists for the fingerprint, **When** create/refresh is triggered again, **Then** the operator is directed to the existing run or review instead of silently creating another conflicting path. + +--- + +### User Story 4 - Show calm review-compose reconciliation language on existing operations surfaces (Priority: P3) + +As an operator, I need existing monitoring and review feedback surfaces to explain adapter-based review reconciliation without surfacing raw DB or constraint text. + +**Why this priority**: the underlying correctness work still needs one calm visible explanation path. + +**Independent Test**: run the bounded browser smoke and confirm that reconciled review-compose runs show review-available or attention wording instead of generic wait-state copy or raw SQL errors. + +**Acceptance Scenarios**: + +1. **Given** a review-compose run was completed from adapter truth, **When** the operations hub or run detail renders it, **Then** the visible explanation states that a matching review/result was already available. +2. **Given** a duplicate or ambiguous compose case cannot be auto-completed safely, **When** the visible surfaces render it, **Then** the copy asks for attention without showing `SQLSTATE`, `duplicate key`, or the constraint name. + +## Edge Cases + +- Missing `workspace_id`, `managed_environment_id`, `review_fingerprint`, or snapshot context on the run. +- A matching fingerprint exists in another workspace or another managed environment. +- Multiple mutable reviews match the same scope and fingerprint. +- A successor review exists through `superseded_by_review_id` but is not yet ready/published. +- The run is already terminal and already carries reconciliation metadata. +- Duplicate-key recovery happens after the job already marked the run `running`. +- A review is published and a later late job tries to recompose the superseded lineage. +- The existing restore adapter path remains present and must not be broken by the local review-compose seam extension. + +## Requirements *(mandatory)* + +- **FR-359-001**: The repo must add one bounded review-compose business-truth reconciliation path by extending the existing adapter reconciliation seam without changing the persisted `OperationRun` status/outcome schema. +- **FR-359-002**: The seam must recognize supported types explicitly and treat unsupported types safely as unsupported/no-op without introducing a generic registry/resolver layer. +- **FR-359-003**: A serializable reconciliation result object must distinguish at least `reconciled_succeeded`, `not_reconciled`, `blocked`, `attention_required`, `failed_unrecoverable`, and `unsupported`. +- **FR-359-004**: Reconciliation results must carry reason code, message key or copy mapping hint, evidence payload, related model metadata, and an explicit `safeForAutoCompletion` signal. +- **FR-359-005**: `OperationRunService` must remain the only owner of terminal run reconciliation writes. Adapter-driven completion may extend the existing reconciliation seam or wrap it, but must not bypass service-owned lifecycle updates. +- **FR-359-006**: Reconciliation metadata must extend the current `context.reconciliation` structure with source, adapter, decision, reason code, previous status/outcome, timestamp, and related review metadata sufficient for shared artifact/link readers to resolve the canonical review even when `operation_run_id` still points at an older or losing review row, plus bounded evidence. +- **FR-359-007**: No new `operation_runs` status column, `reconciled` boolean, or new `environment_reviews` state family may be introduced. +- **FR-359-008**: A review-compose adapter must support only `environment.review.compose` and must prove same-scope review truth from existing `EnvironmentReview` records. +- **FR-359-009**: Same-scope review truth must validate workspace, managed environment, fingerprint, and any explicit review lineage context before reuse. +- **FR-359-010**: A same-scope ready or published review may reconcile the run to `completed/succeeded` when the adapter can prove the review is usable and unambiguous. +- **FR-359-011**: Missing scope, cross-scope candidates, ambiguous matches, non-ready successors, or unknown review state must fail closed as `not_reconciled`, `blocked`, or `attention_required`, never as auto-success. +- **FR-359-012**: Superseded or successor review cases must be handled deterministically. A ready successor may prove success; an in-progress or failed successor may only block or require attention. +- **FR-359-013**: `ComposeEnvironmentReviewJob` must use the adapter seam before or after duplicate-key recovery so recoverable duplicate fingerprint cases do not leave queued/running ghosts. +- **FR-359-014**: `EnvironmentReviewService` must remain idempotent for repeated create/refresh triggers and must consider existing mutable review truth plus existing active compose runs before dispatching new work. +- **FR-359-015**: Existing restore-specific adapter reconciliation may be preserved or minimally bridged, but Spec 359 must not expand restore, report, evidence, sync, backup, or alert business reconciliation behavior. +- **FR-359-016**: Operator-facing copy for adapter-reconciled review-compose runs must stay calm and audit-friendly and must not surface raw SQL, duplicate-key, constraint-name, or worker-crash wording. +- **FR-359-017**: Localization must extend existing EN/DE review/operations copy families rather than inventing a new translation namespace unless repo truth clearly prefers one. +- **FR-359-018**: The adapter must not mutate unrelated domain records, delete rows, or perform destructive cleanup. + +## Non-Functional Requirements + +- **NFR-359-001 Auditability**: every adapter-driven reconciliation must be visible in `context.reconciliation` and in the terminal audit path. +- **NFR-359-002 Fail closed**: ambiguous or missing proof must never auto-complete as succeeded. +- **NFR-359-003 Idempotence**: repeating adapter evaluation on the same run must not duplicate context spirals or create new domain side effects. +- **NFR-359-004 Minimal invasiveness**: reuse current `OperationRunService`, current review-compose service/job seams, and current monitoring presenters wherever possible. +- **NFR-359-005 Livewire v4 posture**: no Livewire v3 APIs or panel-provider drift. +- **NFR-359-006 No asset/panel change**: no new Filament assets, no provider registration move, no new panel path. + +## Out Of Scope + +- Report, evidence snapshot, review-pack, sync, backup, restore, or alert-delivery adapter expansion +- New queue table/job family/schema redesign +- New `OperationRun` status/outcome persistence shape +- Destructive cleanup, delete, purge, or hide flows +- Customer-facing review portal/productization changes +- New PDF or report rendering architecture +- New provider or Graph abstraction work + +## Acceptance Criteria + +- **AC-359-001**: Existing adapter reconciliation supports both `restore.execute` and `environment.review.compose` without adding a new generic registry layer. +- **AC-359-002**: `environment.review.compose` resolves to a dedicated adapter and unknown types remain unsupported safely. +- **AC-359-003**: A same-scope ready or published review can reconcile a queued/running review-compose run to `completed/succeeded`. +- **AC-359-004**: Cross-workspace, cross-environment, or wrong-fingerprint reviews are never reused. +- **AC-359-005**: Duplicate fingerprint collisions end as recovered success, blocked/attention, or failed-unrecoverable, but never as queued/running forever. +- **AC-359-006**: Superseded review lineages are handled deterministically and fail closed when successor truth is incomplete or ambiguous. +- **AC-359-007**: `context.reconciliation` records source, adapter, decision, reason code, previous status/outcome, timestamp, related review metadata, and bounded evidence sufficient for shared review link resolution. +- **AC-359-008**: Existing restore-specific reconciliation behavior is not broken by the new contract shape. +- **AC-359-009**: No new persistence shape or status column is introduced. +- **AC-359-010**: Existing operations/review surfaces explain adapter-reconciled review-compose outcomes without raw SQL/constraint wording and can resolve the winning review from reconciliation metadata when needed. +- **AC-359-011**: Repeated adapter evaluation and repeated review-start triggers remain idempotent. + +## Success Criteria + +- Operators no longer need to infer review truth manually from a stale review-compose run plus a separate ready review row. +- Duplicate fingerprint cases are either safely recovered or explicitly blocked/failed with audit-visible metadata. +- Review-compose reconciliation remains bounded to current repo truth and does not silently widen into universal business completion. + +## Risks + +- The existing restore adapter precedent could tempt a too-broad framework refactor. +- Review lineage rules can overfit mutable vs superseded semantics if the adapter tries to solve report/evidence truth too early. +- PGSQL-only duplicate/locking behavior could be under-tested if the implementation relies only on default fast-feedback coverage. + +## Assumptions + +- `EnvironmentReviewStatus::Ready` and `Published` are the only review states that may prove immediately usable review truth for this slice. +- Existing `workspace_id`, `managed_environment_id`, `review_fingerprint`, and review lineage metadata are sufficient to prove scope-safe matching without a schema change. +- The current `EnvironmentReviewResource` create/refresh notification pattern is the correct UX seam for already-available versus queued background work. + +## Open Questions + +- Non-blocking implementation choice only: keep the current restore-specific reconciler as-is or bridge it behind the new contract, provided no restore business behavior changes and no second operator language is introduced. diff --git a/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/tasks.md b/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/tasks.md new file mode 100644 index 00000000..c7ddfb8a --- /dev/null +++ b/specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/tasks.md @@ -0,0 +1,209 @@ +# Tasks: OperationRun Reconciliation Adapter Framework & Review Compose Adapter + +**Input**: `specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/spec.md`, `plan.md`, and `checklists/requirements.md` +**Prerequisites**: `spec.md` and `plan.md` +**Tests**: REQUIRED (Pest). Keep proof bounded to Unit + Feature + PGSQL + one explicit Browser smoke. +**Operations**: Reuse current `OperationRun` lifecycle ownership. No new run status column, no new queue family, no new schema, and no destructive cleanup. +**RBAC**: Reuse current workspace-first run access plus `Capabilities::ENVIRONMENT_REVIEW_MANAGE` for review initiation. Presentation must never reveal cross-workspace or cross-environment review truth. +**Shared Pattern Reuse**: Reuse `OperationRunService`, `OperationRun::reconciliation()`, current monitoring/detail surfaces, current review-create feedback, and the current restore adapter precedent instead of building a parallel framework. +**Filament / Panel Guardrails**: Filament remains v5 on Livewire v4. Provider registration stays in `apps/platform/bootstrap/providers.php`. No new panel, route family, or asset strategy is allowed. +**Organization**: Tasks are grouped by user story so the new contract, duplicate recovery, review-start idempotency, and visible wording each remain independently reviewable. + +## Repo Baseline At Prep Time + +- **Branch**: `359-operationrun-reconciliation-adapter-framework-review-compose-adapter` +- **HEAD**: `2a12729d feat: implement operation run queue truth foundation (spec 358) (#429)` +- **`git status --short --branch`**: clean before Spec Kit branch creation; this prep adds only `specs/359-operationrun-reconciliation-adapter-framework-review-compose-adapter/` +- **Relevant runtime surfaces**: + - `apps/platform/app/Services/OperationRunService.php` + - `apps/platform/app/Services/Operations/OperationLifecycleReconciler.php` + - `apps/platform/app/Services/AdapterRunReconciler.php` + - `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewService.php` + - `apps/platform/app/Jobs/ComposeEnvironmentReviewJob.php` + - `apps/platform/app/Models/EnvironmentReview.php` + - `apps/platform/app/Filament/Pages/Monitoring/Operations.php` + - `apps/platform/app/Filament/Pages/Operations/TenantlessOperationRunViewer.php` + - `apps/platform/app/Filament/Resources/OperationRunResource.php` + - `apps/platform/app/Filament/Resources/EnvironmentReviewResource.php` +- **Spec 358 baseline status**: completed foundation context only; use it as the generic queue-truth baseline and do not rewrite its historical close-out or validation language. +- **Related completed context**: Spec 357 remains regression context for report/review output; Spec 311 remains completed shell/scope foundation context only. + +## Test Governance Checklist + +- [ ] Lane assignment is named and is the narrowest sufficient proof for the changed behavior. +- [ ] New or changed tests stay in the smallest honest family, and the PGSQL/browser additions remain explicit. +- [ ] Shared helpers, factories, seeds, fixtures, and context defaults stay cheap by default; any widening is isolated or documented. +- [ ] Planned validation commands cover the change without widening into unrelated lane cost. +- [ ] The declared monitoring/detail surface profile is explicit. +- [ ] Any material budget, baseline, trend, or escalation note is recorded in the active feature close-out. + +## Phase 1: Setup (Repo Truth Inventory) + +**Purpose**: confirm the existing reconciliation, review-compose, and visible UX seams before runtime edits begin. + +- [x] T001 Re-read `spec.md`, `plan.md`, `checklists/requirements.md`, `.specify/memory/constitution.md`, `docs/ai-coding-rules.md`, `docs/architecture-guidelines.md`, `docs/testing-guidelines.md`, `docs/security-guidelines.md`, `docs/filament-guidelines.md`, and `specs/358-operationrun-queue-truth-foundation/{spec,plan,tasks}.md` together before touching runtime code. +- [x] T002 [P] Confirm the current reconciliation seams in `apps/platform/app/Services/OperationRunService.php`, `apps/platform/app/Services/Operations/OperationLifecycleReconciler.php`, `apps/platform/app/Services/AdapterRunReconciler.php`, and `apps/platform/app/Models/OperationRun.php`. +- [x] T003 [P] Confirm the current review-compose seams in `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewService.php`, `apps/platform/app/Jobs/ComposeEnvironmentReviewJob.php`, `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewComposer.php`, `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewFingerprint.php`, `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewLifecycleService.php`, and `apps/platform/app/Models/EnvironmentReview.php`. +- [x] T004 [P] Confirm the current visible proof owners in `apps/platform/app/Filament/Pages/Monitoring/Operations.php`, `apps/platform/app/Filament/Pages/Operations/TenantlessOperationRunViewer.php`, `apps/platform/app/Filament/Resources/OperationRunResource.php`, `apps/platform/app/Filament/Resources/EnvironmentReviewResource.php`, `apps/platform/app/Filament/Widgets/Dashboard/RecentOperations.php`, `apps/platform/app/Filament/System/Pages/Ops/Runs.php`, `apps/platform/app/Support/EnvironmentDashboard/EnvironmentDashboardSummaryBuilder.php`, and the current EN/DE localization families. +- [x] T005 Confirm that no new schema, no new panel/provider path, no new asset registration, and no new provider-boundary work are required. + +--- + +## Phase 2: Foundational (Blocking Reconciliation Contract) + +**Purpose**: settle one narrow business-truth reconciliation seam before touching individual review/job surfaces. + +**Critical**: no user-story runtime work should begin until this phase is complete. + +- [x] T006 [P] Add failing Unit coverage for the result/decision model in `apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359ReconciliationResultTest.php`. +- [x] T007 [P] Add failing Unit coverage for `apps/platform/app/Services/AdapterRunReconciler.php` supported-type behavior in `apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359AdapterRunReconcilerSupportedTypesTest.php`, including `restore.execute`, `environment.review.compose`, unsupported-type handling, and current restore-path preservation expectations. +- [x] T008 [P] Add failing Unit coverage for same-scope proof and fail-closed branches in `apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359EnvironmentReviewComposeAdapterTest.php`. +- [x] T009 Extend `apps/platform/app/Services/AdapterRunReconciler.php` with one local `environment.review.compose` path plus a thin decision/result helper, without introducing a new registry/resolver namespace or a universal business reconciliation engine. +- [x] T010 Extend or wrap `apps/platform/app/Services/OperationRunService.php` so adapter-driven reconciliation writes merge richer `context.reconciliation` metadata idempotently and preserve current terminal audit behavior. +- [x] T011 Keep `apps/platform/app/Services/AdapterRunReconciler.php` behavior intact or bridge it minimally behind the new contract without changing restore business behavior. + +**Checkpoint**: one shared adapter contract exists, richer reconciliation metadata is service-owned, and restore-specific behavior still works. + +--- + +## Phase 3: User Story 1 - Reuse proven review truth for a stale review-compose run (Priority: P1) + +**Goal**: a same-scope ready or published review can safely complete a queued/running `environment.review.compose` run as succeeded. + +**Independent Test**: seed a queued/running compose run plus a same-scope ready review and verify succeeded reconciliation with related review metadata. + +### Tests for User Story 1 + +- [x] T012 [P] [US1] Add `apps/platform/tests/Feature/Operations/Spec359OperationRunAdapterReconciliationTest.php` for `completed/succeeded` reconciliation from existing ready/published review truth. +- [x] T013 [P] [US1] Extend monitoring/detail presentation coverage in `apps/platform/tests/Feature/Monitoring/OperationLifecycleFreshnessPresentationTest.php` or a new Spec 359-focused feature file so the reconciled review-compose explanation is visible without generic wait-state copy. +- [x] T013A [P] [US1] Extend shared-consumer feature coverage for review-compose explanation on dashboard/system/summary surfaces if shared presenter wording changes remain in scope. + +### Implementation for User Story 1 + +- [x] T014 [US1] Implement the `environment.review.compose` adapter so it proves same-workspace, same-environment, same-fingerprint review truth and returns `reconciled_succeeded` only for safe ready/published review states. +- [x] T015 [US1] Extend `apps/platform/app/Services/OperationRunService.php` so adapter success records previous status/outcome, related model metadata, and bounded evidence in `context.reconciliation`. +- [x] T016 [US1] Update the current operations/detail presentation seams in `apps/platform/app/Support/OpsUx/OperationUxPresenter.php`, `apps/platform/app/Filament/Pages/Operations/TenantlessOperationRunViewer.php`, and `apps/platform/app/Filament/Resources/OperationRunResource.php` only as needed so review-compose reconciliation reads calmly and consistently. +- [x] T016A [US1] Update `apps/platform/app/Support/Ui/GovernanceArtifactTruth/ArtifactTruthPresenter.php` and `apps/platform/app/Support/OperationRunLinks.php` to prefer reconciliation-related review metadata before falling back to `operation_run_id` lookups. + +**Checkpoint**: User Story 1 is independently functional when same-scope review truth safely completes the run and visible monitoring surfaces explain it. + +--- + +## Phase 4: User Story 2 - Recover duplicate fingerprint and superseded review lineages deterministically (Priority: P1) + +**Goal**: duplicate collisions and late jobs end as succeeded/blocked/attention deterministically, never as endless active runs. + +**Independent Test**: simulate duplicate-key and superseded-successor cases in PGSQL-aware coverage and verify deterministic outcomes. + +### Tests for User Story 2 + +- [x] T017 [P] [US2] Add duplicate recovery and unrecoverable-duplicate cases to `apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeReconciliationTest.php`. +- [ ] T018 [P] [US2] Add PGSQL duplicate-index / locking proof to `apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeIdempotencyTest.php` and run it in the `phpunit.pgsql.xml` lane. +- [x] T019 [P] [US2] Add ambiguous/superseded successor cases to `apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359EnvironmentReviewComposeAdapterTest.php`. + +### Implementation for User Story 2 + +- [x] T020 [US2] Integrate the adapter seam into `apps/platform/app/Jobs/ComposeEnvironmentReviewJob.php` for pre-proof checks, duplicate-key recovery, and clean blocked/attention/failed fallback behavior. +- [x] T021 [US2] Keep `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewComposer.php` pure composition only; duplicate recovery and review reuse must stay outside the composer. +- [x] T022 [US2] Ensure the final decision evidence includes considered review IDs, chosen related review ID, fingerprint, workspace ID, managed environment ID, and safe status lineage only. + +**Checkpoint**: User Story 2 is independently functional when recoverable duplicates succeed from review truth and unrecoverable cases stop cleanly in terminal follow-up. + +--- + +## Phase 5: User Story 3 - Keep review-start dispatch idempotent for repeated triggers (Priority: P2) + +**Goal**: repeated create/refresh triggers reuse the correct mutable review or active run instead of multiplying conflicting state. + +**Independent Test**: trigger the same review composition repeatedly and verify reuse of the active review/run path. + +### Tests for User Story 3 + +- [x] T023 [P] [US3] Add repeated-trigger coverage to `apps/platform/tests/Feature/EnvironmentReview/Spec359ReviewComposeIdempotencyTest.php` for existing mutable review reuse and existing active run reuse. +- [ ] T024 [P] [US3] Extend `apps/platform/tests/Feature/EnvironmentReview/EnvironmentReviewOperationsUxTest.php` or add a Spec 359 UX companion file so review-create feedback remains truthful for already-available and open-operation cases. + +### Implementation for User Story 3 + +- [x] T025 [US3] Tighten `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewService.php` so `create()` / `refresh()` consider existing mutable review truth plus active compose runs before dispatching another job. +- [ ] T026 [US3] Keep `apps/platform/app/Filament/Resources/EnvironmentReviewResource.php` on the current notification/action family and extend it only as needed to surface reused review/run truth. + +**Checkpoint**: User Story 3 is independently functional when duplicate clicks reuse truth and do not create competing mutable reviews or ghost runs. + +--- + +## Phase 6: User Story 4 - Show calm review-compose reconciliation language on existing operations surfaces (Priority: P3) + +**Goal**: existing monitoring and review feedback surfaces explain adapter-based review truth without raw SQL/constraint wording. + +**Independent Test**: run one bounded browser smoke and confirm calm review-available or attention wording. + +### Tests for User Story 4 + +- [x] T027 [P] [US4] Add `apps/platform/tests/Browser/Spec359ReviewComposeReconciliationSmokeTest.php` covering reconciled-success, attention-required, and duplicate-recovered visible states. +- [ ] T028 [P] [US4] Extend or add focused feature assertions for EN/DE localization keys if the visible copy is mapped through current localization helpers. + +### Implementation for User Story 4 + +- [ ] T029 [US4] Add or extend bounded EN/DE localization keys in `apps/platform/lang/en/localization.php` and `apps/platform/lang/de/localization.php` for review-compose reconciliation outcomes, reusing the current review/operations families where possible. +- [x] T030 [US4] Ensure visible copy on monitoring/detail/review-start surfaces never exposes `SQLSTATE`, `duplicate key`, `environment_reviews_fingerprint_mutable_unique`, or crash/orphan claims as primary operator messaging. + +**Checkpoint**: User Story 4 is independently functional when operations and review-start surfaces show calm adapter-backed wording only. + +--- + +## Phase 7: Polish & Validation + +- [ ] T031 [P] Refresh `spec.md`, `plan.md`, and `checklists/requirements.md` only if implementation proves a thinner or broader touched-file boundary. +- [ ] T032 [P] Run `cd apps/platform && ./vendor/bin/sail artisan test --compact --filter=Spec359`. +- [ ] T033 [P] Run `cd apps/platform && ./vendor/bin/sail php vendor/bin/pest -c phpunit.pgsql.xml --filter=Spec359`. +- [ ] T034 [P] Run `cd apps/platform && ./vendor/bin/sail artisan test --compact --filter=Spec358`. +- [ ] T035 [P] Run the named review/report regressions from the active spec: + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Browser/CustomerReviewWorkspaceSmokeTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Browser/Spec357ReportProfilesSmokeTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/Spec357RenderedReportProfileTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/ReviewPackDownloadTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/EnvironmentReviewDerivedReviewPackTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/EnvironmentReviewExecutivePackTest.php` +- [ ] T036 [P] Run `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent`. +- [x] T037 [P] Run `git diff --check`. +- [ ] T038 [P] Record the final adapter contract shape, restore-precedent handling decision, duplicate-recovery proof boundaries, and visible wording decisions in the active feature close-out entry `Guardrail / Smoke Coverage`. + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Setup (Phase 1)**: no dependencies +- **Foundational (Phase 2)**: depends on Setup and blocks all story work +- **US1 (Phase 3)**: depends on Foundational completion +- **US2 (Phase 4)**: depends on Foundational completion and is easiest after US1 settles the adapter contract +- **US3 (Phase 5)**: depends on US1 and US2 because repeated-trigger reuse should point at the same proven truth path +- **US4 (Phase 6)**: depends on US1 and US2 because visible copy needs the final decision vocabulary +- **Polish (Phase 7)**: depends on all desired user stories + +### Parallel Opportunities + +- `T002`, `T003`, and `T004` can run in parallel. +- `T006`, `T007`, and `T008` can run in parallel. +- `T012` and `T013` can run in parallel. +- `T017`, `T018`, and `T019` can run in parallel. +- `T023` and `T024` can run in parallel. +- `T027` and `T028` can run in parallel. +- `T032` through `T037` can run in parallel after implementation stabilizes. + +### Implementation Strategy + +1. Freeze the narrow contract and service-owned metadata path first. +2. Ship ready-review reuse so succeeded reconciliation has one authoritative shape. +3. Ship duplicate/superseded recovery on top of that contract. +4. Tighten repeated review-start dispatch so the entry path reuses the same truth. +5. Finish with calm visible copy, focused regressions, and explicit close-out notes. + +## Non-Goals / Must-Not-Do + +- [ ] NT001 Do not add a new `reconciled` status column, boolean, or `OperationRun` state family. +- [ ] NT002 Do not expand report/evidence/review-pack/restore/sync/backup/alert business reconciliation in this feature. +- [ ] NT003 Do not add a new queue/job family, a second operator-center UI, or a generic provider framework. +- [ ] NT004 Do not mutate unrelated `EnvironmentReview` records, delete records, or perform cleanup/purge work. +- [ ] NT005 Do not expose raw SQL/constraint/duplicate-key wording on operator-primary or customer-facing surfaces. -- 2.45.2