feat(skills): add react useEffect ban skill

scripts/sync/skills.json

@@ -28,6 +28,10 @@
       "name": "gh-setup",
       "source": "uinaf/agents"
     },
+    {
+      "name": "react-ban-use-effect",
+      "source": "uinaf/agents"
+    },
     {
       "name": "react-doctor",
       "source": "millionco/react-doctor"

skills/autoreview/SKILL.md

@@ -26,7 +26,7 @@ Use when:
 - Route security-audit suppression closeout through [references/troubleshooting.md](references/troubleshooting.md).
 - Do not push just to review. Push only when the user requested push/ship/PR update.
 
-For upstream provenance, use [references/upstream.md](references/upstream.md).
+For upstream provenance and uinaf tailoring notes, use [references/upstream.md](references/upstream.md).
 
 ## Helper Path
 

skills/autoreview/references/upstream.md

@@ -1,12 +1,13 @@
 # Upstream
 
-This skill borrows review-closeout ideas from Peter Steinberger's agent scripts.
+This skill credits Peter Steinberger's agent scripts as upstream inspiration.
 
 - Source: https://github.com/steipete/agent-scripts/tree/main/skills/autoreview
 - Rename commit: https://github.com/steipete/agent-scripts/commit/e6d810d5fe11c931f09e2bdd9b1d9da74fffd634
 - Applied upstream commit: `e6d810d`
 - License: [../LICENSE.upstream](../LICENSE.upstream)
 
-This repo preserves the upstream helper's engine support while using the
-`autoreview` skill name directly. Do not keep old `codex-review` skill names or
+This repo's version is tailored and structured for `uinaf/agents`: it preserves
+the upstream helper's engine support, uses the `autoreview` skill name directly,
+adds repo-specific closeout and rerun rules, and avoids old `codex-review`
 compatibility shims when borrowing upstream ideas.

skills/react-ban-use-effect/SKILL.md

@@ -0,0 +1,71 @@
+---
+name: react-ban-use-effect
+description: "Ban direct `useEffect` in React code. Use when writing, refactoring, reviewing, or migrating React components or hooks that import, call, add, or replace direct `useEffect`; when an agent reaches for effects for derived state, fetching, event reactions, resets, or external sync; or when adding lint/agent rules for a no-direct-useEffect policy. Do not use for ordinary React work with no effect smell, non-React code, or legitimate effect architecture outside React."
+---
+
+# React Ban useEffect
+
+Default stance: do not import or call `useEffect` directly in React components. Treat effects as an escape hatch for synchronizing with external systems, not as the default place to put render, event, data, or reset logic.
+
+## Start Here
+
+1. Search the touched React surface for direct `useEffect` imports and calls.
+2. Classify each effect by intent before editing:
+   - render-time derivation
+   - data fetching or server state
+   - response to a user action
+   - local state reset on identity change
+   - async UI, pending state, or request waterfall
+   - external-system synchronization
+3. Replace it with the narrowest declarative pattern from [references/replacements.md](references/replacements.md).
+4. For data, forms, external stores, or performance work, also check the stronger alternative map in [references/alternatives.md](references/alternatives.md).
+5. Keep behavior proof concrete: run the repo's lint/type/test gate, React Doctor diff scan when available, plus the smallest runtime or component check that exercises the changed path.
+
+## Replacement Ladder
+
+Use the highest applicable layer:
+
+1. render-time calculation
+2. server, loader, or framework data API
+3. server-state library such as TanStack Query, SWR, Relay, or Apollo
+4. event handler, form action, or mutation
+5. keyed component boundary
+6. `useSyncExternalStore`, approved mount sync, or a reviewed dependency-aware external-sync exception
+
+If none of these fit, stop and explain why the code truly needs an effect instead of adding direct `useEffect`.
+
+## Allowed Escape Hatches
+
+Prefer existing repo wrappers. If the repo has no standard, add mount-only sync close to shared React hooks:
+
+```tsx
+import { useEffect } from "react";
+
+export function useMountEffect(effect: () => void | (() => void)) {
+  // eslint-disable-next-line react-hooks/exhaustive-deps
+  useEffect(effect, []);
+}
+```
+
+Good mount uses are browser API setup, DOM focus/scroll integration, and third-party widget lifecycles. Prefer `useSyncExternalStore` for external stores or browser values that change over time. If an external system must resync when a prop or state value changes, require an explicit reviewed exception or dependency-aware wrapper that keeps dependencies honest. Do not use wrappers to hide dependency problems, fetch server state, copy props into state, or relay user actions.
+
+## Enforcement
+
+For new policy work, prefer the repo's existing ESLint shape. The usual rule is `no-restricted-imports` against `useEffect` from `react`, with the message pointing developers to declarative replacements and approved wrappers. Also block namespace calls such as `React.useEffect(...)` with `no-restricted-syntax` or a custom rule. Allow shared wrapper files as the only direct-import/call exceptions. If the repo already uses another lint shape, preserve that local convention.
+
+If the repo uses React Doctor, prefer its native rules for effect smells (`no-fetch-in-effect`, `no-derived-state-effect`) in addition to the import ban. Keep suppressions line-local and rule-specific.
+
+For reviews, treat new direct `useEffect` as a finding unless the diff also introduces a clear, reviewed exception. Ask for a replacement plan rather than dependency-array tuning.
+
+For upstream provenance and uinaf tailoring notes, use [references/upstream.md](references/upstream.md).
+
+## Boundaries
+
+- Scope the change to the touched path or requested migration slice.
+- Preserve the repo's existing data, lint, hook, and framework conventions.
+- Leave `useLayoutEffect`, framework lifecycle APIs, and non-React effect systems alone unless requested.
+- Use performance primitives only for real UI/perf evidence, React Doctor findings, or established repo patterns.
+
+## Sources
+
+Core sources and the broader alternatives bibliography live in [references/alternatives.md](references/alternatives.md).

skills/react-ban-use-effect/agents/openai.yaml

@@ -0,0 +1,20 @@
+interface:
+  display_name: "React Ban useEffect"
+  short_description: "Replace direct useEffect in React"
+  default_prompt: |
+    Use $react-ban-use-effect.
+
+    Goal: write, refactor, or review React code without direct useEffect.
+
+    Success criteria:
+    - Direct useEffect calls in the touched surface are removed or flagged.
+    - Each effect is classified by intent before replacement.
+    - Derived state, data fetching, user actions, resets, and mount-only sync use the repo's existing patterns.
+    - Any useMountEffect escape hatch is narrow and tied to an external system.
+
+    Constraints:
+    - Do not add a new data-fetching dependency without approval.
+    - Do not rewrite unrelated legacy effect usage.
+    - Preserve repo lint, hooks, data, and component conventions.
+
+    Output: lead with changed or reviewed behavior, then evidence. Include any remaining direct effect as an explicit exception or blocker.

skills/react-ban-use-effect/references/alternatives.md

@@ -0,0 +1,104 @@
+# Alternative Map
+
+Use this after the basic replacement guide when an effect is hiding a bigger React architecture problem.
+
+## Data Fetching and Server State
+
+Choose the highest available layer:
+
+1. **Server/framework data** for initial render: React Server Components, route loaders, server functions, framework fetch/cache APIs, or page/layout data APIs.
+2. **Client server-state library** for interactive, client-owned server state: TanStack Query, SWR, Relay, Apollo, or the repo's existing SDK hooks.
+3. **Local async state** only when there is no shared server-state layer and the slice is too small to justify adding one.
+
+TanStack Query replacements should usually include:
+
+- `queryOptions` or the repo's existing query factory pattern
+- query keys that encode every variable that changes the result
+- `enabled` for dependent queries instead of conditional hooks or effect flags
+- `select` for deriving server-state views
+- query functions that accept and pass `signal` to `fetch`
+- `useMutation` for writes, followed by invalidation or cache updates
+- `onMutate` cancellation and rollback context for optimistic updates
+- `placeholderData` for pagination instead of copying previous pages into local state
+
+Use regular `useQuery` instead of Suspense query hooks when dependent `enabled` gates, placeholder pagination, or cancellation semantics matter. Suspense is good for boundary-driven loading, not for every query replacement.
+
+## Forms, Writes, and User Actions
+
+If the work happens because the user clicked, typed, submitted, dragged, or navigated, keep it at that cause.
+
+Prefer:
+
+- event handlers for simple client-only work
+- framework actions or React form actions when the repo uses them
+- `useActionState` for action result/error state
+- `useFormStatus` for submit pending UI in a child component rendered inside the form
+- `useOptimistic` for local optimistic UI when updates are dispatched from an action or `startTransition`
+- TanStack Query `useMutation` for client server-state writes
+
+Avoid `setFlag(true)` followed by an effect that notices the flag. That splits cause from effect and creates stale-closure and dependency-array risk.
+
+## External Stores and Browser Values
+
+If the component needs a changing value from outside React, prefer `useSyncExternalStore` over an effect that subscribes and copies into local state.
+
+Good candidates:
+
+- external state stores without first-class React bindings
+- browser APIs such as online/offline status
+- cross-tab, media-query, or storage-backed values
+
+Keep `subscribe` stable outside the component when possible. `getSnapshot` must return a stable immutable snapshot while the external value has not changed.
+
+## Performance and Waterfalls
+
+Effects often hide performance work that should be explicit.
+
+Prioritize high-impact fixes first: waterfalls, bundle/server boundaries, and data ownership usually beat low-level memoization. Do not jump to `useMemo` while a request waterfall or client-only fetch is still dominating the path.
+
+Prefer:
+
+- parallelizing independent async work with `Promise.all`
+- Suspense boundaries to reveal independent async UI
+- server-side fetch and hydration for initial data in framework apps
+- dynamic import/code splitting for heavy optional client widgets
+- `useDeferredValue` for slow children that can lag behind urgent input
+- `useTransition` for non-urgent updates and navigations
+- lazy `useState` initialization for expensive initial local values
+- deriving cheap values inline instead of memoizing them
+
+Only apply performance primitives when they map to a real bottleneck, React Doctor diagnostic, or existing repo standard.
+
+## Verification and Diagnostics
+
+Run the repo's normal guardrails first. When React Doctor is available, add:
+
+```bash
+npx react-doctor@latest --verbose --diff
+```
+
+Use React Doctor's explanation mode for unclear diagnostics or suppressions:
+
+```bash
+npx react-doctor@latest --explain src/App.tsx:42
+```
+
+Relevant effect-adjacent diagnostics include derived-state effects, fetch-in-effect, missing cleanup, stale closure capture, async without cleanup, exhaustive dependencies, and giant components created by trying to centralize too much orchestration in one component.
+
+Suppress only when the code is intentionally unusual, and keep suppressions line-local and rule-specific.
+
+## Sources
+
+- Factory: https://factoryai.com/blog/why-we-banned-useeffect
+- Factory thread mirror: https://threadnavigator.com/thread/2033969062834045089/
+- React: https://react.dev/learn/you-might-not-need-an-effect
+- React: https://react.dev/learn/synchronizing-with-effects
+- React: https://react.dev/learn/separating-events-from-effects
+- React: https://react.dev/learn/removing-effect-dependencies
+- React: https://react.dev/reference/react/useSyncExternalStore
+- React: https://react.dev/reference/eslint-plugin-react-hooks/lints/set-state-in-effect
+- TanStack Query: https://tanstack.com/query/latest/docs/framework/react/overview
+- TanStack Query options: https://tanstack.com/query/latest/docs/framework/react/guides/query-options
+- Vercel React Best Practices: https://vercel.com/blog/introducing-react-best-practices
+- Vercel React Server Components: https://vercel.com/blog/understanding-react-server-components
+- React Doctor: https://www.react.doctor/docs

skills/react-ban-use-effect/references/replacements.md

@@ -0,0 +1,161 @@
+# Replacement Guide
+
+Use this when a React task touches direct `useEffect`. Classify the effect by intent; then replace it with the smallest pattern that preserves behavior.
+
+## Why This Rule Exists
+
+React's own guidance frames effects as an escape hatch for synchronizing with systems outside React. Factory's no-direct-useEffect rule applies the same idea as an agent guardrail: hidden synchronization through dependency arrays is easy for agents to add and hard for humans to trace later.
+
+Common failure modes:
+
+- dependency arrays hide coupling and drift during refactors
+- state update -> render -> effect -> state update loops
+- effect chains turn control flow into timing-sensitive synchronization
+- event-caused behavior loses the original event context
+- fetch effects recreate caching, retry, cancellation, and stale-data logic badly
+
+## 1. Derive During Render
+
+Use when an effect sets state from props, state, URL params, query params, feature flags, or selectors.
+
+Smells:
+
+- `useEffect(() => setX(f(y)), [y])`
+- state mirrors props or other state
+- an effect's only side effect is a setter
+
+Prefer:
+
+```tsx
+const filteredProducts = products.filter((product) => product.inStock);
+```
+
+For expensive pure calculations, use `useMemo` only after the repo's performance expectations justify it. React Compiler may remove some need for manual memoization, so do not add `useMemo` reflexively.
+
+## 2. Use Server or Server-State Data APIs
+
+Use when an effect fetches data and stores it in component state.
+
+Smells:
+
+- `fetch(...).then(setState)` inside an effect
+- manual `ignore`, `cancelled`, or stale response flags
+- hand-rolled cache, retry, revalidation, pagination, or dedupe logic
+
+Prefer, in repo order:
+
+- server components, route loaders/actions, server functions, or framework data APIs when the data is needed for initial render
+- TanStack Query, SWR, Relay, Apollo, or the repo's existing server-state layer for client-owned server state
+- a shared SDK hook already present in the codebase
+
+When using TanStack Query, prefer `queryOptions`, query keys that include variables, `enabled` for dependent queries, `select` for server-state derivation, AbortSignal-aware query functions, mutations with invalidation, and optimistic updates with cancellation/rollback where appropriate.
+
+Use regular `useQuery` when dependent `enabled` gates, placeholder pagination, or cancellation semantics matter. TanStack v5 Suspense query hooks intentionally do not support all regular-query options.
+
+Do not introduce a new dependency without explicit approval. If the repo has no server or server-state layer, state that the direct effect is a design gap and keep the smallest local fix honest.
+
+## 3. Move User-Caused Work to Handlers, Actions, or Mutations
+
+Use when an effect waits for state to change so it can run work caused by a click, form submit, keyboard action, drag, or route interaction.
+
+Smells:
+
+- `setShouldSubmit(true)` then an effect posts data
+- flags that exist only to trigger an effect
+- effect body contains navigation, toast, analytics, mutation, or imperative API calls caused by a known event
+
+Prefer:
+
+```tsx
+async function handleSubmit() {
+  await saveDraft(formState);
+  navigate("/drafts");
+}
+```
+
+In React 19 or framework code that supports form actions, prefer the repo's action pattern plus `useActionState`, `useFormStatus`, or `useOptimistic` for pending/error/optimistic UI. `useFormStatus` belongs in a child component rendered inside the form. Call `useOptimistic` updates from an action or transition. In TanStack Query code, use `useMutation` and invalidate or update the relevant query keys. Share duplicated behavior by extracting a function that event handlers call directly. Do not put the shared behavior in an effect just to avoid a helper.
+
+## 4. Reset with Keys or Derived Selection
+
+Use when an effect clears or reloads local state after an identity prop changes.
+
+Smells:
+
+- `useEffect(() => setComment(""), [userId])`
+- "load this entity from scratch when ID changes"
+- nested children also need reset behavior
+
+Prefer a keyed boundary:
+
+```tsx
+function ProfilePage({ userId }: { userId: string }) {
+  return <Profile key={userId} userId={userId} />;
+}
+```
+
+When only selection needs adjusting, prefer storing stable IDs and deriving selected objects during render. Updating state during render is a last resort and must be tightly guarded.
+
+## 5. Synchronize External Systems
+
+Use `useMountEffect` only when the work is naturally setup on mount and cleanup on unmount. For external systems that must resync when props or state change, use a reviewed explicit exception or dependency-aware wrapper instead of pretending the work is mount-only.
+
+Good candidates:
+
+- subscribing to a browser API or external store not covered by `useSyncExternalStore`
+- initializing and disposing a third-party widget
+- focusing, scrolling, measuring, or integrating with imperative DOM APIs after mount
+- starting and cleaning up a process tied to the component's presence
+
+Prefer `useSyncExternalStore` when the component reads a changing external store or browser value. That gives React a subscription and snapshot contract instead of a hand-rolled effect that copies external state into local state.
+
+Before using it, try conditional mounting:
+
+```tsx
+function VideoPlayerContainer({ isReady }: { isReady: boolean }) {
+  if (!isReady) return null;
+  return <VideoPlayer />;
+}
+
+function VideoPlayer() {
+  useMountEffect(() => {
+    const player = startPlayback();
+    return () => player.stop();
+  });
+}
+```
+
+This keeps preconditions in the parent and lifecycle in the child.
+
+## 6. Keep Expensive UI Responsive Without Effects
+
+Use when an effect exists only to debounce, delay, copy, or stage render work after input changes.
+
+Smells:
+
+- state mirrors input so a later effect can update a slow list or chart
+- effect-managed loading flags for non-urgent UI updates
+- sequential awaits for independent data before render
+
+Prefer:
+
+- `useDeferredValue` when a slow child can lag behind an urgent input
+- `useTransition` or framework navigation pending state for non-urgent updates
+- `Suspense` boundaries for async UI that can stream or reveal independently
+- `Promise.all` or framework parallel data APIs for independent requests
+- dynamic import or route/code splitting for heavy optional client components
+
+Use these only when the changed path has visible latency, React Doctor findings, profiler evidence, or an existing repo convention. Do not scatter performance hooks as decoration.
+
+## Review Checklist
+
+- No direct `useEffect` import remains in touched React components.
+- No `React.useEffect(...)` namespace calls remain in touched React components.
+- Shared wrapper files are the only direct `useEffect` import/call exceptions.
+- Any wrapper use synchronizes with a real external system and has cleanup when setup allocates work.
+- Data fetching follows the repo's existing server-state path.
+- Client query code uses keys, cancellation, dependent-query gates, mutation invalidation, and optimistic rollback where relevant.
+- User-caused work stays in event handlers.
+- Forms/mutations use action, mutation, pending, and optimistic primitives instead of state flags plus effects.
+- External subscriptions use `useSyncExternalStore` when they expose changing values.
+- Reset semantics use `key`, component boundaries, or render-time derivation.
+- Verification includes lint/type/test, React Doctor diff scan when available, plus the changed UI, hook, or component behavior.