Some checks failed
Main Confidence / confidence (push) Failing after 44s
## Summary - enforce shared operation run link generation across admin and system surfaces - add guard coverage to block new raw operation route bypasses outside explicit exceptions - harden Filament theme asset resolution so stale or wrong-stack hot files fall back to built assets ## Testing - export PATH="/bin:/usr/bin:/usr/local/bin:$PATH" && cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent - export PATH="/bin:/usr/bin:/usr/local/bin:$PATH" && cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/OpsUx/CanonicalViewRunLinksTest.php tests/Feature/Monitoring/OperationsDashboardDrillthroughTest.php tests/Feature/Filament/RecentOperationsSummaryWidgetTest.php tests/Feature/Filament/InventoryCoverageRunContinuityTest.php tests/Feature/ReviewPack/ReviewPackResourceTest.php tests/Feature/144/CanonicalOperationViewerDeepLinkTrustTest.php tests/Feature/078/RelatedLinksOnDetailTest.php tests/Feature/RunAuthorizationTenantIsolationTest.php tests/Feature/System/Spec195/SystemDirectoryResidualSurfaceTest.php tests/Feature/System/Spec113/AuthorizationSemanticsTest.php tests/Feature/Guards/OperationRunLinkContractGuardTest.php tests/Unit/Filament/PanelThemeAssetTest.php Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de> Reviewed-on: #268
231 lines
23 KiB
Markdown
231 lines
23 KiB
Markdown
# Implementation Plan: Operation Run Link Contract Enforcement
|
|
|
|
**Branch**: `232-operation-run-link-contract` | **Date**: 2026-04-23 | **Spec**: [spec.md](./spec.md)
|
|
**Input**: Feature specification from `/specs/232-operation-run-link-contract/spec.md`
|
|
|
|
## Summary
|
|
|
|
Enforce one canonical contract for platform-owned `OperationRun` collection and detail links by migrating confirmed raw admin-plane producers to `OperationRunLinks`, preserving and regression-protecting the existing system-plane helper path through `SystemOperationRunLinks`, recording only the narrow infrastructure exceptions that cannot safely depend on the helper family, and adding one bounded regression guard that blocks new raw bypasses inside the declared UI and shared-navigation boundary.
|
|
|
|
## Technical Context
|
|
|
|
**Language/Version**: PHP 8.4.15, Laravel 12, Filament v5, Livewire v4
|
|
**Primary Dependencies**: Filament Resources/Pages/Widgets, Pest v4, `App\Support\OperationRunLinks`, `App\Support\System\SystemOperationRunLinks`, `App\Support\Navigation\CanonicalNavigationContext`, `App\Support\Navigation\RelatedNavigationResolver`, existing workspace and tenant authorization helpers
|
|
**Storage**: PostgreSQL-backed existing `operation_runs`, `tenants`, and `workspaces` records plus current session-backed canonical navigation state; no new persistence
|
|
**Testing**: Focused Pest feature tests for canonical link behavior, representative admin drill-throughs, admin and system authorization semantics, and one bounded guard test
|
|
**Validation Lanes**: `fast-feedback`, `confidence`
|
|
**Target Platform**: Laravel admin web application running in Sail Linux containers, with admin plane at `/admin` and platform plane at `/system`
|
|
**Project Type**: Monorepo with one Laravel runtime in `apps/platform` and spec artifacts at repository root
|
|
**Performance Goals**: Link generation remains helper-owned string construction with no new remote work, no new persisted navigation state, and no broadened destination queries beyond existing canonical helper semantics
|
|
**Constraints**: No new operations route family, no second link presenter stack, no compatibility shim layer, no weakening of `404` versus `403` semantics, and no repo-wide guard broadening beyond platform-owned UI and shared navigation code in this slice
|
|
**Scale/Scope**: Migrate confirmed raw admin-plane producers in widgets, pages, resources, and shared navigation; validate already-helper-backed system-plane producers; keep bootstrapping and redirect exceptions explicit and narrow
|
|
|
|
## Filament v5 Implementation Contract
|
|
|
|
- **Livewire v4.0+ compliance**: Preserved. The feature stays inside existing Filament v5 and Livewire v4 primitives and does not introduce legacy Livewire v3 patterns.
|
|
- **Provider registration location**: Unchanged. Panel providers remain registered in `bootstrap/providers.php`, not `bootstrap/app.php`.
|
|
- **Global search coverage**:
|
|
- `InventoryItemResource` remains compatible with global search expectations because it already exposes a `view` page.
|
|
- `ReviewPackResource` keeps global search disabled via `$isGloballySearchable = false` while still exposing a `view` page for direct navigation.
|
|
- The affected pages, widgets, and shared navigation helpers do not introduce or change any additional global-search surface.
|
|
- **Destructive actions**: No new destructive actions are introduced. Existing destructive or destructive-like actions on operations surfaces remain governed by their current `Action::make(...)->action(...)` definitions, and existing system run-detail actions already retain `->requiresConfirmation()`.
|
|
- **Asset strategy**: No new panel-only or shared assets are planned. Deployment expectations remain unchanged, including `cd apps/platform && php artisan filament:assets` only when registered Filament assets change.
|
|
- **Testing plan**: Prove the feature with focused feature coverage on canonical admin links, representative dashboard and shared-resolver drill-throughs, explicit admin `404`/`403` authorization preservation, the new bounded guard, and system-plane continuity plus authorization semantics. No browser or heavy-governance family is needed for this plan.
|
|
|
|
## UI / Surface Guardrail Plan
|
|
|
|
- **Guardrail scope**: Changed admin monitoring collection/detail link producers, shared related-navigation builders, helper-owned URL-query continuity, and system-plane regression protection for canonical operations links
|
|
- **Native vs custom classification summary**: Mixed shared-family change using native Filament resources/pages/widgets plus existing shared link helpers
|
|
- **Shared-family relevance**: Navigation, action links, related links, deep links, and canonical monitoring entry points
|
|
- **State layers in scope**: `page`, `detail`, and helper-owned `URL-query` continuity
|
|
- **Handling modes by drift class or surface**: Hard-stop for new raw bypasses inside the declared guard boundary; review-mandatory for explicit infrastructure exceptions
|
|
- **Repository-signal treatment**: Review-mandatory because the feature adds a bounded repo guard and a named exception list
|
|
- **Special surface test profiles**: `standard-native-filament`, `monitoring-state-page`
|
|
- **Required tests or manual smoke**: `functional-core`, `state-contract`, `manual-smoke`
|
|
- **Exception path and spread control**: One named exception boundary for bootstrapping, middleware, and redirect surfaces that cannot safely depend on runtime navigation context; each retained path must be file-specific and justified
|
|
- **Active feature PR close-out entry**: `Guardrail`
|
|
|
|
## Shared Pattern & System Fit
|
|
|
|
- **Cross-cutting feature marker**: yes
|
|
- **Systems touched**: `OperationRunLinks`, `SystemOperationRunLinks`, `CanonicalNavigationContext`, `RecentOperationsSummary`, `InventoryCoverage`, `InventoryItemResource`, `ReviewPackResource`, `TenantlessOperationRunViewer`, `RelatedNavigationResolver`, panel navigation providers, tenant-selection middleware, and clear-tenant-context redirect behavior
|
|
- **Shared abstractions reused**: `App\Support\OperationRunLinks`, `App\Support\System\SystemOperationRunLinks`, `App\Support\Navigation\CanonicalNavigationContext`; the existing `App\Support\OpsUx\OperationRunUrl` wrapper remains acceptable where it simply delegates to `OperationRunLinks`
|
|
- **New abstraction introduced? why?**: none; the only new structure is a test-local allowlist boundary for legitimate raw producers
|
|
- **Why the existing abstraction was sufficient or insufficient**: The helper families already encode canonical labels, admin/system plane selection, entitled tenant continuity, and canonical query semantics. The only current-release gap is adoption and enforcement on specific producers that still assemble routes locally.
|
|
- **Bounded deviation / spread control**: Infrastructure-only exceptions are explicit, file-scoped, and non-precedential. UI surfaces, shared navigation, and related-link builders stay on the helper path.
|
|
|
|
## Constitution Check
|
|
|
|
*GATE: Passed before Phase 0 research. Re-checked after Phase 1 design: still passed with one bounded guard and no new persisted truth.*
|
|
|
|
| Gate | Status | Plan Notes |
|
|
|------|--------|------------|
|
|
| Inventory-first / read-write separation | PASS | The feature changes link generation only. It introduces no new writes, no new restore or remediation flow, and no new persisted artifact. |
|
|
| RBAC, workspace isolation, tenant isolation | PASS | Admin-plane destinations keep workspace and tenant entitlement checks; system-plane destinations remain platform-only; cross-plane access stays `404`. |
|
|
| Run observability / Ops-UX | PASS | Existing operations pages remain the canonical monitoring destination, but the feature does not start, mutate, or reclassify `OperationRun` lifecycle behavior. |
|
|
| Shared pattern first | PASS | The plan converges on `OperationRunLinks` and `SystemOperationRunLinks` instead of introducing a second link presenter or navigation framework. |
|
|
| Proportionality / no premature abstraction | PASS | The change is confined to migrating confirmed producers plus one bounded guard allowlist. No new registry, resolver family, or persisted truth is introduced. |
|
|
| UI semantics / Filament-native discipline | PASS | Existing Filament pages, resources, widgets, and navigation items remain intact; only helper-owned URLs change. No ad-hoc UI replacement or new action chrome is introduced. |
|
|
| Test governance | PASS | Proof stays in focused feature lanes with a narrow guard boundary. No browser lane or heavy-governance promotion is required for this slice. |
|
|
|
|
## Test Governance Check
|
|
|
|
- **Test purpose / classification by changed surface**: `Feature` for link continuity, representative admin drill-throughs, authorization-path preservation, and the bounded guard
|
|
- **Affected validation lanes**: `fast-feedback`, `confidence`
|
|
- **Why this lane mix is the narrowest sufficient proof**: The business truth is helper-owned URL generation, correct plane and scope continuity, and bounded prevention of new raw bypasses. Those behaviors are fully provable with targeted feature tests and one repo-guard test; browser coverage would add cost without validating additional business logic.
|
|
- **Narrowest proving command(s)**:
|
|
- `export PATH="/bin:/usr/bin:/usr/local/bin:$PATH" && cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent`
|
|
- `export PATH="/bin:/usr/bin:/usr/local/bin:$PATH" && cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/OpsUx/CanonicalViewRunLinksTest.php tests/Feature/Monitoring/OperationsDashboardDrillthroughTest.php tests/Feature/Filament/RecentOperationsSummaryWidgetTest.php tests/Feature/Filament/InventoryCoverageRunContinuityTest.php tests/Feature/ReviewPack/ReviewPackResourceTest.php tests/Feature/144/CanonicalOperationViewerDeepLinkTrustTest.php tests/Feature/078/RelatedLinksOnDetailTest.php tests/Feature/RunAuthorizationTenantIsolationTest.php tests/Feature/System/Spec195/SystemDirectoryResidualSurfaceTest.php tests/Feature/System/Spec113/AuthorizationSemanticsTest.php tests/Feature/Guards/OperationRunLinkContractGuardTest.php`
|
|
- **Fixture / helper / factory / seed / context cost risks**: Minimal. Existing `OperationRun` factories, workspace membership helpers, tenant fixtures, and platform-user fixtures are sufficient.
|
|
- **Expensive defaults or shared helper growth introduced?**: No. The guard allowlist remains opt-in and local to this feature; no new global test helper is required.
|
|
- **Heavy-family additions, promotions, or visibility changes**: none
|
|
- **Surface-class relief / special coverage rule**: Standard native-Filament relief plus the existing `monitoring-state-page` profile for canonical operations destinations
|
|
- **Closing validation and reviewer handoff**: Re-run `pint`, then the focused test command above. Reviewers should verify that each migrated source now uses the correct helper family, that remaining raw producers sit only in the named exception boundary, and that both admin and system authorization semantics are unchanged.
|
|
- **Budget / baseline / trend follow-up**: none
|
|
- **Review-stop questions**: Did the guard accidentally absorb tests or non-operator infrastructure? Did any admin-plane producer still hand-assemble `admin.operations` URLs? Did any system-plane producer start bypassing `SystemOperationRunLinks`? Did any exception remain convenience-based instead of infrastructure-based?
|
|
- **Escalation path**: `document-in-feature`
|
|
- **Active feature PR close-out entry**: `Guardrail`
|
|
- **Why no dedicated follow-up spec is needed**: This is current-release contract enforcement around an existing shared interaction family. Only a later desire for repo-wide routing policy would justify a separate governance spec.
|
|
|
|
## Project Structure
|
|
|
|
### Documentation (this feature)
|
|
|
|
```text
|
|
specs/232-operation-run-link-contract/
|
|
├── plan.md
|
|
├── research.md
|
|
├── data-model.md
|
|
├── quickstart.md
|
|
├── contracts/
|
|
│ └── operation-run-link-contract.logical.openapi.yaml
|
|
└── tasks.md
|
|
```
|
|
|
|
### Source Code (repository root)
|
|
|
|
```text
|
|
apps/platform/
|
|
├── app/
|
|
│ ├── Filament/
|
|
│ │ ├── Pages/
|
|
│ │ │ ├── InventoryCoverage.php
|
|
│ │ │ └── Operations/TenantlessOperationRunViewer.php
|
|
│ │ ├── Resources/
|
|
│ │ │ ├── InventoryItemResource.php
|
|
│ │ │ └── ReviewPackResource.php
|
|
│ │ └── Widgets/Tenant/RecentOperationsSummary.php
|
|
│ ├── Http/Controllers/ClearTenantContextController.php
|
|
│ ├── Providers/Filament/
|
|
│ │ ├── AdminPanelProvider.php
|
|
│ │ └── TenantPanelProvider.php
|
|
│ └── Support/
|
|
│ ├── Middleware/EnsureFilamentTenantSelected.php
|
|
│ ├── Navigation/
|
|
│ │ ├── CanonicalNavigationContext.php
|
|
│ │ └── RelatedNavigationResolver.php
|
|
│ ├── OperationRunLinks.php
|
|
│ ├── OpsUx/OperationRunUrl.php
|
|
│ └── System/SystemOperationRunLinks.php
|
|
└── tests/
|
|
└── Feature/
|
|
├── 078/RelatedLinksOnDetailTest.php
|
|
├── 144/CanonicalOperationViewerDeepLinkTrustTest.php
|
|
├── Filament/
|
|
│ ├── InventoryCoverageRunContinuityTest.php
|
|
│ └── RecentOperationsSummaryWidgetTest.php
|
|
├── Guards/OperationRunLinkContractGuardTest.php
|
|
├── Monitoring/OperationsDashboardDrillthroughTest.php
|
|
├── OpsUx/CanonicalViewRunLinksTest.php
|
|
├── ReviewPack/ReviewPackResourceTest.php
|
|
├── RunAuthorizationTenantIsolationTest.php
|
|
└── System/
|
|
├── Spec113/AuthorizationSemanticsTest.php
|
|
└── Spec195/SystemDirectoryResidualSurfaceTest.php
|
|
```
|
|
|
|
**Structure Decision**: Single Laravel application inside the monorepo. Runtime changes stay inside `apps/platform`, while planning artifacts remain under `specs/232-operation-run-link-contract`.
|
|
|
|
## Complexity Tracking
|
|
|
|
No constitutional violation is planned. One bounded review artifact is tracked explicitly because the feature adds an allowlisted guard boundary.
|
|
|
|
| Violation | Why Needed | Simpler Alternative Rejected Because |
|
|
|-----------|------------|-------------------------------------|
|
|
| BLOAT-001 bounded exception inventory | The guard needs a small explicit exception list so bootstrapping and redirect code does not have to fake helper usage or fabricate runtime context it does not own. | A raw repo-wide ban without named exceptions would create false positives, blur the operator-facing boundary, and push the feature into heavy-governance scope. |
|
|
|
|
## Proportionality Review
|
|
|
|
- **Current operator problem**: Platform-owned admin and system surfaces still have parallel ways to open the same canonical operations destinations, which creates plane and scope drift and keeps the next contributor free to reintroduce raw route assembly.
|
|
- **Existing structure is insufficient because**: The helper families already exist, but they are optional in practice. Without a bounded enforcement slice, the repository keeps two competing link-generation paths.
|
|
- **Narrowest correct implementation**: Migrate the confirmed raw admin producers, preserve the existing system helper path, record the few legitimate infrastructure exceptions, and add one route-bounded guard that fails on new bypasses.
|
|
- **Ownership cost created**: Small ongoing maintenance of the exception list and targeted regression coverage for a shared interaction family.
|
|
- **Alternative intentionally rejected**: A repo-wide route-string ban or a new navigation presenter stack. Both would be broader than the current operator problem and would add governance or architecture cost the release does not need.
|
|
- **Release truth**: Current-release contract enforcement and cleanup.
|
|
|
|
## Phase 0 Research Summary
|
|
|
|
- Reuse `OperationRunLinks` for every admin-plane producer that opens `/admin/operations` or `/admin/operations/{run}`; do not create a second admin helper or presenter.
|
|
- Treat the currently confirmed raw admin-plane producers as first-slice migration targets: `RecentOperationsSummary`, `InventoryCoverage`, `InventoryItemResource`, `ReviewPackResource`, `TenantlessOperationRunViewer`, and `RelatedNavigationResolver`.
|
|
- Keep the initial explicit exception boundary narrow and infrastructure-only: panel providers, tenant-selection middleware, and clear-tenant-context redirects may stay raw if helper adoption would fabricate the wrong runtime context.
|
|
- Preserve `SystemOperationRunLinks` as the canonical system-plane path. Current system widgets and pages are already largely converged, so the first slice focuses on regression prevention rather than inventing synthetic migration work.
|
|
- Treat `OperationRunUrl` as an acceptable thin delegating seam because it forwards directly to `OperationRunLinks` and does not create parallel routing truth.
|
|
- Keep the guard route-bounded to platform-owned UI and shared navigation code under `apps/platform/app`; do not scan tests, helpers themselves, or unrelated infrastructure.
|
|
|
|
## Phase 1 Design Summary
|
|
|
|
- `data-model.md` documents the feature as a derived contract over existing `OperationRun`, tenant, workspace, and canonical navigation truth plus a bounded producer inventory and explicit exception model.
|
|
- `contracts/operation-run-link-contract.logical.openapi.yaml` defines the internal logical contract for canonical admin/system collection and detail links plus the bounded guard request and result shape.
|
|
- `quickstart.md` provides the focused validation path for representative admin drill-throughs, system-plane continuity, allowlisted exceptions, and authorization semantics.
|
|
|
|
## Implementation Close-Out Inventory
|
|
|
|
- **Migrated admin producers**: `RecentOperationsSummary` collection links now use `OperationRunLinks::index(...)`; `InventoryCoverage` basis/detail and inventory-sync history links now use `OperationRunLinks::view(...)` and helper-owned type filtering; `InventoryItemResource` and `ReviewPackResource` detail links now use `OperationRunLinks::view(...)` or `tenantlessView(...)`; `TenantlessOperationRunViewer` default collection fallbacks now use `OperationRunLinks::index()`; `RelatedNavigationResolver` operation-run audit target links now use `OperationRunLinks::tenantlessView(...)`.
|
|
- **Verified system producers**: `ViewTenant`, `ViewWorkspace`, `Runs`, and `ViewRun` remain on `SystemOperationRunLinks::index()` and `SystemOperationRunLinks::view(...)` with no admin-plane fallback.
|
|
- **Accepted delegate**: `App\Support\OpsUx\OperationRunUrl` remains a thin delegate to `OperationRunLinks` and is explicitly covered by helper-contract tests.
|
|
- **Allowlisted infrastructure exceptions**: `AdminPanelProvider`, `TenantPanelProvider`, `EnsureFilamentTenantSelected`, and `ClearTenantContextController` retain raw `admin.operations.index` routes because they own panel bootstrapping or redirect behavior rather than source-surface drill-through context.
|
|
- **Guard boundary**: `OperationRunLinkContractGuardTest` scans the migrated admin producers, verified system producers, accepted delegate, and four explicit infrastructure exceptions. It blocks raw `admin.operations.index`, raw `admin.operations.view`, direct `/system/ops/runs` paths, and direct `Runs::getUrl(...)` / `ViewRun::getUrl(...)` use outside the allowlist.
|
|
- **Test-governance disposition**: `document-in-feature`. The cost is contained to a focused feature guard and representative feature coverage; no follow-up spec or heavy-governance lane is needed.
|
|
|
|
## Phase 1 Agent Context Update
|
|
|
|
- Run `.specify/scripts/bash/update-agent-context.sh copilot` after the plan, research, data model, quickstart, and contract artifacts are written.
|
|
- The update must preserve manual additions between generated markers and add only the new technology and change notes relevant to Spec 232.
|
|
- The generated agent-context file is supporting output for the planning workflow, not a reason to widen the feature scope.
|
|
|
|
## Implementation Strategy
|
|
|
|
1. **Inventory and classify producers**
|
|
- Freeze the first-slice inventory of raw admin-plane link producers and distinguish between migration targets, verified helper-backed system producers, and explicit infrastructure exceptions.
|
|
|
|
2. **Migrate direct admin collection and detail links**
|
|
- Replace raw `route('admin.operations.index')` and `route('admin.operations.view')` usage in widgets, pages, and resources with `OperationRunLinks` methods.
|
|
- Preserve canonical tenant continuity, problem-class filters, active-tab semantics, and current operator-facing labels through the helper contract only.
|
|
|
|
3. **Normalize shared related-navigation and back-link paths**
|
|
- Refactor `RelatedNavigationResolver` and `TenantlessOperationRunViewer` to consume canonical helper methods rather than local route assembly.
|
|
- Keep back-link context helper-owned when `CanonicalNavigationContext` is present and degrade cleanly to the canonical admin collection when it is absent.
|
|
|
|
4. **Retain only justified infrastructure exceptions**
|
|
- Keep panel navigation providers, tenant-selection middleware, and clear-tenant-context redirects as the narrow allowlisted exception set for this slice.
|
|
- Record each allowlisted exception with a reason tied to bootstrapping or redirect ownership, not convenience.
|
|
|
|
5. **Protect the already-converged system plane**
|
|
- Audit verified helper-backed system pages and widgets and keep them on `SystemOperationRunLinks`, applying only minimal cleanup if a direct page URL slips through.
|
|
- Use the guard and authorization tests to prove that no platform-facing system producer regresses to admin-plane or direct page URL assembly.
|
|
|
|
6. **Add bounded regression coverage**
|
|
- Add one guard test that scans only the declared app-side boundary and fails with file-plus-snippet output on representative bypasses.
|
|
- Extend or update focused feature tests so admin-plane continuity, explicit admin `404`/`403` preservation, system-plane continuity, and negative authorization behavior remain explicit.
|
|
|
|
## Risks and Mitigations
|
|
|
|
- **False-positive guard scope**: A broad scan would catch tests or infrastructure code. Mitigation: keep the boundary on platform-owned UI and shared navigation files only and maintain a file-scoped allowlist for legitimate exceptions.
|
|
- **Tenant continuity drift**: Replacing raw URLs could accidentally drop canonical filters or navigation context. Mitigation: route collection and detail links through existing helper parameters and keep representative continuity tests in scope.
|
|
- **Back-link regression on run detail**: `TenantlessOperationRunViewer` currently mixes raw fallbacks with helper-owned detail refresh behavior. Mitigation: migrate both the back-to-operations and show-all-operations fallbacks in the same slice so behavior stays coherent.
|
|
- **Over-scoping into routing cleanup**: It is easy to turn this into a general route-string purge. Mitigation: keep the feature limited to `OperationRun` collection and detail link producers plus their bounded exception list.
|
|
|
|
## Post-Design Re-check
|
|
|
|
Phase 0 and Phase 1 outputs resolve the planning questions without introducing a new routing framework, new persisted navigation truth, or a repo-wide governance lane change. The plan remains constitution-compliant, helper-first, and ready for `/speckit.tasks`.
|