TenantAtlas/specs/362-sync-capture-backup-operation-semantics/artifacts/action-context-root-cause-audit.md
ahmido 548a37c888 feat: implement sync capture backup operation semantics (#433)
Implemented sync capture backup operation semantics as requested.

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #433
2026-06-07 01:19:08 +00:00

370 lines
38 KiB
Markdown

# Livewire Action Context Contract Root-Cause Audit
Date: 2026-06-07
Branch: `362-sync-capture-backup-operation-semantics`
Scope: read-only root-cause audit plus this report artifact. No runtime/source/test files were changed by this audit.
## Executive Summary
- Final classification: **Case C - Systemic Action Context Contract Gap** (`derived`, confidence: high).
- Readiness impact: workspace/environment-scoped header and page actions remain fragile unless they pass explicit action context and have first-click modal regression tests (`repo-verified`, `test-covered` for the fixed inventory/policy actions only).
- Sellability impact: this can surface as "button does nothing", "modal does not open", or "action is disabled for an authorized operator" in admin-safe workflows, which weakens trust in backup, sync, evidence, review, and restore surfaces (`derived`).
- Product risk: the highest risk is not unauthorized execution; the current failure mode usually fails closed. The risk is silent UX failure, wrong disabled state, inconsistent render-vs-submit context, and repeated implementation drift (`repo-verified`, `derived`).
- Recommended next move: create a dedicated spec for an explicit Livewire action-context contract before adding new modal-first workspace/environment actions (`derived`).
Selected case:
| Case | Result | Evidence |
| --- | --- | --- |
| Case A - Local bug only | Rejected | Same fix pattern appears in `InventoryItemResource` and `PolicyResource`, and similar no-record actions still exist (`repo-verified`). |
| Case B - Repeated implementation drift | Partially true | Several pages resolve tenant/environment context independently (`repo-verified`). |
| Case C - Systemic Action Context Contract Gap | **Selected** | `UiEnforcement` still falls back to `Filament::getTenant()` for no-record actions, while workspace-scoped Livewire requests need canonical page/referer context (`repo-verified`, `test-covered`). |
| Case D - Fundamental architectural mismatch | Not selected | The repo already has canonical workspace/environment resolvers and `WorkspaceUiEnforcement`; the issue is incomplete enforcement of that contract, not total architectural inversion (`repo-verified`, `derived`). |
## Repo State
Commands executed:
```bash
git status --short --branch
git diff --name-only
git diff --stat
git diff --cached --name-only
```
Current branch: `362-sync-capture-backup-operation-semantics` (`repo-verified`).
Staged files: none (`repo-verified`).
Tracked dirty files before report creation (`repo-verified`):
- `apps/platform/app/Filament/Concerns/ResolvesPanelTenantContext.php`
- `apps/platform/app/Filament/Resources/InventoryItemResource/Pages/ListInventoryItems.php`
- `apps/platform/app/Filament/Resources/PolicyResource.php`
- `apps/platform/app/Filament/Resources/PolicyResource/Pages/ListPolicies.php`
- `apps/platform/app/Models/OperationRun.php`
- `apps/platform/app/Services/AdapterRunReconciler.php`
- `apps/platform/app/Support/Navigation/RelatedNavigationResolver.php`
- `apps/platform/app/Support/OperationRunLinks.php`
- `apps/platform/app/Support/Operations/LifecycleReconciliationReason.php`
- `apps/platform/app/Support/Operations/Reconciliation/OperationRunReconciliationRegistry.php`
- `apps/platform/app/Support/Operations/Reconciliation/RestoreExecuteReconciliationAdapter.php`
- `apps/platform/app/Support/Rbac/UiEnforcement.php`
- `apps/platform/tests/Feature/Console/ReconcileBackupScheduleOperationRunsCommandTest.php`
- `apps/platform/tests/Feature/Filament/InventoryItemResourceTest.php`
- `apps/platform/tests/Feature/PolicySyncStartSurfaceTest.php`
- `apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec359ReconciliationResultTest.php`
- `apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec360CanonicalAdapterRegistryTest.php`
- `apps/platform/tests/Unit/Support/Operations/Reconciliation/Spec361ArtifactRegistryResolutionTest.php`
Untracked files before report creation include Spec 362 code/tests and `specs/362-sync-capture-backup-operation-semantics/` (`repo-verified`).
Diff summary before report creation: 18 tracked files changed, 241 insertions, 25 deletions (`repo-verified`).
Is the local fix still open? Yes. The dirty diff includes the `UiEnforcement`, inventory action, policy action, and Livewire referer resolver changes (`repo-verified`).
Is the worktree safe for a read-only audit? Yes, with caution: runtime files are dirty, so this audit did not overwrite, format, refactor, or stage them (`derived`).
## Tests Executed
Read-only targeted test command:
```bash
cd apps/platform
./vendor/bin/sail artisan test --compact \
tests/Feature/Filament/InventoryItemResourceTest.php \
tests/Feature/PolicySyncStartSurfaceTest.php \
tests/Unit/Filament/ResolvesPanelTenantContextLivewireRefererTest.php \
tests/Feature/RunStartAuthorizationTest.php
```
Result: **18 passed, 121 assertions** (`test-covered`).
Browser verification was not executed in this audit (`not verified`). The request described prior browser verification for `Run Inventory Sync`; this report treats that as prior context, not as audit-executed evidence.
## Trigger Case
What broke:
- A no-record header/page action such as `run_inventory_sync` or `sync` rendered in a workspace-scoped admin page, but the first Livewire action click/mount evaluated action visibility/disabled state without stable environment context (`repo-verified`, `derived`).
- The result was a modal that did not open, a wrong disabled/visibility state, or an action state mismatch between initial render and Livewire mounted action request (`derived`).
Why the local fix worked:
- `UiEnforcement::forAction()` now accepts `Model|Closure|null $record` and stores it as an explicit context source (`apps/platform/app/Support/Rbac/UiEnforcement.php:65`) (`repo-verified`).
- `ListInventoryItems` passes `fn (): ?ManagedEnvironment => static::resolveTenantContextForCurrentPanel()` into `UiEnforcement` (`apps/platform/app/Filament/Resources/InventoryItemResource/Pages/ListInventoryItems.php:69`, `:245`) (`repo-verified`).
- `PolicyResource::makeSyncAction()` passes the same explicit resolver (`apps/platform/app/Filament/Resources/PolicyResource.php:122`, `:182`) (`repo-verified`).
- `ResolvesPanelTenantContext` now infers the admin panel from Livewire update requests using the `referer` path when route/current panel context is absent (`apps/platform/app/Filament/Concerns/ResolvesPanelTenantContext.php:56-117`) (`repo-verified`, `test-covered`).
Which context was missing:
- Product environment context, represented by `ManagedEnvironment`, was missing during the mounted action Livewire request (`derived`).
- `Filament::getTenant()` was not a stable truth source in that request shape because `/livewire/update` is not the original `/admin/...` page route (`repo-verified`, `derived`).
Which previous assumption was wrong:
- `UiEnforcement` assumed a no-record header/page action could safely fall back to `Filament::getTenant()` (`apps/platform/app/Support/Rbac/UiEnforcement.php:641-694`) (`repo-verified`).
- In TenantPilot, workspace-first admin surfaces can represent environment context through page shell, workspace session, URL/referer, or record scope rather than Filament tenancy alone (`repo-verified`, `derived`).
Tests that cover the local fix:
- Inventory first-click modal mount with no Filament tenant and Livewire referer: `apps/platform/tests/Feature/Filament/InventoryItemResourceTest.php:93-117` (`test-covered`).
- Policy sync first-click modal mount with no Filament tenant and Livewire referer: `apps/platform/tests/Feature/PolicySyncStartSurfaceTest.php:93-116` (`test-covered`).
- Resolver unit test for Livewire referer panel/environment reconstruction: `apps/platform/tests/Unit/Filament/ResolvesPanelTenantContextLivewireRefererTest.php:15-55` (`test-covered`).
Tests that do not cover the broader class:
- No central test enumerates all workspace/environment-scoped no-record header actions and asserts first-click modal mount (`repo-verified`).
- No static guard fails new `UiEnforcement::forAction(...)` no-record OperationRun actions without explicit context (`not implemented`).
- `RunStartAuthorizationTest` covers readonly and cross-tenant start prevention for representative actions, but mostly with `Filament::setTenant(...)`, not workspace-scoped Livewire referer (`apps/platform/tests/Feature/RunStartAuthorizationTest.php:18-132`) (`test-covered`, `test-file-only` for the missing referer variant).
## Context Model
| Context Axis | Primary Source | Stable Across Livewire Mounted Action? | Used By `UiEnforcement`? | Risk |
| --- | --- | --- | --- | --- |
| Workspace | `WorkspaceContext`, route/query/page context, workspace record | Usually stable if explicitly passed | No, except via `WorkspaceUiEnforcement` | Medium (`repo-verified`) |
| Managed Environment / Tenant | record, page resolver, `OperateHubShell`, `ManagedEnvironment::current()` | Stable only if explicit resolver handles Livewire referer | Yes | High for no-record header actions (`repo-verified`, `derived`) |
| Filament Tenant Context | `Filament::getTenant()` / `Filament::setTenant()` | Not stable enough for workspace-scoped Livewire update requests | Yes, final fallback | High (`repo-verified`, `test-covered`) |
| Page Context | page methods, shell context, URL/referer | Stable when resolver is designed for Livewire | Indirect only if closure passed | Medium (`repo-verified`) |
| Livewire Request Context | `/livewire/update`, `x-livewire`, `referer` | Different from initial page route | Only through custom resolver | High (`repo-verified`) |
| Mounted Action Context | Filament action record/form state | Stable for record-backed actions, weak for no-record actions | Partly via `$action->getRecord()` | Medium (`repo-verified`) |
| OperationRun Context | explicit `tenant`, `managed_environment_id`, identity inputs, context payload | Stable if created from canonical tenant | Not directly | High if UI and execution context diverge (`repo-verified`, `derived`) |
| RBAC Capability Context | `CapabilityResolver`, `WorkspaceCapabilityResolver` | Stable if scope is explicit | Yes | High if scope is implicit/missing (`repo-verified`) |
Product architecture conclusion: TenantPilot is workspace-first; environment/tenant is a product scope inside workspace. Filament tenant context can be a transport/detail, but it must not be treated as product scope truth unless explicitly justified (`derived`).
## UiEnforcement Contract Assessment
Current design (`repo-verified`):
- `UiEnforcement::forAction(Action $action, Model|Closure|null $record = null)` accepts a direct model or closure (`apps/platform/app/Support/Rbac/UiEnforcement.php:65`).
- `resolveTenantWithRecord()` resolves in this order: passed record, owned record, action record, stored closure/model, then `Filament::getTenant()` (`apps/platform/app/Support/Rbac/UiEnforcement.php:641-694`).
- Missing user or tenant returns `TenantAccessContext(user: null, tenant: null, isMember: false, hasCapability: false)` (`apps/platform/app/Support/Rbac/UiEnforcement.php:607-620`).
- That fail-closed behavior hides or disables actions depending on membership/capability application (`repo-verified`).
Observed issue (`repo-verified`, `derived`):
- For no-record header actions, the final fallback is still `Filament::getTenant()`.
- Missing context is treated the same as non-membership for UI state, so the user can see a silent hidden/disabled action instead of an explanatory "context unavailable" state.
- `UiEnforcement` is a UX gate, but several action handlers also resolve scope again, sometimes through page resolvers and sometimes from records.
Better contract (`derived`):
- `UiEnforcement` should not infer product scope for scoped no-record actions.
- New workspace/environment actions should receive an explicit `UiActionContext` or canonical page/record resolver.
- Missing context should be distinguishable from non-membership and insufficient capability for observability and UX copy.
- OperationRun-starting handlers must reauthorize at submit/execution time using the same explicit scope.
Migration effort (`derived`):
- Minimal guardrail: low/medium, by requiring explicit context on no-record `UiEnforcement::forAction` calls that start runs or mutate state.
- DTO/contract: medium, because many callsites are record-backed and should not be churned unnecessarily.
- Static CI guard: medium, because source scanning must avoid false positives for record-backed detail/table actions.
## Action Lifecycle Assessment
| Phase | What Happens | Risk |
| --- | --- | --- |
| Initial render | Filament evaluates visibility/disabled closures with page request context. | Usually correct when `/admin/...` context exists (`derived`). |
| First click / mount action | Livewire sends update request; route/path no longer proves original admin page. | Highest risk for no-record header actions (`repo-verified`, `test-covered`). |
| Modal mount | Form defaults and modal visibility are evaluated. | Hidden fields/defaults can be wrong if environment context is missing (`repo-verified`, `derived`). |
| Modal open | Should not create OperationRun or dispatch job. | Covered for inventory/policy; not centrally covered (`test-covered`, `not implemented`). |
| Modal submit / action execution | Action handler resolves tenant and creates run/job or mutates state. | Safer when handler re-resolves/validates context; risky when render and submit use different sources (`repo-verified`, `derived`). |
| OperationRun creation | `OperationRunService` receives tenant and context. | Correct only if tenant source is canonical and reauthorized (`repo-verified`, `derived`). |
## Callsite Inventory
Static search result: 83 `UiEnforcement::forAction` matches in `apps/platform/app/Filament` (`repo-verified`). The table below groups the relevant action-context risk surfaces rather than treating record-backed CRUD helpers as the same failure class.
| File / Class | Action | Type | Scope | Explicit Context | Record? | Modal? | OperationRun / External Effect | Reauth / Submit Guard | Risk | Evidence |
| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- |
| `InventoryItemResource/Pages/ListInventoryItems.php` | `run_inventory_sync` | Header | Environment | Yes, page resolver | No | Yes | Creates `inventory.sync` run/job on submit | Context mismatch guard via hidden `managed_environment_id`; readonly tests | Medium after local fix | `repo-verified`, `test-covered` |
| `PolicyResource.php` / `ListPolicies.php` | `sync` | Header | Environment | Yes, page resolver | No | Confirmation modal | Creates `policy.sync` run/job on submit | Missing context aborts 404 | Medium after local fix | `repo-verified`, `test-covered` |
| `EntraGroupResource/Pages/ListEntraGroups.php` | `sync_groups` | Header | Environment | No | No | No modal | Starts directory group sync via service | Handler re-resolves tenant and returns on missing context | High latent | `repo-verified`, `test-file-only` |
| `EvidenceSnapshotResource/Pages/ListEvidenceSnapshots.php` | `create_snapshot` | Header | Environment | No | No | Form modal | Generates evidence snapshot/run | Execution resolves tenant and returns notification on missing context | High latent | `repo-verified` |
| `EvidenceSnapshotResource.php` | `create_first_snapshot` | Empty-state action | Environment | No | No | No explicit modal, action executes | Generates evidence snapshot/run | Execution resolves tenant and returns notification on missing context | Medium/high | `repo-verified` |
| `ReviewPackResource.php` | `generate_pack` / `generate_first` | Header/empty-state action | Environment | No | No | Form modal | Generates review pack/run | Execution resolves current tenant and handles missing context | High latent | `repo-verified` |
| `EnvironmentReviewResource.php` | `create_review` | Header/empty-state action | Environment | No | No | Form modal | Creates review and compose run | Execution checks access/capability after tenant resolution | High latent | `repo-verified` |
| `EnvironmentDiagnostics.php` | `bootstrapOwner`, `mergeDuplicateMemberships` | Header | Environment | No | No | Confirmation modal | Tenant membership repair mutation | Handler resolves tenant or fails | High, sensitive admin repair | `repo-verified` |
| `EnvironmentDashboard.php` | `submit_support_request`, `openSupportDiagnostics` | Page actions | Environment | No | No | Form/slide-over modal | Support request creation, diagnostics audit | Uses page resolver methods | Medium/high | `repo-verified` |
| `BaselineCompareLanding.php` | baseline compare action | Page/header action | Environment | No explicit `UiEnforcement` context | Page state | Action/queue | Creates baseline compare run | Uses current environment/page state | Medium/high | `repo-verified`, `derived` |
| `CrossEnvironmentComparePage.php` | `generatePromotionPreflight` | Header/page action | Workspace | Yes, workspace closure | No | No modal | Preflight generation | Workspace capability resolver | Low current, high future feature class | `repo-verified` |
| `BaselineCompareMatrix.php` | `compareAssignedTenants` | Header/page action | Workspace | Yes, workspace closure | Profile/page | Confirmation modal | Starts environment-owned compare path | Workspace capability resolver | Low current | `repo-verified` |
| `CustomerReviewWorkspace.php` | `create_next_review` | Page action | Environment via review record | Record closure | Yes | Confirmation modal | Review lifecycle mutation | Handler validates user/review | Medium | `repo-verified` |
| `PolicyResource/Pages/ViewPolicy.php` | `capture_snapshot` | Detail header | Environment | No explicit closure, but action record/policy tenant available | Yes | Form confirmation | Creates capture snapshot run/job | Handler uses policy tenant and missing context notification | Medium | `repo-verified` |
| `PolicyResource/RelationManagers/VersionsRelationManager.php` | `restore_to_intune` | Relation row action | Environment | No explicit closure | Yes | Form confirmation | Starts restore via service | Owner-scoped record resolution and tenant mismatch guard | Medium/high impact | `repo-verified` |
| `BackupScheduleResource.php` | `runNow`, `retry` | Table row actions | Environment | No explicit closure | Yes via table record/action | No modal | Creates backup schedule run/job | Handler resolves tenant; readonly tests | Medium | `repo-verified`, `test-covered` |
| `RestoreRunResource.php` | create restore run | Header create action/wizard | Environment | No explicit closure | No before create | Wizard/modal-like | Restore flow and potential restore run | Wizard and write gate checks later | High impact | `repo-verified` |
| `RestoreRunResource.php` | restore/archive/forceDelete rows | Table row actions | Environment | Yes in table actions for row lifecycle actions | Yes | Confirmation modal | Mutates restore records | Scoped record resolver/audit | Medium | `repo-verified` |
| `ProviderConnectionResource.php` | check/inventory/compliance | Row/detail actions | Environment | No explicit closure | Provider connection record | No modal | Provider jobs/runs | Handler resolves tenant from record and gate service | Medium | `repo-verified` |
| `ProviderConnectionResource.php` | dedicated credential actions | Row/detail actions | Environment | No explicit closure | Provider connection record | Form/confirmation modal | Secret-bearing provider mutation | Handler resolves tenant from record | Medium/high impact | `repo-verified` |
| `ViewProviderConnection.php` | `grant_admin_consent` | Detail header URL action | Environment | No explicit closure | Detail record/page tenant | URL action | External admin consent navigation | Capability UI gate only | Medium | `repo-verified` |
| `ManagedEnvironmentResource.php` | tenant sync/verify/archive/remove/restore | Record/detail/table actions | Environment record | Record/action scope | Yes | Confirmation on sensitive actions | Sync jobs and lifecycle mutation | Uses record tenant identity | Medium | `repo-verified` |
| `FindingResource.php` | triage/assign/resolve/exception actions | Row/bulk/detail actions | Environment | Mostly record/bulk selected records | Yes | Many confirmation modals | Finding lifecycle mutations | Capability checks via `UiEnforcement` | Medium | `repo-verified` |
| `AlertDestinationResource.php`, `AlertRuleResource.php` | toggle/delete/create | Table/create actions | Workspace/environment depending model | No `UiEnforcement` hits in searched files | Records | Confirmation for delete/toggle | Alert mutation, no OperationRun | Resource/policy-dependent | Medium/unknown | `repo-verified`, `not verified` |
| `StoredReportResource/Pages/ViewStoredReport.php` | `open_current_report` | Detail header URL action | Record/report | No `UiEnforcement` | Record | No | URL only | Scoped record resolver | Low | `repo-verified` |
## Similar Patterns Found
| Pattern | Files | Evidence | Severity |
| --- | --- | --- | --- |
| No-record environment header action depends on `UiEnforcement` fallback | `ListEntraGroups`, `ListEvidenceSnapshots`, `ReviewPackResource::generatePackAction`, `EnvironmentReviewResource::makeCreateReviewAction`, `EnvironmentDiagnostics` | Actions call `UiEnforcement::forAction(...)` without explicit context closure and later resolve tenant in execution | High (`repo-verified`, `derived`) |
| Local resolver fixes repeated per surface | Inventory and policy sync now pass identical page resolver closures | Same shape in two separate implementations | High (`repo-verified`) |
| Modal open versus submit not centrally guarded | Inventory/policy have direct tests; evidence/review/pack/diagnostics do not show equivalent first-click matrix | Existing coverage is per-resource | Medium/high (`repo-verified`, `not implemented`) |
| Fail-closed missing context is silent | `TenantAccessContext` collapses missing tenant into non-member false state | No distinct missing-context UI state in `UiEnforcement` | Medium (`repo-verified`, `derived`) |
| Workspace enforcement has stricter explicit context pattern | `WorkspaceUiEnforcement` only resolves direct/closure workspace, no global fallback | Better model for new context contract | Medium positive evidence (`repo-verified`) |
## Blast Radius
| Area | Relevant Actions | Potentially Affected | Evidence | Missing Tests | Risk |
| --- | --- | --- | --- | --- | --- |
| Inventory | `run_inventory_sync` | Fixed locally, recurrence risk remains | Explicit resolver now passed | Central all-actions guard absent | Medium |
| Policies | `sync`, `capture_snapshot`, row/bulk sync/export | Header fixed locally; record actions lower risk | Header explicit resolver; detail uses record | Central first-click guard absent | Medium |
| Provider Connections | check, inventory sync, compliance, credential actions | Latent lower risk due records; high impact | Record-backed actions resolve provider tenant | Workspace-referer first-click modal not broad | Medium |
| Baseline Profiles | compare assigned tenants | Currently better guarded | `WorkspaceUiEnforcement` explicit workspace closure | Future environment action guard absent | Low/medium |
| Baseline Compare | landing compare, cross-environment preflight | Future/present risk in page-state actions | Mixed explicit workspace and page-state environment context | No central mounted action matrix | Medium/high |
| Drift Findings | finding lifecycle actions | Mostly record/bulk action class, not trigger shape | `FindingResource` uses row/bulk actions | Exhaustive first-click modal coverage not verified | Medium |
| Restore Runs | create/execute/rerun/archive/delete | High impact; not same current trigger everywhere | Create action no explicit context; row actions have scoped resolvers | Modal-first context matrix not central | High |
| Tenant Reviews | create, refresh, publish, archive, create-next | Latent risk for no-record create; record detail lower | Resource/page execution resolvers | First-click no-run tests not central | High |
| Evidence Snapshots | create, create-first, refresh, expire | Latent risk for create actions | Header/empty actions no explicit resolver | First-click no-run tests missing | High |
| Review Packs | generate, regenerate, expire | Latent risk for generate actions | `generatePackAction` no explicit resolver | First-click/modal-no-run tests missing | High |
| Stored Reports | open current report | Not affected by current class | URL action, scoped record | N/A | Low |
| Support Diagnostics | open diagnostics, support request | Latent UX/audit risk | Page actions use `UiEnforcement` without explicit context | First-click modal/audit timing not central | Medium/high |
| Support Requests | submit support request | Latent modal context risk | EnvironmentDashboard form action | Missing context consistency tests not verified | Medium/high |
| Alerts / Notification Routing | alert rule/destination table actions | Not clearly same `UiEnforcement` issue | No `UiEnforcement` hits in searched files | Authorization model not fully audited here | Medium/unknown |
| Permission Posture | evidence/review/dashboard surfaces consume posture | Future risk if actions added | No direct current action found in this audit slice | Future guard required | Medium |
| Entra Admin Roles / Groups | `sync_groups`, admin roles widgets | `sync_groups` is latent current risk | Header action no explicit context; readonly test uses Filament tenant | Workspace-referer first-click test missing | High |
| Customer Health | dashboards/system directory | Future risk | OperationRun-heavy metrics, not same action trigger in inspected files | Future guard required | Medium |
| Operational Controls | restore execute write gate | Future/current high-impact execution context | Restore flow has write gate; action context still mixed | Central guard absent | High |
| Cross-Tenant Compare | promotion/preflight | Future risk but current explicit workspace example is good | `WorkspaceUiEnforcement` closures | OperationRun action context spec needed before promotion | Medium/high |
| Workspace Entitlements | review packs, support, billing-gated operations | Risk when disabled state differs render/submit | Review pack blocks use tenant/workspace entitlement decisions | Entitlement consistency matrix missing | High |
| Commercial/Billing State | entitlement-gated modals | Future risk | Review pack generation blocked by entitlement exception | Guard before new billing-gated actions | High |
## Existing Latent Risks
| Risk | Evidence | Affected Surfaces | Severity | Recommended Fix |
| --- | --- | --- | --- | --- |
| `sync_groups` repeats the no-record header action shape without explicit context | `ListEntraGroups.php:51-80` | Entra groups | Severity 2: High | Add to follow-up spec as representative third action for the new contract. |
| Evidence/review/pack generation actions can lose context on modal mount | `ListEvidenceSnapshots.php:22-37`, `ReviewPackResource.php:382-400`, `EnvironmentReviewResource.php:405-423` | Evidence snapshots, review packs, tenant reviews | Severity 2: High | Require explicit environment context on no-record generation actions. |
| Sensitive diagnostics repair actions rely on fallback context | `EnvironmentDiagnostics.php:64-86` | Environment diagnostics | Severity 2: High | Require explicit tenant resolver and missing-context copy. |
| Restore create action is high impact and context-sensitive | `RestoreRunResource.php:282-289` | Restore runs | Severity 2: High | Include restore create/wizard in guardrail tests before new restore UX work. |
| Missing context is not semantically distinct from non-membership | `UiEnforcement.php:614-620` | All `UiEnforcement` actions | Severity 3: Medium | Model missing context as its own state in contract/DTO. |
TenantPilot audit classification:
| Finding | Classification | Severity | Delivery |
| --- | --- | --- | --- |
| No-record scoped actions depend on implicit Filament tenant fallback | Architectural Drift | Severity 2: High | Dedicated spec required |
| Missing context silently collapses into non-member UI state | Workflow Trust Gap | Severity 3: Medium | Follow-up refactor under same spec |
| No central first-click modal/no-run regression guard | Test Blind Spot | Severity 2: High | Dedicated spec required |
| OperationRun-starting action context is not statically enforceable | Workflow Trust Gap | Severity 2: High | Dedicated spec required |
## Test Coverage
| Test | Scenario | First Click | Modal No-Run | Reauth / Execution Guard | Gap |
| --- | --- | --- | --- | --- | --- |
| `InventoryItemResourceTest.php:93-117` | Inventory sync mounts from workspace-scoped Livewire referer | Yes | Yes, `Queue::assertNothingPushed()` | No submit in that test | Does not cover every similar action |
| `PolicySyncStartSurfaceTest.php:93-116` | Policy sync mounts from workspace-scoped Livewire referer | Yes | Yes, `Queue::assertNothingPushed()` | No submit in that test | Does not cover every similar action |
| `ResolvesPanelTenantContextLivewireRefererTest.php:15-55` | Resolver infers tenant from Livewire referer | N/A | N/A | N/A | Resolver only, not action matrix |
| `RunStartAuthorizationTest.php:18-132` | Cross-tenant/readonly no run for inventory, Entra, backup schedule | No referer first-click | No modal matrix | Yes for representative run starts | Uses `Filament::setTenant()` |
| Provider operation tests found by search | Provider operation gates/runs | Not verified in this audit | Not verified | Partly covered by service/gate tests | First-click mounted action matrix absent |
| Evidence/review/pack tests | Operation artifact reconciliation exists | Not verified for first-click | Not verified | Some service/output tests exist | Needs action lifecycle tests |
Coverage conclusion: the local fix is test-covered. The class is not centrally guarded (`derived`).
## Future Feature Risk
| Planned Feature | Why It Is Exposed | Required Guardrail Before Build |
| --- | --- | --- |
| Customer Review Workspace v1 | Adds customer-safe workspace actions, review acknowledgement, package/evidence flows | Explicit workspace/environment action context and modal-no-run tests |
| Decision-Based Governance Inbox v1 | Likely adds modal decisions, approvals, exception lifecycle actions | No implicit Filament tenant; record/page context must be explicit |
| Localization v1 | Translated labels/tooltips can hide context/missing-state copy gaps | Missing-context state must have copy keys and tests |
| Cross-Tenant Compare and Promotion v1 | Multi-environment actions and promotion preflight can mix source/target contexts | Explicit DTO with source workspace, source environment, target environment |
| Commercial Entitlements and Billing-State Maturity | Disabled state can differ between render and submit if entitlement context is implicit | Entitlement decision must use same explicit action context |
| External Support Desk / PSA Handoff | Support handoff actions are modal-first and tenant-sensitive | First-click modal, audit-on-open, submit reauth tests |
| Private AI Execution Governance Foundation | AI execution approvals are high-trust, likely modal/OperationRun actions | Explicit operation scope, approval context, no-run-on-modal-open tests |
Most critical future features: Decision-Based Governance Inbox, Cross-Tenant Compare and Promotion, Commercial entitlement-gated actions, External Support Desk handoff, and AI execution approval actions (`derived`).
## New Feature Action Contract
| Requirement | Required? | Current Support | Gap |
| --- | --- | --- | --- |
| Declare product scope: workspace/environment/tenant/system | Yes | Informal in many pages/specs | Not enforced for actions |
| Declare context source: record/page resolver/DTO | Yes | Some explicit closures, many implicit fallbacks | Not mandatory |
| No implicit `Filament::getTenant()` for no-record scoped actions | Yes | Not currently enforced | Main gap |
| Modal open must not execute or create OperationRun | Yes | Covered for inventory/policy only | No central guard |
| Submit must reauthorize | Yes | Many handlers re-resolve/guard; not uniform | Need standard test/helper |
| Execution receives explicit workspace/environment IDs | Yes | OperationRun often receives tenant; source varies | Need context contract |
| Missing context fails closed with understandable UX | Yes | Fails closed, but often silent | Need distinct state/copy |
| Tests: render, first-click, modal-no-run, submit, wrong context, readonly | Yes | Point coverage only | Need reusable test/helper or matrix |
## Recommended Guardrail
Minimal (`recommended immediate spec scope`):
- Require explicit context closure/DTO for every no-record `UiEnforcement::forAction` that is workspace/environment-scoped.
- Cover representative actions: inventory sync, policy sync, Entra group sync, evidence snapshot create, review pack generate, environment review create, restore create.
- Add reusable Pest helper/assertion for first-click modal mount and modal-no-run.
- Add a static audit command/test that lists no-record `UiEnforcement::forAction` actions with `OperationRun`, queue dispatch, provider access, or modal forms and no explicit context.
Medium:
- Introduce `UiActionContext` with scope type, workspace, environment, source, and missing-context reason.
- Make `UiEnforcement` distinguish missing context from non-member and insufficient capability.
- Add `ResolvesActionContext` trait for page/header actions that wraps `ResolvesPanelTenantContext`, record scope, workspace scope, and Livewire referer.
Ideal:
- Make scoped action construction impossible without an explicit action context.
- Add CI guard for new OperationRun/provider/write actions without execution reauthorization and first-click lifecycle tests.
- Move action context and OperationRun start authorization into a shared contract that UI actions, jobs, and service gates can validate consistently.
Option evaluation:
| Option | Problem Solved | Problem Not Solved | Effort | Regression Risk | Recommendation |
| --- | --- | --- | --- | --- | --- |
| Keep local resolver | Fixes current action only | Recurrence on next action | Low | Low | Insufficient |
| Centralize resolver | Reduces duplication | Does not force use | Medium | Low/medium | Good |
| Introduce `UiActionContext` DTO | Models scope explicitly | Requires callsite migration | Medium | Medium | Best medium path |
| Extend `UiEnforcement` contract | Enforces guardrail near UX gate | Needs careful compatibility for record-backed actions | Medium | Medium | Recommended |
| Add trait for workspace/environment page actions | Improves ergonomics | Can become another optional helper | Low/medium | Low | Recommended with DTO |
| Add test helper only | Catches regressions | Does not improve design | Low | Low | Required but not enough |
| Static audit script/CI guard | Prevents new obvious violations | Needs false-positive tuning | Medium | Low | Recommended |
| Do nothing | Avoids churn | Recurrence likely | None | High product risk | Reject |
## Anti-Regression Rule
Proposed hard rule (`derived`):
> No new workspace-/environment-scoped `HeaderAction` or no-record page action may call `UiEnforcement` or create an `OperationRun` unless it receives an explicit action context resolved from the canonical page/record scope and has tests for render, first-click modal mount, modal-no-run, submit reauthorization, and missing-context fail-closed behavior.
Is this realistic now? Yes, if scoped to no-record header/page actions first. Applying it retroactively to every record-backed row/detail action would create too much churn and false positives (`derived`).
## Filament v5 Blueprint Notes
- Livewire compliance: app is on Livewire 4.1.4 and Filament 5.2.1 (`repo-verified` via Laravel Boost).
- Provider registration: this audit did not add or change Filament panels; Laravel 11+/12 provider registration location remains `apps/platform/bootstrap/providers.php` (`not changed`).
- Global search: this audit did not add resources or global search behavior. For any future globally searchable resource, the v5 rule remains: it needs Edit/View page or global search disabled (`not changed`).
- Destructive actions: this audit did not change actions. Existing sensitive examples inspected use `->requiresConfirmation()` on destructive/confirmation actions such as policy restore, review archive, restore archive/delete, tenant lifecycle, and provider credential changes (`repo-verified`).
- Assets: no assets added; no `filament:assets` deployment impact (`not implemented`).
- Testing plan: future spec should add Livewire tests for pages/actions and Filament action tests using `mountAction()` for modal open and `callMountedAction()`/`callAction()` for execution (`derived`, version-specific Filament guidance checked through Boost earlier in this workstream).
## Final Answer
1. Where else does this currently occur?
- Confirmed latent current risk exists in no-record environment actions that still call `UiEnforcement::forAction` without explicit context: `sync_groups`, evidence snapshot create, review pack generate, environment review create, environment diagnostics repair, restore create, and support dashboard actions (`repo-verified`, `derived`).
2. Where could it occur next?
- Any new workspace/environment-scoped header/page action that opens a modal, starts an `OperationRun`, dispatches a provider job, gates on readonly/entitlements, or uses page state instead of a record (`derived`).
3. Is this a pattern or isolated bug?
- It is a **systemic action context contract gap**, with repeated implementation drift as a symptom (`derived`, confidence high).
4. What should be mandatory for new features?
- Explicit action context, no implicit `Filament::getTenant()` for no-record scoped actions, first-click modal tests, modal-no-run tests, submit reauthorization, missing-context fail-closed tests, and explicit OperationRun scope (`derived`).
5. What is the smallest safe spec to prevent recurrence?
- Spec title: **Livewire Action Context Contract for Workspace/Environment-Scoped Actions**.
- Scope: add action context contract/trait/DTO, retrofit representative no-record actions, add reusable test helper/static guard, and document the anti-regression rule. No migrations or assets are required unless the chosen implementation adds persisted audit/telemetry for missing context (`derived`).