57 lines
7.2 KiB
Markdown
57 lines
7.2 KiB
Markdown
# Research: Finding Outcome Taxonomy & Verification Semantics
|
|
|
|
## Decision 1: Preserve the primary findings status family and derive verification meaning
|
|
|
|
- **Decision**: Keep the existing `Finding` lifecycle statuses (`new`, `triaged`, `in_progress`, `reopened`, `resolved`, `closed`, `risk_accepted`) and derive `Resolved pending verification` versus `Verified cleared` from bounded canonical reason keys plus existing audit history.
|
|
- **Rationale**: `App\Models\Finding` already defines open and terminal status sets, `FindingWorkflowService` and its tests assume that matrix, and the spec explicitly forbids a new primary status just to represent verification. The narrowest viable change is to make terminal meaning derive from bounded reason families instead of expanding the workflow state machine.
|
|
- **Alternatives considered**:
|
|
- Add a new `verified` status: rejected because it widens queue semantics, transition rules, and operator routing for a problem that can remain derived.
|
|
- Add a second outcome table or reporting-only register: rejected because there is no new independent source of truth or lifecycle to persist.
|
|
|
|
## Decision 2: Reuse existing `resolved_reason` and `closed_reason` fields, and keep reopen reasons in audit metadata
|
|
|
|
- **Decision**: Reuse the existing `resolved_reason` and `closed_reason` columns as canonical stable keys, and keep `reopened_reason` as structured audit metadata instead of adding a new `reopened_reason` column.
|
|
- **Rationale**: `FindingWorkflowService` already writes `resolved_reason`, `closed_reason`, `system_origin`, and `reopened_reason` into current record state and audit metadata. The service only needs to stop accepting arbitrary free-form text and instead validate against bounded key families. This keeps the implementation inside existing persistence truth and matches the spec's no-new-entity posture.
|
|
- **Alternatives considered**:
|
|
- Add a new JSON outcome payload on `findings`: rejected because it would create a second semantic store for data already represented by existing columns and audit events.
|
|
- Keep free-form textarea input as the primary meaning: rejected because list filters, review consumers, and audit readers cannot safely depend on prose.
|
|
|
|
## Decision 3: Use one findings-local semantics helper rather than page-local mappings or a generic framework
|
|
|
|
- **Decision**: Add one findings-local helper, such as `App\Support\Findings\FindingOutcomeSemantics`, to convert current finding truth into terminal-outcome labels, verification state, and report buckets.
|
|
- **Rationale**: Multiple readers already need the same meaning: `FindingResource`, `FindingRiskGovernanceResolver`, `ReviewRegister`, `TenantReviewResource`, and findings-derived summary or projection code. A single findings-local helper is justified because at least two real consumers already exist, while a generic governance-wide taxonomy engine would be broader than the current release truth.
|
|
- **Alternatives considered**:
|
|
- Keep logic inside `FindingResource` only: rejected because review and reporting consumers would immediately drift.
|
|
- Build a broad cross-domain governance taxonomy framework: rejected because the current scope is bounded to findings terminal outcomes.
|
|
|
|
## Decision 4: Use canonical reason keys to make system-cleared outcomes row-visible, and keep audit history for path reconstruction
|
|
|
|
- **Decision**: Treat canonical resolve keys as the row-level source for pending-versus-verified meaning. Manual resolve uses a bounded operator key such as `remediated`, while trusted system-clear reasons continue to use bounded system keys such as `no_longer_drifting`, `permission_granted`, `permission_removed_from_registry`, `role_assignment_removed`, and `ga_count_within_threshold`.
|
|
- **Rationale**: `resolveBySystem()` currently writes `system_origin` into audit metadata, but that audit-only flag is not enough for list filters or report buckets. Using canonical resolve keys as the current-row truth makes terminal meaning queryable without adding a new column, while the audit trail still records whether the final verified-clear state came from a direct system clear or a later confirmation after manual resolution.
|
|
- **Alternatives considered**:
|
|
- Add a persisted `verification_state` column: rejected because the same meaning can be derived from the current status plus canonical reason keys.
|
|
- Depend only on audit metadata for verified-clear meaning: rejected because list filters and read models should not need audit log reconstruction to classify current records.
|
|
|
|
## Decision 5: Widen the system resolve path narrowly enough to confirm already resolved findings
|
|
|
|
- **Decision**: Allow the system-clear path to update a finding that is already `resolved` when the purpose is to move it from operator-declared resolution to a trusted verified-clear reason.
|
|
- **Rationale**: `FindingWorkflowService::resolveBySystem()` currently only accepts open findings, which is too narrow for the spec requirement that a previously resolved finding may later be confirmed clear by trusted evidence. The narrowest fix is to widen the system path for this exact case instead of adding a new status or a second persistence field.
|
|
- **Alternatives considered**:
|
|
- Force a reopen-then-resolve cycle to represent verification: rejected because it would falsify the user-visible history and generate unnecessary workflow churn.
|
|
- Add a second workflow method dedicated to verification with separate persisted state: rejected because it adds more surface than the current release needs.
|
|
|
|
## Decision 6: Keep `risk_accepted` separate from remediation and verification semantics
|
|
|
|
- **Decision**: Preserve `risk_accepted` as its own terminal class governed by exception validity, not as a close reason and not as a verified-clear outcome.
|
|
- **Rationale**: `FindingRiskGovernanceResolver` already interprets `accepted_risk` and exception validity separately, and the spec requires Spec 154 semantics to remain authoritative. This means the outcome taxonomy should expose `risk_accepted` as a distinct terminal bucket while continuing to compute governance validity independently.
|
|
- **Alternatives considered**:
|
|
- Fold `risk_accepted` into generic closed outcomes: rejected because that would hide governance validity consequences.
|
|
- Treat valid accepted risk as verified clear: rejected because risk acceptance is an administrative governance decision, not proof of remediation.
|
|
|
|
## Decision 7: Replace existing ad hoc keys in one pass instead of preserving aliases
|
|
|
|
- **Decision**: Replace existing ad hoc or legacy-like reason strings inside current code, factories, and backfill jobs during implementation rather than preserving alias maps.
|
|
- **Rationale**: The repository is still pre-production, LEAN-001 explicitly rejects compatibility shims when there is no live production data, and current code already shows several ad hoc reason strings such as `duplicate`, `accepted_risk`, and `consolidated_duplicate`. The cleanest implementation is one canonical bounded family, updated everywhere in the same slice.
|
|
- **Alternatives considered**:
|
|
- Maintain alias translation tables for old and new reason keys: rejected because it adds avoidable compatibility machinery.
|
|
- Leave existing reasons mixed and normalize only in the UI: rejected because backend tests, report buckets, and audit semantics would remain ambiguous. |