refactor: handle error results in sendStrictRequest

webui/.agents/skills/migrate-to-tanstack/SKILL.md

@@ -11,23 +11,26 @@ description: Migrate a data-fetching endpoint from the legacy ExtensionRegistryS
 
 `src/extension-registry-service.ts` (`ExtensionRegistryService`, plus `service.admin.*`) holds every server call. Legacy, un-migrated consumers call these methods straight from a component, passing an `AbortController` and relying on `fetch-retry`'s 10-attempt backoff inside `sendRequest`. That drags along per-component `AbortController` refs, `useEffect` fetch-on-mount wiring, and hand-rolled loading/error state.
 
-Migrated endpoints instead go through a `use*` hook wrapping `useQuery`/`useMutation`, and retries move to the shared query client (`src/query-client.ts`). Roughly half the service is migrated — grep before assuming either state.
+Migrated endpoints instead go through a `use*` hook wrapping `useQuery`/`useMutation`, retries move to the shared query client (`src/query-client.ts`), and error handling moves into the transport via `sendStrictRequest`. Roughly half the service is migrated — grep before assuming either state.
 
 ## Steps
 
 1. **Find every consumer** of the method you're migrating: `grep -rn "service\.<method>\|\.<method>(" src`. List them — you'll migrate all of them or a named subset.
 
-2. **Decide the retry scope — ask if unsure.** Ideally the service method flips from `sendRequest` (retriable) to `sendNonRetriableRequest`, handing retries to TanStack. Only do that when **every** consumer is moving to a hook — a legacy consumer still calling the method directly would silently lose its retry. If you're migrating just one of several consumers, either leave the method retriable (the query then double-retries, tolerated in the interim) or confirm scope with the user. When the request doesn't make the consumer scope clear, ask.
+2. **Decide the retry scope — ask if unsure.** Ideally the service method flips from `sendRequest` (retriable) to `sendStrictRequest`, handing retries to TanStack. Only do that when **every** consumer is moving to a hook — a legacy consumer still calling the method directly would silently lose its retry *and* start seeing rejections where it used to get a resolved error result. If you're migrating just one of several consumers, either leave the method retriable (the query then double-retries, tolerated in the interim) or confirm scope with the user. When the request doesn't make the consumer scope clear, ask.
 
 3. **Adjust the service method.**
+   - Switch it to `sendStrictRequest` and **drop `| ErrorResult` from its return type** (`Promise<Readonly<SuccessResult | ErrorResult>>` → `Promise<Readonly<SuccessResult>>`). The method now resolves with data or rejects; the hook needs no `isError` check, and consumers lose their `as SuccessResult` casts.
    - Query methods: keep the `AbortController` param — the hook passes `controllerFromSignal(signal)`.
    - Mutation methods: **drop the `AbortController` param** — we no longer abort writes.
 
 4. **Create the hook** — shape and naming per the `tanstack-query-conventions` skill. Co-locate it in the feature's folder first; move to `src/hooks/` only when a second place needs it. The hook returns the react-query result object **as-is**, never just `data` or a picked subset.
 
 5. **Update the consumers.** Replace the `AbortController` / `useEffect` / manual-state boilerplate with the hook, destructuring and renaming its result (`const { data: user, error: userError } = ...`; `const { mutateAsync, isPending } = ...`). Delete the dead boilerplate.
 
-6. **Finish per the `write-code` skill:** add or update tests (`write-tests`), add a changelog entry, and pass `yarn lint`.
+6. **Fix the tests that stubbed the old contract.** A spec stubbing the service method with `mockResolvedValue({ error: '…' })` was standing in for the old resolve-an-error-result behaviour — flip it to `mockRejectedValue({ error: '…' })`. `sendStrictRequest` itself is covered once, in `test/unit/server-request.spec.ts`; don't re-test it per endpoint.
+
+7. **Finish per the `write-code` skill:** add or update tests (`write-tests`), add a changelog entry, and pass `yarn lint`.
 
 ## Don't
 

webui/.agents/skills/tanstack-query-conventions/SKILL.md

