TenantAtlas/specs/241-support-diagnostic-pack/research.md
Ahmed Darrazi 45e6142a67
Some checks failed
PR Fast Feedback / fast-feedback (pull_request) Failing after 1m0s
feat(support-diagnostics): guardrail refactor and UI polish (agent)
2026-04-26 01:30:28 +02:00

140 lines
9.2 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Research — Support Diagnostic Pack
**Date**: 2026-04-25
**Spec**: [spec.md](spec.md)
This document captures design decisions and supporting rationale for the first support-diagnostics slice. All decisions are grounded in current repository truth and the TenantPilot Constitution.
## Decision 1 — The support diagnostic bundle stays derived and read-only
**Decision**: Keep the first slice as a derived bundle assembled at request time from existing records. Do not introduce a persisted `SupportDiagnosticPack` model, table, queue, or export artifact.
**Rationale**:
- The operator problem is fast support-safe context gathering, not a new long-lived domain object.
- Constitution `PERSIST-001` and `PROP-001` require derive-before-persist when current-release truth does not need an independent lifecycle.
- The bundle needs deterministic structure and auditability, but neither requires a new stored entity in this slice.
**Evidence**:
- Existing canonical truth already exists on `OperationRun`, `ProviderConnection`, `Finding`, `StoredReport`, `TenantReview`, `ReviewPack`, and `AuditLog`.
- The spec explicitly forbids a persisted support-pack entity and raw export pipeline.
**Alternatives considered**:
- Persist a generated support pack with its own lifecycle.
- Rejected: introduces new truth, retention concerns, and export expectations the first slice does not need.
- Add page-local copy/export helpers only.
- Rejected: too narrow and drift-prone, and fails the cross-surface reuse goal.
## Decision 2 — Reuse the two existing entry surfaces instead of creating a new support page
**Decision**: Add read-only `Open support diagnostics` actions to `TenantDashboard` and `TenantlessOperationRunViewer` only. Do not create a standalone Filament resource or route.
**Rationale**:
- The user already reaches support context from an existing tenant or run workflow.
- Constitution `DECIDE-001`, `UI-FIL-001`, and the specs surface contract require support diagnostics to stay secondary or tertiary, not become a new queue or product area.
- Existing surfaces already establish the correct workspace/tenant/run authorization context.
**Evidence**:
- Tenant entry surface: `apps/platform/app/Filament/Pages/TenantDashboard.php`
- Canonical run detail surface: `apps/platform/app/Filament/Pages/Operations/TenantlessOperationRunViewer.php`
- The run detail page already owns scope, back navigation, refresh, and grouped related links.
**Alternatives considered**:
- Create `/admin/support-diagnostics/...` pages.
- Rejected: wider surface area, new navigation semantics, and unnecessary support-product expansion.
## Decision 3 — Add one tenant/admin-plane support capability, but do not widen tenant visibility
**Decision**: Introduce one tenant/admin-plane capability, `support_diagnostics.view`, in the canonical tenant capability registry and role mapping. Keep support diagnostics available only on already-authorized tenant and run surfaces. Do not add any System Panel support-diagnostics capability in this slice.
**Rationale**:
- The spec requires a dedicated support-diagnostics capability and strict `404` vs `403` boundaries.
- The roadmaps open question about support diagnostics revealing tenant metadata without tenant-directory access is resolved narrowly here: this slice does not create a new metadata visibility path. It only augments already-authorized admin-plane surfaces.
- That keeps current tenant/workspace isolation intact and defers platform-plane least-privilege splitting to the separate system-panel RBAC work.
**Evidence**:
- Current tenant/admin capability registry: `apps/platform/app/Support/Auth/Capabilities.php`
- Current run authorization semantics: `apps/platform/app/Policies/OperationRunPolicy.php`
- Product candidate open question on tenant metadata visibility: `docs/product/spec-candidates.md`
**Alternatives considered**:
- Reuse an existing broad capability such as `tenant.view` or `audit.view`.
- Rejected: too coarse for a support-safe bundle that aggregates multiple record types.
- Add a platform/system-plane support capability now.
- Rejected: outside the current admin-plane slice and would broaden scope into least-privilege platform RBAC.
## Decision 4 — Existing shared helpers remain authoritative for labels, links, and run explanation
**Decision**: The bundle builder must reuse `OperationRunLinks`, `GovernanceRunDiagnosticSummaryBuilder`, `ProviderReasonTranslator`, `ProviderConnectionSurfaceSummary`, `RelatedNavigationResolver`, and `RedactionIntegrity` instead of creating support-local link builders or explanation text.
**Rationale**:
- Constitution `XCUT-001` requires reuse of the existing shared path for cross-cutting interaction classes.
- The support bundle should not create a second vocabulary for operation wording, provider readiness, or related-record labels.
- Reuse reduces drift and makes future AI-safe consumers rely on the same canonical operator phrasing.
**Evidence**:
- Canonical run link helpers: `apps/platform/app/Support/OperationRunLinks.php`
- Humanized run diagnostic summaries: `apps/platform/app/Support/OpsUx/GovernanceRunDiagnosticSummaryBuilder.php`
- Provider reason translation: `apps/platform/app/Support/Providers/ProviderReasonTranslator.php`
- Related-record navigation and audit target linking: `apps/platform/app/Support/Navigation/RelatedNavigationResolver.php`
- Existing redaction note wording: `apps/platform/app/Support/RedactionIntegrity.php`
**Alternatives considered**:
- Build support-specific strings and URLs inside each page action.
- Rejected: duplicated semantics, harder review, and immediate drift risk.
## Decision 5 — Deterministic ordering is part of the bundle contract, not a UI afterthought
**Decision**: Fix one section order for every bundle and sort references by stable business signals first, then stable record identity. Prefer run-bound references when the current run explicitly identifies them; otherwise fall back to the current tenants latest authorized canonical records.
**Rationale**:
- The spec requires that the same authorized input and unchanged source truth produce the same section order, reference order, and redaction output.
- Incidental query order would make later AI consumption and regression testing unreliable.
- This decision can stay local to the bundle builder and documented contract without adding a new enum or persistence layer.
**Expected ordering**:
- Section order: overview, provider connection, operation context, findings, stored reports, tenant review, review pack, audit history.
- Reference ordering: prefer run-bound reference first; otherwise order by the relevant lifecycle timestamp (`recorded_at`, `generated_at`, `last_seen_at`, `completed_at`, `id`) with deterministic tie-breakers.
**Evidence**:
- `AuditLog` already defines a canonical latest-first ordering via `scopeLatestFirst()`.
- `ReviewPack`, `TenantReview`, and `OperationRun` expose stable lifecycle timestamps and latest-first retrieval patterns.
**Alternatives considered**:
- Let each section use its default Eloquent query order.
- Rejected: not deterministic enough for the contract promised by the spec.
## Decision 6 — Audit bundle-open activity through the existing recorder with redacted metadata only
**Decision**: Record bundle-open activity through `WorkspaceAuditLogger` with a new audit action identifier. Include actor, workspace, tenant when present, primary context type/id, redaction mode, and section/reference counts. Exclude raw provider payloads, secrets, tokens, and unrestricted log excerpts.
**Rationale**:
- The feature is read-only, but support-diagnostics access is still security- and audit-relevant.
- `WorkspaceAuditLogger` already supports workspace-scoped and tenant-scoped audit entries without introducing a support-specific persistence path.
- Redacted metadata is sufficient for evidence and review.
**Evidence**:
- Workspace-scoped audit writer: `apps/platform/app/Services/Audit/WorkspaceAuditLogger.php`
- Canonical audit action IDs: `apps/platform/app/Support/Audit/AuditActionId.php`
**Alternatives considered**:
- Skip auditing because the action is read-only.
- Rejected: contradicts the specs auditability requirement.
- Persist the full generated bundle in the audit log.
- Rejected: leaks excluded data and violates the derive-before-persist goal.
## Decision 7 — Proof stays in Unit + Feature lanes only
**Decision**: Keep proof in focused unit and feature suites. Do not introduce browser coverage, heavy-governance flows, or new lane families for this slice.
**Rationale**:
- The business truth is deterministic bundle composition plus server-side authorization, redaction, canonical link reuse, and audit logging.
- Browser tests would mostly duplicate modal/preview rendering and slow down the narrow support slice.
- Constitution `TEST-GOV-001` requires the narrowest proving lane mix.
**Evidence**:
- Existing feature coverage already exercises tenant dashboard, canonical run detail, monitoring navigation, audit visibility, and operation-link contracts.
- Existing unit coverage already protects shared helpers such as run summaries and navigation resolvers.
**Alternatives considered**:
- Add browser smoke tests for modal interaction.
- Rejected: browser proof is not necessary to establish the core business truth of this first slice.