# Research — RBAC Troubleshooting & Tenant UI Bugfix Pack v1

**Date**: 2026-01-31  
**Spec**: [spec.md](spec.md)

This document captures design decisions and supporting rationale. Where applicable, decisions are grounded in the current codebase and the TenantPilot Constitution.

## Decision 1 — Archived tenants must remain resolvable in tenant-scoped routes

**Decision**: Make the tenant binding used by Filament tenancy include archived (soft-deleted) tenants so entitled members can view `/admin/t/{tenant}/...` even when archived.

**Rationale**:
- The current tenant lifecycle uses soft deletes + `status='archived'` on delete and restores back to `active`.
- If tenancy resolution excludes trashed records, the app will 404 *before* membership middleware can apply “deny-as-not-found for non-members”, causing false 404s for legitimate members.
- Constitution RBAC-UX requires:
  - non-members → 404
  - members but missing capability → 403
  - archived is a lifecycle state, not an isolation boundary.

**Evidence**:
- Tenant tenancy config (slug is `external_id`, route prefix `t`): [Admin panel tenancy](../../app/Providers/Filament/AdminPanelProvider.php#L32-L45)
- Archived behavior is implemented via soft deletes + status mutations: [Tenant booted hooks](../../app/Models/Tenant.php#L63-L92)

**Alternatives considered**:
- Stop using soft deletes for tenants and represent archived purely via a `status` field.
  - Rejected: larger behavioral change, wider surface area (queries, “activeQuery”, etc.), higher regression risk.
- Keep current 404 and treat “archived” as inaccessible.
  - Rejected: contradicts spec FR-004 and the operational need to restore.

## Decision 2 — Standardize Tenant view header actions via `UiEnforcement`

**Decision**: Wrap Tenant view page header actions (Edit/Deactivate) in the existing UI enforcement helper so tenant members without the capability see disabled actions with tooltips, avoiding “normal click leads to 403 page”.

**Rationale**:
- Constitution RBAC-UX-004 requires visible-but-disabled for members lacking capability.
- Existing code already follows this pattern in multiple resources (e.g., tenant list actions, membership manager relation managers).
- The current `ViewTenant` header actions include an unconditional edit action and an archive action that executes without capability checks.

**Evidence**:
- Unconditional edit header action: [ViewTenant header action group](../../app/Filament/Resources/TenantResource/Pages/ViewTenant.php#L19-L24)
- Deactivate executes without capability gating: [ViewTenant archive action](../../app/Filament/Resources/TenantResource/Pages/ViewTenant.php#L51-L74)

**Alternatives considered**:
- Hide actions entirely for users without capability.
  - Rejected by default: RBAC-UX prefers visible-but-disabled.
- Leave the action visible and rely only on server-side 403.
  - Rejected: fails the UX requirement (normal click to 403).

## Decision 3 — Restore icon consistency is a UI-only patch

**Decision**: Add an icon to the tenant row `Restore` action.

**Rationale**:
- Improves consistency and reduces “is this safe?” uncertainty.
- No behavioral/authorization changes; minimal regression risk.

**Evidence**:
- Restore action currently has no icon: [TenantResource restore action](../../app/Filament/Resources/TenantResource.php#L406-L454)

**Alternatives considered**:
- Leave it as-is.
  - Rejected: explicit FR-001.

## Decision 4 — Diagnostics is DB-only and uses existing audit logging patterns

**Decision**: Implement tenant-scoped diagnostics as DB-only rendering and DB-only repairs with explicit confirmation + server-side authorization, recording audit logs.

**Rationale**:
- Constitution requires read/write separation and that monitoring/diagnostics pages do not trigger external calls during render.
- Repairs are security-relevant mutations and must be audited.

**Alternatives considered**:
- Attach diagnostics to Monitoring/Operations.
  - Rejected: these repairs are DB-only and do not need `OperationRun` by default; keep scope small.

## Decision 5 — Membership invariants

**Decision**:
- Continue using server-side guards preventing last-owner removal/demotion.
- Add diagnostics checks for “missing owner” and provide a safe repair (promote chosen member to owner) gated by an existing management capability.

**Rationale**:
- Last-owner guard exists and is correct; the missing-owner case needs a UI recovery path.

**Evidence**:
- Last-owner guard behavior: [TenantMembershipManager last-owner guards](../../app/Services/Auth/TenantMembershipManager.php#L253-L287)

**Alternatives considered**:
- DB constraint enforcing at least one owner.
  - Rejected: non-trivial, would complicate bulk edits and migrations; better handled at app layer with explicit repair tools.

## Decision 6 — Duplicate memberships

**Decision**: Keep the DB-level uniqueness constraint as the primary protection, but still implement diagnostics that can detect historical duplicates and merge them safely (no-op when none exist).

**Rationale**:
- The uniqueness constraint exists today, but the product requirement includes a recovery/repair flow.

**Evidence**:
- Unique constraint in migration: [tenant_memberships unique index](../../database/migrations/2026_01_25_022729_create_tenant_memberships_table.php#L12-L26)

**Alternatives considered**:
- Remove diagnostics for duplicates since the DB constraint exists.
  - Rejected: conflicts with FR-006 and the “UI-only recovery” goal.

## Decision 7 — GUID vs bigint guardrails

**Decision**: Explicitly distinguish:
- internal tenant primary key (`tenants.id`, bigint)
- external tenant identifiers (`tenant_id` GUID / `external_id` string)

and ensure code paths that need an internal FK use `$tenant->getKey()` (cast to int), not `$tenant->tenant_id`.

**Rationale**:
- Prevent PostgreSQL `invalid input syntax for type bigint` errors caused by passing the GUID into bigint `tenant_id` columns.

**Alternatives considered**:
- Rename columns.
  - Rejected (out of scope / migration-heavy).