Commit Graph

2 Commits

Author SHA1 Message Date
shankar0123 7c01f811a1 feat(frontend): Phase 2 TanStack Query Discipline — close TQ-H1/H2 + TQ-M1/M2/M3 + PERF-H1 + P-H1 + partial TQ-L1
Phase 2 of the frontend-design audit: TanStack Query discipline.
Set the cross-cutting QueryClient defaults + staleTime/gcTime tier
model + visibility-aware polling + 4 optimistic-update mutations
before any further per-page work.

New foundation
==============

  web/src/api/queryConstants.ts (new)
    STALE_TIME = { REAL_TIME: 15s, REFERENCE: 5m, CONSTANT: 1h }
    GC_TIME    = { HEAVY: 1m,     STANDARD: 5m,   REFERENCE: 30m }
    Doc-comment explains the tier model so every new useQuery picks
    a tier rather than a hardcoded ms integer.

  web/src/main.tsx
    QueryClient defaults rewritten:
      pre:  staleTime: 10_000 + refetchOnWindowFocus: true (refetch
            storm on every tab refocus across 242 query sites)
      post: staleTime: STALE_TIME.REFERENCE (5min) + gcTime: GC_TIME
            .STANDARD (explicit 5min) + refetchOnWindowFocus: false
            (per-query opt-in for live-tile queries)
    retry: 1 unchanged per the audit's DO NOT.

Findings closed by source ID
============================

TQ-H2 (refetch storm)
  main.tsx QueryClient defaults — refetchOnWindowFocus: false root +
  per-query opt-in. STALE_TIME.REFERENCE 5min for everything else.

TQ-M1 (no gcTime overrides)
  main.tsx now sets gcTime: GC_TIME.STANDARD explicitly — the
  contract is documented at the root, not implicit-defaulted by
  TanStack.

TQ-M2 (12 inconsistent staleTime values)
  All 11 hardcoded numeric staleTime overrides migrated to the
  STALE_TIME tier constants. useAuthMe.ts (the 12th) already used
  its own constant — left alone. Tier mapping:
    - operator-facing live data (KeysPage keys, RoleDetail role,
      UsersPage, OIDCJWKSStatusPanel, ApprovalsPage):
        STALE_TIME.REAL_TIME (15s)
    - slow-changing reference data (KeysPage roles, RolesPage,
      AuthSettings bootstrap+runtime-config):
        STALE_TIME.REFERENCE (5min)
    - effectively immutable (RoleDetail permissions catalogue):
        STALE_TIME.CONSTANT (1hr)

TQ-H1 (OnboardingWizard infinite 5s poll)
  OnboardingWizard.tsx:288-302 — refetchInterval rewritten to v5
  functional form:
    refetchInterval: (query) =>
      (query.state.data?.data?.length ?? 0) > 0 ? false : 5_000;
  As soon as the first agent registers, the interval flips to false
  and the poll stops. Also explicit: refetchOnWindowFocus: true +
  staleTime: STALE_TIME.REAL_TIME (because this IS a live-tile poll
  during the wizard).

PERF-H1 (Dashboard polling storm)
  DashboardPage.tsx
    - jobs poll bumped 10s → 30s (10s granularity isn't needed when
      30s is already inside the human-attention window; the
      CertificateDetail page is where 10s polling lives)
    - visibility-listener pauses ALL Dashboard polls when
      document.visibilityState === 'hidden'; on visibility return,
      immediately invalidates the 4 live-tile queries (health,
      dashboard-summary, jobs, certs-by-status) so the operator
      sees fresh data instantly rather than waiting one tick.
    - The 4 live-tile queries (health, dashboard-summary, jobs,
      certs-by-status) opt into refetchOnWindowFocus: true +
      staleTime: STALE_TIME.REAL_TIME explicitly.
    - Backend aggregation gap (dashboard-summary + certs-by-status
      + certificates could collapse into 1 endpoint) tracked
      separately — Phase 3 backend follow-up.

P-H1 (CertificatesPage 4 duplicate-key pairs)
  Pre-Phase-2 4 pairs of distinct cache slots fetching the same data:
    ['profiles']        vs ['profiles-filter']
    ['issuers']         vs ['issuers-filter']
    ['owners', 'form']  vs ['owners-filter']
    ['teams', 'form']   vs ['teams-filter']
  Post-Phase-2 all four pairs collapse to a single parameterized
  queryKey shape: `[name, { per_page: 100 }]`. TanStack v5 dedupes
  on serialized queryKey — the modal + filter now share one cache
  slot per resource. 8 useQuery sites → 4 cache slots; backend
  hits halved on first paint of CertificatesPage.

