ahmido
c86b399b43
Some checks failed
Main Confidence / confidence (push) Failing after 53s
## Summary This PR delivers three related improvements: ### 1. Finding Ownership Semantics (Spec 219) - Add responsibility/accountability labels to findings and finding exceptions - `owner_user_id` = accountable party (governance owner) - `assignee_user_id` = responsible party (technical implementer) - Expose Assign/Reassign actions in FindingResource with audit logging - Add ownership columns and filters to finding list - Propagate owner from finding to exception on creation - Tests: ownership semantics, assignment audit, workflow actions ### 2. Constitution v2.7.0 — LEAN-001 Pre-Production Lean Doctrine - New principle forbidding legacy aliases, migration shims, dual-write logic, and compatibility fixtures in a pre-production codebase - AI-agent 4-question verification gate before adding any compatibility path - Review rule: compatibility shims without answering the gate questions = merge blocker - Exit condition: LEAN-001 expires at first production deployment - Spec template: added default "Compatibility posture" block - Agent instructions: added "Pre-production compatibility check" section ### 3. Backup Set Operation Type Unification - Unified `backup_set.add_policies` and `backup_set.remove_policies` into single canonical `backup_set.update` - Removed all legacy aliases, constants, and test fixtures - Added lifecycle coverage for `backup_set.update` in config - Updated all 14+ test files referencing legacy types ### Spec Artifacts - `specs/219-finding-ownership-semantics/` — full spec, plan, tasks, research, data model, contracts, checklist ### Tests - All affected tests pass (OperationCatalog, backup set, finding workflow, ownership semantics) Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de> Reviewed-on: #256
52 lines
5.0 KiB
Markdown
52 lines
5.0 KiB
Markdown
# Research: Finding Ownership Semantics Clarification
|
||
|
||
**Date**: 2026-04-20
|
||
**Branch**: `219-finding-ownership-semantics`
|
||
|
||
## Decision 1: Reuse the existing finding owner and assignee fields as the only responsibility truth
|
||
|
||
- **Decision**: Keep `findings.owner_user_id` and `findings.assignee_user_id` as the sole persisted responsibility fields and derive any responsibility-state wording from them.
|
||
- **Rationale**: The current model, workflow service, migrations, and tests already support independent owner and assignee assignment. The spec problem is semantic ambiguity, not missing persistence.
|
||
- **Alternatives considered**:
|
||
- Add a new persisted responsibility status column. Rejected because the state is directly derivable from existing nullable fields.
|
||
- Introduce a separate responsibility aggregate/service object. Rejected because one resource and one model can express the needed truth without adding a new layer.
|
||
|
||
## Decision 2: Treat assignee-without-owner as an accountability gap, not a healthy assigned state
|
||
|
||
- **Decision**: Use a derived responsibility contract where owner presence determines accountability. `owner != null && assignee == null` is `owned but unassigned`; `owner != null && assignee != null` is `assigned`; any `owner == null` case is an accountability gap, even if an assignee exists.
|
||
- **Rationale**: The spec defines owner as accountable for the outcome and assignee as actively executing work. Existing resolver logic already treats missing owner or missing assignee as follow-up needing attention.
|
||
- **Alternatives considered**:
|
||
- Treat any assignee as `assigned` even without an owner. Rejected because it hides the exact accountability gap the feature exists to surface.
|
||
- Introduce a fourth persisted state family. Rejected because the business consequence is presentation and next-action guidance, not new workflow routing.
|
||
|
||
## Decision 3: Keep exception owner explicitly separate from finding owner
|
||
|
||
- **Decision**: Preserve `finding_exceptions.owner_user_id` as a distinct governance concept and label it only as `Exception owner` when shown in a finding context.
|
||
- **Rationale**: The current resource already surfaces exception owner separately, and the spec explicitly calls out the risk of accepted-risk flows reintroducing ambiguity under a second `Owner` label.
|
||
- **Alternatives considered**:
|
||
- Collapse exception owner into finding owner language. Rejected because it would blur responsibility for the finding with responsibility for the exception artifact.
|
||
- Rename finding owner around exception flows only. Rejected because local relabeling would increase rather than reduce semantic drift.
|
||
|
||
## Decision 4: Keep the implementation local to the existing findings resource and current workflow services
|
||
|
||
- **Decision**: Make the semantics change in `FindingResource`, with only small supporting adjustments in `Finding`, `FindingWorkflowService`, `FindingExceptionService`, and `FindingRiskGovernanceResolver` if wording or derived next-action copy needs alignment.
|
||
- **Rationale**: The current UI already displays owner, assignee, and exception owner. The gap is where those semantics are explained, prioritized, and tested.
|
||
- **Alternatives considered**:
|
||
- Add a new presenter or explanation layer for responsibility semantics. Rejected because a local derived helper is sufficient and aligned with the constitution’s anti-layering bias.
|
||
- Add a dedicated responsibility page. Rejected because the findings list/detail surfaces are already the operator decision context.
|
||
|
||
## Decision 5: Use focused Pest feature + Livewire coverage, not browser or heavy-governance tests
|
||
|
||
- **Decision**: Cover the feature with focused findings resource and workflow feature tests, including explicit tenant-context setup and Filament routing discipline.
|
||
- **Rationale**: Existing tests already prove tenant-member validation and row-action behavior. The missing coverage is list/detail semantics and owner-only versus assignee-only feedback.
|
||
- **Alternatives considered**:
|
||
- Browser tests. Rejected because the behavior is visible and provable through current Filament Livewire test patterns.
|
||
- Unit-only tests. Rejected because they would miss the operator-facing semantics that motivated the spec.
|
||
|
||
## Decision 6: Preserve current tenant-panel and canonical-admin context rules during testing
|
||
|
||
- **Decision**: Keep tenant context explicit in tests and use explicit panel selection for tenant-route assertions when necessary.
|
||
- **Rationale**: Repository memory shows two relevant pitfalls: tenant-scoped Filament list pages can lose context on later Livewire interactions, and `Resource::getUrl(..., tenant: $tenant)` defaults to the admin panel unless `panel: 'tenant'` is passed.
|
||
- **Alternatives considered**:
|
||
- Rely on implicit panel resolution in tests. Rejected because it can produce false redirects to `/admin/choose-tenant` instead of the intended authorization result.
|
||
- Broaden shared fixtures to always set tenant context. Rejected because the constitution prefers opt-in context rather than expensive defaults. |