Fix high-risk React Doctor findings

[arjunkomath]

Summary

• register the control-plane update and agent-upgrade timeout cron functions with the Inngest route, with a regression test
• reset serverless form drafts when persisted settings change without collapsing the configuration section
• surface failed server and GitHub repository requests instead of treating error payloads as successful data
• always clear auth submission loading states after rejected requests while keeping exception details private
• add accessible names to the flagged icon-only controls

React Doctor 0.9.2 warnings dropped from 147 to 135. The remaining findings are lower-risk heuristics or require separate performance/design work.

Validation

• pnpm test — 46 files, 307 tests passed
• tsc --noEmit
• biome check on changed files
• React Doctor 0.9.2 full scan — 0 errors, 135 warnings, all 323 files analyzed
• Oracle review — approved after addressing three medium-severity review findings

Out of scope

The pnpm trustPolicy warning was not changed: enabling no-downgrade rejects four existing lockfile entries, so that hardening needs a separate dependency provenance review rather than a lint-driven lockfile rewrite.

[github-actions]

React Doctor found no issues. 🎉

⚠️ Warning: .github/workflows/react-doctor.yml is configured incorrectly. See below to fix.

React Doctor compares against main to report only the issues this pull request introduces. This run couldn't complete that comparison (usually a shallow CI checkout with no merge base), so it listed every issue in the changed files, including ones that already existed on main.

Add fetch-depth: 0 to the actions/checkout step in .github/workflows/react-doctor.yml so the checkout includes the history React Doctor needs:

 jobs:
   react-doctor:
     steps:
       - uses: actions/checkout@v5
+        with:
+          fetch-depth: 0

       - uses: millionco/react-doctor@v2

To silence this warning, set silence-missing-baseline-warning: true on the React Doctor action.

_{Reviewed by React Doctor for commit c119375.}

[techulus-agent]

Review

Overview

Ten files, mostly React Doctor remediation: registering two orphaned Inngest crons, switching two SWR call sites to the shared throwing fetcher and rendering error states, wrapping three auth submit handlers in try/finally, re-keying the serverless editor, and adding aria-labels to icon-only buttons.

The highest-value change here is not a lint fix: controlPlaneUpdateCheck and agentUpgradeTimeoutCheck were exported from lib/inngest/functions but never passed to serve(), so both crons have been dead. Good catch.

Correctness

• app/api/inngest/route.ts — correct; both symbols exist in lib/inngest/functions/index.ts and are now registered.
• replicas-section.tsx — the old inline fetcher called res.json() unconditionally, so a 500 HTML/JSON error body would flow into servers.map(...) and either throw inside SWR or render an empty list as "no servers available". Routing through lib/fetcher plus the explicit error Empty state is the right fix, and AlertTriangle was already imported.
• github-repo-selector.tsx — same class of fix. One dead condition though: in publicRepoFromSearch, if (!error && alreadyInList) can never differ from if (alreadyInList), because when error is set data is undefined and repos is EMPTY_REPOS, making alreadyInList always false. Same for the !error && guards on the two empty-state branches — hasInstallations is already false and filteredRepos already empty on error. They're harmless but they read as if they're guarding something. I'd drop error from the useMemo deps and the guards, keeping only the new role="alert" block.
• isLoading && !data — redundant: SWR's isLoading is already !data && !error. Not wrong, just noise.
• Auth pages — try/catch/finally correctly guarantees the button un-sticks on a rejected promise. Behaviour on the success and twoFactorRedirect paths is unchanged (setLoading(false) ran before the router.push previously too). Swallowing the exception and showing a generic message is the right call for auth surfaces; no info leak.
• serverless-section.tsx — splitting into a keyed inner editor with keepMounted on ConfigSection is a sound way to reset drafts without collapsing the section, and matches the existing keepMounted usage in networking-section.tsx. Worth being explicit that this means an in-flight unsaved edit is discarded the moment a refresh changes any of the four keyed fields; that's the intended semantics, but it's the kind of thing that deserves a one-line comment next to settingsKey rather than only on the useState.

Test coverage

tests/inngest-route.test.ts guards the specific regression, but only the specific one. Because the whole @/lib/inngest/functions module is replaced by a hand-maintained object, the next function that gets exported and not registered will slip through exactly the same way — the mock simply won't have it, route.ts will import undefined, and arrayContaining on two unrelated entries still passes.

Stronger and less maintenance for the same effort: mock the module but assert the registered array covers every mocked export, e.g.

expect(options?.functions).toEqual(
  expect.arrayContaining(Object.values(mocks.functions)),
);
expect(options?.functions).toHaveLength(Object.keys(mocks.functions).length);

That turns the hand-maintained list into the contract instead of a prop, and fails loudly when route.ts drifts.

Style / conventions

Consistent with the codebase — tabs, memo retained, shared fetcher reused rather than re-inlined. The oxlint-disable-next-line carries a real justification, which is the right way to use one.

Security & performance

Nothing concerning. lib/fetcher uses cache: "no-store", so the replicas/repos requests are no longer subject to whatever default caching fetch applied — that's a behavioural change in the right direction for these endpoints, but flagging it since it wasn't in the summary. keepMounted on the serverless section means the editor mounts even while collapsed; it's a handful of useStates, negligible.

Verdict

Approve with minor follow-ups: drop the no-op error guards in the repo selector, and consider tightening the Inngest route test so it catches the next unregistered function rather than only these two.

[arjunkomath]

Arjun addressed the review feedback in c119375:

• tightened the Inngest route test to require every mocked function exactly once
• removed the redundant !data check from the SWR loading branch
• documented that persisted serverless changes intentionally replace stale drafts

The error guards in the repository selector remain intentionally: SWR can expose stale data together with a revalidation error. In that state, public URL selection must remain available, stale connected repositories should remain visible, and the normal empty-state copy should not appear beside the error.

Focused test, TypeScript, and Biome checks pass.