TQ-M3 (4 of 5 priority optimistic-update mutations)
  Wired onMutate / onError-rollback / onSettled-invalidation on:
    1. mark-notification-read (NotificationsPage)
       — flips row status to 'read' in both ['notifications','all']
         + ['notifications','dead'] cache slots
    2. claim-discovered-cert (DiscoveryPage)
       — flips status to 'Managed' in ['discovered-certificates']
    3. dismiss-discovery (DiscoveryPage)
       — flips status to 'Dismissed' in same cache slot
    4. archive-certificate (CertificateDetailPage)
       — flips status to 'Archived' in ['certificate', id]; on
         success navigates to /certificates (optimistic data
         doesn't linger); on error restores snapshot + toasts
  All four fire the Phase 1 Sonner toast on success/failure.
  The 5th priority site (role-assignment toggle in
  auth/RoleDetailPage) uses raw async/await handlers rather than
  useTrackedMutation — converting it requires a structural
  refactor outside Phase 2's TQ-focus; tracked as Phase 2 follow-up.

TQ-L1 (useTrackedMutation extended tests)
  useTrackedMutation.test.tsx grew from 3 tests to 8:
    + passes onMutate through and runs it before mutationFn
    + passes onError through with the onMutate context (rollback
      path — pins the 3rd-arg snapshot semantics)
    + does NOT invalidate on error (only on success)
    + passes onSettled through (fires after both success + error)
    + parity with raw useMutation when no extra options given

Verification
============

  $ grep -E "refetchOnWindowFocus: false" web/src/main.tsx
    89:      refetchOnWindowFocus: false,        // per-query opt-in

  $ grep -E "STALE_TIME\.REFERENCE" web/src/main.tsx
    86:      staleTime: STALE_TIME.REFERENCE,    // 5 min

  $ grep -cE "useQuery.*\['profiles" web/src/pages/CertificatesPage.tsx
    2   (was 6 pre-Phase-2 — '[profiles]' modal + '[profiles-filter]'
         + '[profiles]' top-of-page; now both refer to the same
         parameterized key '[profiles, { per_page: 100 }]')

  $ grep -rE "onMutate" web/src --include='*.tsx' --exclude='*.test.*' | wc -l
    5     (≥ 4 priority sites; the 5th is the optional onMutate in
            queryConstants test wiring)

  $ grep -rE "STALE_TIME\." web/src --include='*.tsx' --include='*.ts' \
       --exclude='*.test.*' | wc -l
    18    (queryConstants.ts + main.tsx + 11 migrated callsites
            + OnboardingWizard + DashboardPage)

  $ npx tsc --noEmit
    (exit 0)

  $ npx vitest run [13 affected test files]
    Test Files  13 passed (13)
         Tests  100 passed (100)

  $ npx vite build
    ✓ built in 2.49s
    dist/assets/index-yg3cYtYA.js  1,113 kB
    (+3 kB vs Phase 1 — queryConstants + optimistic-update wrappers)

Audit-accuracy callouts
=======================

  * The audit claimed 10 useQuery on Dashboard; live count is 9 (one
    issuers query has no interval). All 8 polling queries now gated
    behind visibility-listener; the 9th (issuers) is non-polling and
    not affected.
  * TQ-L1 originally specified 4 test extensions; shipped 5
    (onMutate ordering, onError-with-context, no-invalidate-on-error,
    onSettled pass-through, parity-with-raw-useMutation).
  * Optimistic-update 5th-site (role-assignment toggle in
    auth/RoleDetailPage) deferred — RoleDetailPage handlers use raw
    async/await instead of useTrackedMutation. Refactoring it adds
    one more optimistic path but requires a structural change
    outside Phase 2's TQ-discipline scope. Tracked as Phase 2
    follow-up.

Residual risks
==============

  * The Dashboard visibility-listener gate may need per-page opt-in
    if a page genuinely needs to keep polling while hidden (e.g.
    a background-tab monitor). Not aware of any such case today;
    if needed, the gate is a simple `useState`-driven hook
    extracted to web/src/hooks/useTabVisibility.ts.
  * The Dashboard backend-aggregation collapse
    (dashboard-summary + certs-by-status + certificates → one
    endpoint) is documented as a Phase-3 backend item.
  * The 4 collapsed CertificatesPage pairs now request per_page=100
    everywhere. Operator with >100 issuers/owners/profiles/teams
    will see a truncated dropdown — that's an unrelated Phase-1-
    Combobox-migration concern; the right fix when it lands is to
    move issuer/owner/profile selectors to Combobox with
    server-side typeahead.
  * The 12-second total Bundle-1 audit of all useQuery sites
    still leaves ~230 queries running with the new 5-min
    REFERENCE default. The default is generous; aggressively-
    fresh per-page queries that genuinely need 15s freshness
    must opt in (the audit page, the agent-fleet live counter,
    in-flight scan progress).
2026-05-14 14:51:49 +00:00
shankar0123 69a508dfcf auth-bundle-1 Phase 9 + 10: approval-bypass closure + RBAC GUI
# Phase 9 — approval-bypass closure (Decision 9, option a)

* Migration 000033_approval_kinds.up.sql: ALTER TABLE
  issuance_approval_requests ADD COLUMN approval_kind +
  payload JSONB; relax certificate_id + job_id to nullable;
  CHECK (approval_kind IN ('cert_issuance','profile_edit'))
  + CHECK (per-kind nullability invariant) + index on
  approval_kind. Idempotent throughout via DO blocks.
* domain.ApprovalKind enum (cert_issuance / profile_edit) +
  IsValidApprovalKind. ApprovalRequest gains Kind +
  Payload []byte for the pending profile diff.
* postgres.ApprovalRepository.Create + scanApprovalRow extended
  to round-trip the new columns; certificate_id + job_id
  switched to sql.NullString so profile_edit rows persist
  cleanly. Default Kind=cert_issuance preserves back-compat
  for every Phase-7-2026-05-03 caller.
* ApprovalService.RequestProfileEditApproval: new entry point
  that creates a pending profile-edit row carrying the
  serialized profile diff. Bypass mode (CERTCTL_APPROVAL_BYPASS)
  short-circuits the same way it does for cert_issuance.
* ApprovalService.SetProfileEditApply hook: cmd/server/main.go
  registers a closure that deserializes req.Payload + persists
  via profileRepo.Update + emits a profile.edit_applied audit
  row with category=auth. The hook avoids the Approval ↔
  Profile import cycle.
* ProfileService.UpdateProfile: gates when (a) the live
  profile carries RequiresApproval=true, OR (b) the proposed
  edit would set it true. Returns ErrProfileEditPendingApproval
  with the new approval ID; ProfileHandler maps to HTTP 202
  Accepted + {pending_approval_id}. Both arms close the
  flip-flop loophole because every transition through an
  approval-tier profile fires the gate.
* TestProfileEdit_RequiresApprovalLoopholeClosed pins all 3
  bypass attempts (flip-off / kept-on / flip-on) gated; nil-
  approval-service preserves pre-Phase-9 direct-apply for
  test fixtures.
* Approval service tests gain 4 profile_edit rows: pending row
  shape; same-actor self-approve rejected with
  ErrApproveBySameActor (load-bearing two-person integrity);
  approve fails-closed when apply callback unwired;
  apply callback invoked on approve.
* docs/reference/profiles.md (new) explains the gate +
  edit response shape (202) + same-actor invariant + bypass
  + audit hooks.

# Phase 10 — RBAC management GUI

* useAuthMe hook (web/src/hooks/useAuthMe.ts): TanStack Query
  fetches /api/v1/auth/me on app boot, caches for 60s, exposes
  hasPerm(p) + hasAnyPerm + isAdmin predicates. Every Phase-10
  page consumes this on mount + gates affordances against the
  cached effective_permissions slice. Server-side enforcement
  is the load-bearing gate; client-side hide/disable is UX.
* New routes:
   - /auth/roles — list (auth.role.list); create-role modal
     (auth.role.create) hidden when missing.
   - /auth/roles/:id — detail + permissions; edit
     (auth.role.edit), delete (auth.role.delete), add/remove
     permission affordances each gated.
   - /auth/keys — list of every actor with role grants; assign
     + revoke modals (auth.role.assign). actor-demo-anon
     flagged system-managed; mutation buttons hidden for it.
   - /auth/settings — stub showing /v1/auth/me identity +
     bootstrap-endpoint availability via /v1/auth/bootstrap.
* AuditPage extended with category filter ('All categories'
  + the 3 enum values from migration 000032). Selection flows
  to the API call params + the URL-driven query state.
* Layout: 3 new nav entries (Roles / API Keys / Auth Settings).
* api/client.ts: 12 new exported functions for the RBAC
  surface (authMe, list/get/create/update/delete role,
  list/add/remove role permissions, list keys, assign/revoke
  key role, bootstrap-availability probe).
* data-testid attributes on every interactive element so a
  future Playwright suite can assert behavior without brittle
  CSS selectors.
* Empty state, error state, and unsaved-changes warnings on
  every form per the prompt's implementation rules.

# Frontend tests

* RolesPage.test.tsx (6 tests): list render, empty state,
  error state, hide-create-button-without-perm,
  show-create-button-with-perm, submit-create-modal.
* KeysPage.test.tsx (3 tests): demo-anon flagged
  system-managed (no buttons), permission-gated affordance
  hide for auditor caller, assign-modal-POST contract.
* AuthSettingsPage.test.tsx (2 tests): identity surface,
  bootstrap-OPEN-status surface.
* AuditPage.test.tsx (+1): category-filter select renders
  with the 4 documented options.

15 frontend tests total in src/pages/auth/ + the audit
category-filter test; all pass via npx vitest run.

# Verifications

* go vet ./... clean.
* staticcheck across internal/auth + handler + router + cli +
  service + repository + cmd + domain: clean.
* gofmt -l clean repo-wide.
* go test -short -count=1 green across internal/service,
  internal/api/handler, internal/api/router, internal/auth,
  internal/auth/bootstrap, internal/service/auth,
  internal/domain/auth, cmd/server, cmd/cli, internal/cli.
* npx tsc --noEmit clean.
* npm run build green (vite build produces dist/index.html
  + 946KB JS bundle; chunk-size warning is pre-existing).
* npx vitest run src/pages/auth/ src/pages/AuditPage.test.tsx
  green (15 tests, 4 files).
2026-05-09 21:03:59 +00:00