fix: post-merge settings bugs — SCIM duplication, crash, error boundary

[TerrifiedBug]

Summary

• SCIM config duplication: Removed inline <ScimSettings /> from the Authentication page — SCIM now lives exclusively at /settings/scim
• Team settings crash: settings.get requires super-admin but TeamSettings calls it unconditionally. Added throwOnError: false + retry: false so team admins degrade gracefully instead of crashing
• Error boundary sticky error: ErrorBoundary didn't reset on navigation, so one page's error broke all subsequent pages. Added componentDidUpdate to reset when children change

Test plan

• Log in as a team admin (not super-admin) and navigate to Team Settings — should load without error
• Navigate to Authentication settings — SCIM section should no longer appear inline
• Navigate to /settings/scim — SCIM settings should still work independently
• Trigger an error on any settings page, then navigate away — other pages should load normally

[greptile-apps]

Greptile Summary

This PR fixes three post-merge regressions across settings and navigation: it removes the duplicate inline <ScimSettings /> from the Authentication page, adds throwOnError: false + retry: false to the settings.get query so team admins (who are FORBIDDEN from that endpoint) degrade gracefully instead of crashing, and adds a componentDidUpdate reset to ErrorBoundary so a single page error no longer poisons subsequent navigation. The PR also includes an unrelated-but-straightforward alerts improvement that introduces GLOBAL_METRICS to hide the pipeline-scope selector for metrics that cannot be meaningfully scoped to a pipeline.

Key changes:

• auth-settings.tsx: <ScimSettings /> and its import removed — SCIM lives exclusively at /settings/scim
• team-settings.tsx: settings.get query now opts out of error-boundary propagation and retries, allowing oidcConfigured and the SCIM/OIDC info banner to silently default to false/hidden for non-super-admins
• error-boundary.tsx: componentDidUpdate resets error state when children reference changes; note that JSX always creates a new object reference, so any parent re-render during an error state can trigger a reset + re-render loop — that issue remains open
• alerts/page.tsx: GLOBAL_METRICS set gates the pipeline selector and table cell display; skipThreshold extended to cover BINARY_METRICS in both the open-edit and submit paths

Confidence Score: 4/5

• Safe to merge — the three targeted fixes are mechanically correct and no new crash vectors are introduced.
• All three fixes achieve their stated goals: the SCIM deduplication is a clean removal, the throwOnError: false pattern is the correct TanStack Query v5 approach for graceful query failure, and the error boundary reset works for the described navigation scenario. Score is 4 rather than 5 because the error boundary's children identity comparison (already flagged in a previous review thread) remains unresolved and can still produce a flash/retry loop on any parent re-render during an error state — it doesn't regress anything but the fix is incomplete.
• src/components/error-boundary.tsx — the children identity comparison fix is incomplete per the prior review thread; the other three files are straightforward.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Team Admin navigates to Team Settings] --> B{settings.get query}
    B -- "FORBIDDEN (non-super-admin)" --> C["throwOnError: false\nretry: false\n→ settingsQuery.data = undefined"]
    B -- "Success (super-admin)" --> D[settingsQuery.data populated]
    C --> E["oidcConfigured = false\nSSO link button hidden\nSCIM/OIDC banner hidden"]
    D --> F["oidcConfigured = true\nSSO link button shown\nSCIM/OIDC banner shown"]

    G[User navigates to /settings/auth] --> H[AuthSettings renders]
    H --> I[No inline ScimSettings]
    J[User navigates to /settings/scim] --> K[ScimSettings renders standalone]

    L[Error thrown in page] --> M[ErrorBoundary shows fallback]
    M --> N{children ref change?\nprevProps.children !== props.children}
    N -- "Yes (parent re-render OR navigation)" --> O[Reset error state\nRe-render children]
    N -- No --> M

Loading

_{Last reviewed commit: 6606bfe}

[greptile-apps]

Children comparison triggers on every parent re-render, not just navigation

prevProps.children !== this.props.children compares JSX element object identity. Because JSX creates a new object on every render, this condition will be true on any parent re-render — not only on route navigation.

If a parent component re-renders while the error fallback is visible (e.g. a polling query fires, or any context update occurs above the boundary), the sequence becomes:

1. Error thrown → fallback shown
2. Parent re-renders (new children object reference) → componentDidUpdate resets error state
3. Crashed children re-render → same error thrown again → getDerivedStateFromError sets hasError: true
4. Back to showing the fallback

This produces a visible flash/retry loop whenever a parent re-renders during an error state.

The canonical fix is to drive resets via a stable identity such as the current route pathname. The simplest approach is for callers to pass the current route as a key to the boundary — when the key changes on navigation, React unmounts and remounts the component, clearing error state without any componentDidUpdate logic. Alternatively, accept an explicit resetKey prop and compare that instead of children.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/components/error-boundary.tsx
Line: 29

Comment:
**Children comparison triggers on every parent re-render, not just navigation**

`prevProps.children !== this.props.children` compares JSX element object identity. Because JSX creates a **new object on every render**, this condition will be `true` on any parent re-render — not only on route navigation. 

If a parent component re-renders while the error fallback is visible (e.g. a polling query fires, or any context update occurs above the boundary), the sequence becomes:

1. Error thrown → fallback shown
2. Parent re-renders (new children object reference) → `componentDidUpdate` resets error state
3. Crashed children re-render → same error thrown again → `getDerivedStateFromError` sets `hasError: true`
4. Back to showing the fallback

This produces a visible flash/retry loop whenever a parent re-renders during an error state.

The canonical fix is to drive resets via a stable identity such as the current route pathname. The simplest approach is for callers to pass the current route as a `key` to the boundary — when the key changes on navigation, React unmounts and remounts the component, clearing error state without any `componentDidUpdate` logic. Alternatively, accept an explicit `resetKey` prop and compare that instead of `children`.

How can I resolve this? If you propose a fix, please make it concise.

[TerrifiedBug]

@greptile review