TenantAtlas/specs/255-enforce-finding-creation-invariants/research.md
ahmido 51ea80ca05
Some checks failed
PR Fast Feedback / fast-feedback (pull_request) Failing after 1m5s
Automatische PR: 255-enforce-finding-creation-invariants → platform-dev (#298)
Automatisch erstellt: Commit & Push aus Workspace (WIP)

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #298
2026-04-29 12:26:21 +00:00

126 lines
7.5 KiB
Markdown

# Research — Enforce Creation-Time Finding Invariants
**Date**: 2026-04-29
**Spec**: [spec.md](spec.md)
This document records the repo-grounded planning decisions for the creation-time findings hardening slice after Specs 253 and 254. All decisions assume the current pre-production LEAN-001 posture.
## Decision 1 — Scope the feature to the three active finding writers that currently persist `Finding` records
**Decision**: Treat baseline compare drift, Entra admin roles, and permission posture as the full active writer set for this feature.
**Rationale**:
- Repo search shows only five direct `Finding` creation sites in app code: one `new Finding` path in `CompareBaselineToTenantJob` and four `Finding::create()` sites split between Entra admin roles and permission posture.
- No other shipped service or job currently persists `Finding` records directly, so widening the slice would be speculative rather than repo-driven.
- This keeps the hardening aligned with the spec's stated bounded scope and avoids inventing a new writer registry.
**Evidence**:
- `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`
- `apps/platform/app/Services/EntraAdminRoles/EntraAdminRolesFindingGenerator.php`
- `apps/platform/app/Services/PermissionPosture/PermissionPostureFindingGenerator.php`
**Alternatives considered**:
- Widen to every findings consumer or downstream summary surface.
- Rejected: those surfaces consume findings truth but do not create it.
- Add a speculative "all writers" registry now.
- Rejected: violates ABSTR-001 because three concrete paths are already directly visible.
## Decision 2 — Enforce lifecycle readiness in the same write path, not through a later repair pass
**Decision**: Require each in-scope writer to create or refresh lifecycle-ready findings inside the same code path that persists or updates the record.
**Rationale**:
- Spec 253 removes runtime backfill surfaces and this feature explicitly exists to prevent reintroducing that repair dependency.
- Current code already initializes lifecycle fields on new creates and updates some fields inline on repeated observations; that makes write-path hardening the narrowest correct implementation.
- Downstream findings pages, inboxes, and intake queues already assume findings are ready for immediate use.
**Evidence**:
- `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`
- `apps/platform/app/Filament/Resources/FindingResource.php`
- `apps/platform/app/Filament/Pages/Findings/MyFindingsInbox.php`
**Alternatives considered**:
- Reintroduce a maintenance action or backfill command.
- Rejected: directly conflicts with the cleanup direction from Spec 253.
- Add a deploy-time or queue-time repair hook.
- Rejected: widens scope and hides invariant ownership.
## Decision 3 — Preserve `FindingWorkflowService::reopenBySystem()` as the only shared reopen path
**Decision**: Keep terminal-to-reopened mutation on `FindingWorkflowService::reopenBySystem()` and treat open-record lifecycle normalization as the actual planning gap.
**Rationale**:
- `reopenBySystem()` already validates terminal-status eligibility, recalculates SLA or due state, clears resolved or closed markers, writes audit context, and dispatches the reopened alert notification.
- Bypassing it would create a second reopen dialect and risk inconsistent audit or notification semantics.
- The repo gap is not reopened-state ownership; it is that current open-record repair is still distributed across per-family `observeFinding()` logic and currently emphasizes seen-history more than full lifecycle readiness.
**Evidence**:
- `apps/platform/app/Services/Findings/FindingWorkflowService.php`
- `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`
- `apps/platform/app/Services/EntraAdminRoles/EntraAdminRolesFindingGenerator.php`
- `apps/platform/app/Services/PermissionPosture/PermissionPostureFindingGenerator.php`
**Alternatives considered**:
- Reopen findings directly inside each writer.
- Rejected: duplicates side effects and weakens audit consistency.
- Create a new generic lifecycle orchestration framework.
- Rejected: too broad for three known writers.
## Decision 4 — Keep recurrence identity family-owned and preserve each writer's current double-count boundary
**Decision**: Keep the existing recurrence identity and observation boundary per family instead of forcing one synthetic cross-domain algorithm.
**Rationale**:
- Baseline compare already uses `recurrence_key` plus `fingerprint` with `current_operation_run_id` to suppress duplicate `times_seen` increments for the same compare run.
- Entra admin roles and permission posture use later `observedAt` comparisons to advance seen history.
- The operator need is one canonical finding identity per issue family, not one universal recurrence engine.
**Evidence**:
- `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`
- `apps/platform/app/Services/EntraAdminRoles/EntraAdminRolesFindingGenerator.php`
- `apps/platform/app/Services/PermissionPosture/PermissionPostureFindingGenerator.php`
**Alternatives considered**:
- Normalize all writers onto a single recurrence service.
- Rejected: would add abstraction without current-release need.
- Count every repeated observation the same way across all writers.
- Rejected: risks breaking baseline retry semantics.
## Decision 5 — The current proof gap is inline repair of incomplete lifecycle fields on existing findings
**Decision**: Plan for explicit regression proof that existing open findings with missing lifecycle fields are repaired inline on active paths, especially for `sla_days` and `due_at`.
**Rationale**:
- Existing tests already prove creation readiness, idempotence, and reopen behavior in all three families.
- Repo code also already repairs `first_seen_at`, `last_seen_at`, and `times_seen` inline when existing findings are re-observed.
- What is not yet clearly owned as one invariant is the repair of incomplete lifecycle fields such as missing due-state data on existing findings encountered through active writers.
**Evidence**:
- `apps/platform/tests/Feature/Baselines/BaselineCompareFindingsTest.php`
- `apps/platform/tests/Feature/EntraAdminRoles/EntraAdminRolesFindingGeneratorTest.php`
- `apps/platform/tests/Feature/PermissionPosture/PermissionPostureFindingGeneratorTest.php`
**Alternatives considered**:
- Rely on current creation and reopen tests only.
- Rejected: leaves FR-255-007 partially implied.
- Add a new browser or broad workflow suite.
- Rejected: too expensive for a write-path invariant gap.
## Decision 6 — Keep schema and DB constraints out of the slice unless they become an explicit stop condition
**Decision**: Keep the default plan app-code-only. Any database-level constraint or migration-based enforcement is a bounded follow-up candidate or an explicit stop condition, not part of this feature by default.
**Rationale**:
- The repo is pre-production and LEAN-001 favors direct replacement over compatibility layers.
- The current code already has the necessary domain seams to harden write-time behavior without changing the schema.
- Folding a constraint into this feature would silently broaden it from write-path hardening into data rollout and compatibility review.
**Evidence**:
- `.specify/memory/constitution.md`
- `specs/255-enforce-finding-creation-invariants/spec.md`
**Alternatives considered**:
- Add `NOT NULL` or check constraints now.
- Rejected: outside the smallest bounded slice.
- Keep the option undefined.
- Rejected: the plan must name the stop condition explicitly so task generation stays bounded.