@@ -18,15 +18,13 @@ export const useUserExtension = (target: UserExtensionTarget) => {
     const { service } = useContext(MainContext);
     return useQuery({
         queryKey: ['user', 'extension', target.namespace, target.extension],
-        queryFn: async ({ signal }) => {
-            const result = await service.getExtension(controllerFromSignal(signal), target.namespace, target.extension);
-            if (isError(result)) throw result; // let errors reach TanStack
-            return result;
-        }
+        queryFn: ({ signal }) =>
+            service.getExtension(controllerFromSignal(signal), target.namespace, target.extension)
     });
 };
 ```
 
+- The `queryFn` is a one-liner because the service method rejects on failure — see "Errors belong to the service" below. Never re-check `isError` in a hook.
 - `controllerFromSignal(signal)` (`query-client.ts`) bridges TanStack's `AbortSignal` to the `AbortController` the service expects — service signatures stay untouched, component-level `AbortController` refs go away.
 - `useQuery` forbids `undefined`; normalise a "no result" case to `null`.
 - Query keys are hierarchical arrays (`['admin', 'namespace', name]`). When a key is reused for invalidation, export a small `*Keys` helper next to the hook.
@@ -46,12 +44,28 @@ export const useCreateNamespace = () => {
 
 - **No `AbortController` / signal in mutations** — we don't abort writes anymore.
 - Mutations don't retry (TanStack's default `retry: 0`), which is correct for non-idempotent writes.
-- `throw` on an error result when the caller relies on a `catch` / `onError` path.
+- The `mutationFn` forwards the service call directly; the service rejects on failure, so `mutateAsync` callers get their `catch` / `onError` path for free.
 - Invalidate or remove affected queries in `onSuccess`.
 
+## Errors belong to the service, not the hook
+
+The registry answers some failures with a `200` carrying an `{ error: '…' }` body instead of a non-2xx status. `sendStrictRequest` (`server-request.ts`) is the single place that normalises this: it is non-retriable *and* rejects on such a body, so a migrated service method resolves with data or rejects — never both.
+
+```ts
+// extension-registry-service.ts — migrated methods
+async getNamespace(abortController: AbortController, name: string): Promise<Readonly<Namespace>> {
+    return sendStrictRequest({ abortController, credentials: true, endpoint: /* … */ });
+}
+```
+
+- A migrated method's return type **drops `| ErrorResult`** — that union is what forced the `isError` check on every caller.
+- Hooks therefore never contain `if (isError(result)) throw result`. If you find yourself writing one, the service method still needs migrating.
+- `sendRequest` / `sendNonRetriableRequest` keep resolving error bodies; legacy, un-migrated consumers check `isError` themselves. Don't change their behaviour.
+- Same rule in tests: stub a failing service method with `mockRejectedValue({ error: '…' })`, not `mockResolvedValue`.
+
 ## Retries and caching are owned by the shared client
 
-- One singleton `queryClient` (`query-client.ts`) retries network/5xx with backoff, never 4xx; 429s are waited out inside `sendRequest`. Migrated service methods use `sendNonRetriableRequest`, so this is the only retry layer.
+- One singleton `queryClient` (`query-client.ts`) retries network/5xx with backoff, never 4xx; 429s are waited out inside `sendRequest`. Migrated service methods use `sendStrictRequest` (fetch-retry disabled for non-429 responses), so TanStack should remain the only layer retrying network/5xx.
 - Defaults: `refetchOnWindowFocus: false`, `staleTime: 60s`. Override per hook only with reason — `staleTime: 0` / `gcTime: 0` when data must always be fresh (right after publish/delete), `retry: false` to let a 404 surface immediately.
 
 ## Options objects, not positional flags

webui/CHANGELOG.md

@@ -30,6 +30,7 @@ This change log covers only the frontend library (webui) of Open VSX.
 - Fix the admin dashboard Scan tab getting stuck on the loading spinner after switching tabs, even though the new tab's data had already loaded successfully
 - Fix the page jumping to the top whenever a menu, select or dialog opens.
 - Fix the extension detail page's download menu so each target-platform option is clickable across its whole row, not just its text: the option was an inline link nested inside a non-interactive menu item, rather than the menu item itself being the link
+- Fix `sendRequest` re-enabling fetch-retry's own retries for the request that follows a 429 wait, because the recursive call didn't forward the original `retry` flag. A `sendStrictRequest`/`sendNonRetriableRequest` call that hit a 429 could end up having its follow-up request retried twice - once by fetch-retry and once by the query client
 - Fix the create-namespace dialog acting on Enter when its button is disabled: an empty or over-long name was submitted anyway, and a held key sent the request more than once
 
 ### Dependencies