fix(api): stop synchronous route work from stalling the whole server

[Pfannkuchensack]

Summary

Kind: fix + perf (backend, frontend, dependencies)

Users reported the backend becoming unresponsive for minutes at a time. The cause was not one bug but a chain, and reproducing it needed three conditions at once — a large library, an active gallery search, and a running generation — which is why it resisted diagnosis.

The mechanism. The gallery list and name routes were declared async def while calling synchronous SQLite services. That work therefore ran on the event loop, so for its entire duration the process served no other HTTP request and delivered no socket.io event. Users experienced this as the whole application freezing mid-generation rather than as a slow gallery. The same defect was present in 167 further route handlers.

Measured on a seeded 200k-image, 1.7 GB database (1.56 GB of it metadata blobs). The metric is the latency of an unrelated trivial request issued while a gallery name query is in flight — the queries themselves are not made faster, the loop is freed:

Scenario	before	after
name query with search term	1662 ms	4 ms
name query without search term	2298 ms	375 ms

Sample counts tell the same story more plainly: in the two-second window the probe returned 1 response before and 28 after.

What each commit does:

• fix(api): run gallery and search routes off the event loop — eight gallery/search routes declared def so FastAPI dispatches them to the threadpool.
• perf(gallery): add a flat item-names endpoint — the name list wrapped every entry in an object carrying a kind discriminator. Building those models cost 820 ms of the 2225 ms service call at 200k items, and every consumer discarded the field, re-deriving the kind from the file extension. A new GET /v1/gallery/item_names returns a flat list; an optional created_date filter subsumes the separate by-date virtual-board route, so regular boards and virtual dates now share one endpoint, one cache and one query-args selector instead of a skipToken branch duplicated in four places. Result: 2.51 s → 1.57 s, 8.48 MB → 3.85 MB. Five legacy name endpoints keep working and are marked deprecated=True — external integrations still call them.
• perf(api): stop gzipping responses that are already compressed — Starlette's GZipMiddleware compresses every content type except text/event-stream. A 3 MB PNG cost 52 ms of event-loop time to gzip and came back at 3.01 MB, larger than it went in; a 12 MB PNG cost 210 ms. With auto-switch on, that lands after every generated image. Lowering compresslevel is not an alternative — on incompressible input level 1 costs 51 ms against level 9's 52 ms.
• build: unpin FastAPI and move to 0.141.1 — see Merge Plan.
• perf(api): run every synchronous route handler off the event loop — the remaining 167 handlers, identified by AST rather than by hand.
• feat(queue): add lightweight item summaries endpoint + fix(api): close the sync-sweep races and wire up the queue summary route — see below; the endpoint originates from @JPPhoto's optimize-queue-return-data and now has its consumer.
• fix(queue): bound and chunk the id list on the queue summary route — the route expanded every client-supplied id into one SQLite bind with no batch limit, so 32 766 ids raised OperationalError: too many SQL variables and came back as an HTTP 500. Bounded at 1000 ids at the API, and chunked at 900 binds in the SQLite layer so no caller — including internal ones not covered by the route bound — can reach the ceiling. 900 stays under the 999-variable limit of SQLite builds predating 3.32, not just the 32 766 of current ones.
• fix(api): close the two check-then-act races the sync sweep opened — an async def body containing no await cannot be interleaved with another request, because the event loop has no point at which to switch. Two handlers relied on that. POST /auth/setup did has_admin() then create_admin() in separate transactions, so two concurrent requests both saw no admin and both created one — the loser ending up with a persistent admin account instead of the intended 400. Custom node install, uninstall and reload all mutate the same directory, sys.modules and invocation registry; interleaved, a failed install's cleanup rmtreed the directory a concurrent install had just cloned into.
• fix(api): finish the review's non-blocking list — require_admin / require_admin_or_default back to async def (they only read a field off already-resolved token data, so def bought a threadpool round-trip and nothing else); the AST guard widened to see handlers below module level and to stop counting awaits inside nested closures; model conversion and HF-token writes serialized explicitly; and the threadpool's own bound documented.

The queue list, measured

The queue list fetched full SessionQueueItem objects carrying the complete GraphExecutionState for every visible row. It now renders from SessionQueueItemSummary, and the full item is fetched only when a row is expanded. Measured in the running app against a 396-item queue, same backend on both sides, identical scenario (page load → queue tab → scroll to 60 %):

	requests	payload (gzip, 30 items)	server time
before	62	262 KB	60 ms
after	2	1.3 KB	4 ms

The request count collapses because the old path was self-amplifying: the range hook re-asks which ids are uncached on every range event, and at ~60 ms per response the cache had not filled yet, so overlapping fetches piled up. The per-item summary query provides the same cache tags as getQueueItem, so every existing invalidation path — socket status events, cancel, delete, retry — covers the list rows with nothing to wire up.

Two sanitizers became one generic function over a single redaction table. The summary and the full item are two projections of the same row, and a field stripped from the list but left on the detail view is leaked anyway; a test walks the intersection of both models and asserts they redact it identically. device is deliberately not redacted in either — it names the instance's GPU rather than anything about the other user's work, and the list has always shown it.

One side effect worth knowing: items_by_ids silently skips items it cannot deserialize, so a queue item whose graph references a node type this build no longer registers left its row permanently blank. Summaries never touch the graph, so the row now renders and only the expanded detail is affected — where a failed fetch is now reported as an error instead of reading as "Loading" forever.

POST /v1/queue/{queue_id}/items_by_ids is unchanged and still served; the UI no longer calls it.

Related Issues / Discussions

• Adopts the queue-summaries work from @JPPhoto's optimize-queue-return-data branch.
• The auth-guard repair protects the invariant introduced after GH [Security] InvokeAI: unauthenticated arbitrary-directory enumeration via /models/scan_folder in multi-user mode #9365.
• Touches gallery_default.py, which PR perf(db): streamline gallery membership queries #9385 also rewrites — see Merge Plan.
• Interacts with fix(auth): revoke privileges immediately on role change, deactivation, or deletion #9360 (@lstein) on the last-administrator invariant — see Merge Plan.

QA Instructions

Automated. Guards that were each verified to fail before their fix and pass after:

• tests/app/routers/test_no_blocking_async_routes.py parses every router module and fails if any handler is async def without awaiting. It now walks the whole module rather than only its top level, so a handler registered from inside a factory function or an if block is seen too, and it no longer counts awaits inside nested closures — a handler could otherwise pass by defining an async helper it never awaits. Both properties have their own tests.
• tests/app/routers/test_event_loop_blocking.py stubs a service call to block for one second and asserts an unrelated route still answers during it. Six routes covered, GET and POST.
• tests/app/routers/test_session_queue_item_id_limits.py posts 32 767 ids and expects a 422 with no database work attempted; without the bound it returns 200.
• tests/app/services/session_queue/test_session_queue_status_user_scoping.py sizes its id list off the limit the running SQLite build actually enforces; unchunked it reproduces OperationalError: too many SQL variables.
• tests/app/services/users/test_user_service.py races two threads through a barrier into create_admin; without the fix both succeed and the instance ends up with two administrators.
• tests/app/routers/test_custom_nodes.py races two installs of the same pack so the loser's cleanup runs after the winner has written its files; without the lock the winner's directory is deleted.
• tests/app/routers/test_model_manager.py covers the 409 a second concurrent conversion gets, and that a failed conversion releases the lock rather than wedging the endpoint for the process's lifetime.

Plus tests/app/routers/test_gallery_item_names.py (7 tests, two of which compare the new endpoint against the deprecated one so ordering and counts cannot drift while both are served), tests/app/api/test_gzip_content_types.py (14 tests, including one asserting the real app has the middleware wired) and tests/app/routers/test_session_queue_sanitization.py (the summary/full-item redaction equivalence).

Verified locally: 713 tests across tests/app/routers/ and tests/app/api/, 1792 frontend tests, ruff / tsc / eslint / knip / dpdm / prettier clean. Nine pre-existing failures in test_download_queue, test_model_install and test_load_api are network-dependent and reproduce identically on an unmodified tree.

Manual, to see the stall fix. Needs a large library — a few hundred MB of image metadata is enough; the effect scales with SUM(LENGTH(metadata)) FROM images.

1. Start a batch of ~10 images with auto-switch enabled.
2. While it runs, type a term into the gallery search box.
3. Before: the UI freezes — progress bar stops, thumbnails stop loading, the queue stops updating. After: the search itself is still slow, but everything else keeps responding.

To measure rather than eyeball it: fire GET /api/v1/gallery/item_names?search_term=… and poll GET /api/v1/app/version concurrently, recording the latency of the second. Note that a benchmark of the search endpoint alone shows no improvement — that is the wrong instrument here.

Manual, to check the queue list. Open the Queue tab with a few hundred items and watch the network panel: it should issue POST item_summaries_by_ids and no items_by_ids, including while scrolling. Expanding a row issues one GET /v1/queue/default/i/{id} — that request is the intended trade. Rows for other users' items must still show redacted identity but a visible GPU column.

Deprecated routes. GET /v1/gallery/items/names, /v1/images/names, /v1/videos/names and both /v1/virtual_boards/by_date/{date}/*_names still return their original shapes; only the OpenAPI deprecated flag changed.

Media responses no longer carry Content-Encoding: gzip. Confirm images and videos still load, including behind a reverse proxy.

Merge Plan

The FastAPI bump needs attention. pyproject.toml moves from fastapi==0.118.3 to >=0.141.1,<0.142; contributors must re-sync (uv sync) after pulling. The old pin carried a comment guessing the OpenAPI crash on 0.119 was "probably Invoke's [bug], because we are doing something unusual with AnyInvocation". It was not: fastapi/_compat/v2.py assumed every field mapping carries a $ref. Upstream fixed it in 0.124.0 with no change needed here.

Two later FastAPI changes break silently and are handled — both are worth a reviewer's attention:

• 0.130 emits contentMediaType instead of format: binary for file uploads. typegen.js mapped only the latter to Blob, so upload call sites would have started typing their File argument as string. Caught only because tsc happened to fail.
• 0.141 keeps an included router as a single node in app.routes instead of copying its routes into it. The default-deny auth guard walked app.routes for APIRoute instances and found 2 of 197 — passing while inspecting almost nothing. It now walks iter_route_contexts (the traversal FastAPI's own OpenAPI generation uses) and asserts a floor on the route count so going blind fails loudly. Only the allowlist-staleness assertion caught this; "fixing" it by trimming PUBLIC_ROUTES would have killed the guard.

Ordering against #9360. The last-administrator invariant in update_user / delete_user is a TOCTOU on main today, but reachable only from non-HTTP paths. This PR makes it reachable from two concurrent HTTP requests, and zero administrators means has_admin() is false, which reopens POST /auth/setup unauthenticated. The fix belongs to #9360 (@lstein) and is deliberately not duplicated here. If this PR merges first, that window is open in main until #9360 follows.

Conflicts with PR #9385, which rewrites _build_half in gallery_default.py. This PR adds a shared _query_name_rows in the same file. Whichever merges second needs a manual pass. Unrelated note for that PR's own review: it introduces INDEXED BY hints into the shared query builder, which is SQLite-only syntax.

Not in scope, deliberately: the metadata LIKE '%…%' full scan (six sites, unindexable by construction) is being addressed differently in v7; the SQLite single-connection/global-lock design is tracked separately; making compresslevel configurable is written up with measurements but not implemented — level 9 costs 5.5× the CPU of level 1 for 0.4 percentage points of output size; the stale old_is_public in the workflow-updated event is cosmetic and would need workflow_records.update() to return the previous row.

Suggested split: if the FastAPI commit would rather be reviewed on its own, it is self-contained and the auth-guard finding deserves its own title.

Checklist

• The PR has a short but descriptive title, suitable for a changelog
• Tests added / updated (if applicable)
• ❗Changes to a redux slice have a corresponding migration — n/a, no slice shape changed
• Documentation added / updated (if applicable) — docs/contributing/blocking-work-in-api-routes, extended with the threadpool's 40-token bound
• Updated What's New copy (if doing a release after this PR)

[keturn]

I was just bumbling around yesterday trying to figure out Invoke's approach to concurrency, as I realized the FastAPI handlers are async but most of our code (including the BaseInvocation API) very much is not.

perf(api): run every synchronous route handler off the event loop

Wait, what? This is just removing "async" from 167 existing "async def"? Is that…?

Okay, the explanation for how this works is at FastAPI, Concurrency and async / await: FastAPI automatically kicks any non-async function to a thread pool.

In an async-first application, "parse every router module and fail if any handler is async def without awaiting" is really not the heuristic you want to use. You want to be async by default and only introduce the complexity of thread switching if you're doing a blocking operation. Especially in Python, where threads don't actually get you multi-core parallel execution.

The aforementioned FastAPI docs back me up on that:

In these cases, it's better to use async def unless your path operation functions use code that performs blocking I/O.

However… in a codebase where most of the code is written synchronously and people aren't used to thinking about whether they're about to call a blocking function?

As a former Twisted developer, it hurts me to even think it, but no-async-by-default might be the right call, I guess?

We're not trying to optimize requests/second throughput, as the number of users and frequency of requests on any one InvokeAI sever is actually pretty low. We're trying to reduce the chances of someone accidentally making a commit that blocks the server's event loop.

So. I can't say I'm a fan, but I guess I understand why you might want to do it that way.

I still feel tempted to argue for some kind of "it doesn't use await but that doesn't mean it's blocking" escape hatch, but I'm not confident I could really back it up with benchmarks. Probably the only high-volume route we have is whatever gets used for serving thumbnails, and hopefully that's backed by some StaticFile handler. (I guess that might not be true if that backend is pluggable to S3 storage or whatever.)

[keturn]

I guess the hazard of "kick it to the thread pool by default" is then all your code has to be thread-safe by default. Which I don't think is an easier/safer assumption to make than knowing if your code is blocking.

i.e. does sending all sqlite-related activity to a general-purpose thread-pool mean we have multiple threads opening and writing to the same sqlite database at once? Is that a thing we can assume it's safe to do?

[Pfannkuchensack]

i.e. does sending all sqlite-related activity to a general-purpose thread-pool mean we have multiple threads opening and writing to the same sqlite database at once? Is that a thing we can assume it's safe to do?

Worth answering precisely, because the premise is slightly off in a way that matters.

We never open a connection per thread. There is exactly one sqlite3.Connection, created once at
startup. Every runtime access goes through SqliteDatabase.transaction(), which holds a process-wide
threading.RLock for the duration. The only code touching _conn outside it is startup-only (PRAGMAs,
VACUUM, the migrator), before the server accepts requests. So access is serialized by the lock, not
merely permitted by SQLite.

And this predates the PR. Multiple threads already write to that database on every generation:
SessionProcessor runs _process in a thread per worker — several concurrently with multi-GPU — calling
set_queue_item_session / complete_queue_item; DownloadQueueService and ModelInstallService have
their own threads; SlidingWindowTokenMiddleware already did its user lookup via run_in_threadpool.
The RLock around a single connection is the design that makes that work, and it has been carrying that
load for a long time. This PR doesn't introduce multi-threaded DB access — it stops the request layer
from pretending it's exempt.

What does change, and I don't want to gloss over it: two async def handlers that never awaited were
implicitly serialized against each other on the loop. As def they can genuinely overlap. Each
transaction() is still atomic; a handler spanning two transactions is not — but it never was, since
the worker threads could always interleave between them. What's gone is atomicity relative to other
handlers, which was an accident of the dispatch model rather than something designed for. (I checked
for module-level mutable state in the routers, which is what loop-serialization would have protected:
there is none.) So the residual risk is a handler doing multi-step read-modify-write that relied on no
other handler running in between — if you know of one, worth flagging.

On the general point, I'd argue the two assumptions aren't symmetric. "Does this handler block?" is
mechanically checkable, and the PR adds test_no_blocking_async_routes.py which parses every router and
fails if a handler is async def without awaiting — before that existed, the assumption was violated 167
times unnoticed, and the cost was the server going unresponsive for minutes. "Is this thread-safe?" isn't
checkable, but it was already required by the threads above. The PR doesn't add that requirement; it
removes a false belief that the request layer was exempt from it.

[Pfannkuchensack]

I was just bumbling around yesterday trying to figure out Invoke's approach to concurrency, as I realized the FastAPI handlers are async but most of our code (including the BaseInvocation API) very much is not.

perf(api): run every synchronous route handler off the event loop

Wait, what? This is just removing "async" from 167 existing "async def"? Is that…?

Okay, the explanation for how this works is at FastAPI, Concurrency and async / await: FastAPI automatically kicks any non-async function to a thread pool.

In an async-first application, "parse every router module and fail if any handler is async def without awaiting" is really not the heuristic you want to use. You want to be async by default and only introduce the complexity of thread switching if you're doing a blocking operation. Especially in Python, where threads don't actually get you multi-core parallel execution.

The aforementioned FastAPI docs back me up on that:

In these cases, it's better to use async def unless your path operation functions use code that performs blocking I/O.

However… in a codebase where most of the code is written synchronously and people aren't used to thinking about whether they're about to call a blocking function?

As a former Twisted developer, it hurts me to even think it, but no-async-by-default might be the right call, I guess?

We're not trying to optimize requests/second throughput, as the number of users and frequency of requests on any one InvokeAI sever is actually pretty low. We're trying to reduce the chances of someone accidentally making a commit that blocks the server's event loop.

So. I can't say I'm a fan, but I guess I understand why you might want to do it that way.

I still feel tempted to argue for some kind of "it doesn't use await but that doesn't mean it's blocking" escape hatch, but I'm not confident I could really back it up with benchmarks. Probably the only high-volume route we have is whatever gets used for serving thumbnails, and hopefully that's backed by some StaticFile handler. (I guess that might not be true if that backend is pluggable to S3 storage or whatever.)

Fair challenge, and the FastAPI docs quote is right — but note its condition: "unless your path
operation functions use code that performs blocking I/O." That's not an edge case here, it's
essentially every handler. The rule lands on def for us by its own terms rather than in spite of them.

Two things I could measure rather than argue:

The thread hop costs ~156 µs. Trivial handler, in-process, 600 requests: 0.320 ms as async def,
0.477 ms as def. Real, but it buys back multi-second event-loop stalls — the gallery name query was
holding the loop for 1.7–2.3 s per request before this.

The high-volume route you're hoping is a StaticFile isn't one. get_image_thumbnail is a regular
handler doing open(path, "rb"); StaticFiles only serves the UI bundle and /static. So the hottest
route in the app is precisely one that performs blocking I/O — and as you guessed, with a pluggable S3
backend it becomes blocking network I/O, which is worse. That's the one route where I'd most want it
off the loop.

One clarification on the GIL point: for pure-Python CPU work you're right that threads buy nothing. But
the calls we're moving release it — sqlite3.execute drops the GIL for the duration of the query, as
does file I/O. So for this particular workload the threads do overlap rather than just take turns.

On the escape hatch — you're right that "doesn't await" ≠ "doesn't block", and the guard is deliberately
blunt. I'd happily make it an allowlist with a justification per entry, the same shape as PUBLIC_ROUTES
in test_model_manager_authorization.py: async def allowed, but only as a conscious, reviewed
exception rather than the default. That keeps the property you want (async where it genuinely helps)
without going back to a rule nobody can check. If that sounds reasonable I'll add it — I just didn't
want to build the exception mechanism before there was a concrete handler that needed it.

[lstein]

Reviewed adversarially at 83683696e5 — the goal was to assume the change is broken and prove it, not to evaluate it. The core diagnosis and the mechanism fix are correct and unusually well evidenced; the write-up made the review much faster than it would otherwise have been. Two findings, both reproduced rather than argued.

---

1. POST /api/v1/auth/setup loses its atomicity → duplicate-admin race

setup_admin went async def → def. An async def handler whose body contains no await cannot yield, so has_admin() → create_admin() was an atomic check-then-act. Dispatched to the threadpool, it is not.

Triggering sequence: multiuser mode, no admin yet (the endpoint is unauthenticated by necessity in this window). Two concurrent POST /api/v1/auth/setup with different emails. Both threads run has_admin(), both see no admin, both create_admin(). Against the real app, with a has_admin that returns its result after a delay (modelling the SELECT returning and the thread being descheduled before the INSERT):

statuses: [200, 200]
admins created: ['attacker0@x.com', 'attacker1@x.com']

Minimal A/B confirming this is the change and not something pre-existing — same body, same two concurrent requests, only the def/async def keyword differs:

async def -> [True, False] admins: 1     # main
def       -> [True, True]  admins: 2     # this PR

users_default.create_admin re-checks has_admin(), which does not close the window — both checks precede the INSERT. Before: a racer got a deterministic 400. Now: a persistent admin account. Fix is cheap — a threading.Lock around check+create inside UserService, or a partial UNIQUE index / INSERT … WHERE NOT EXISTS so the database arbitrates.

This is the general hazard of the sweep, and setup_admin is the one place I found where it is security-relevant. Worth a scan of the other 166 for check-then-act on process-global state before merge — the AST guard can't see this class of problem.

2. The queue-summaries commit has no consumer

POST /v1/queue/{queue_id}/item_summaries_by_ids and SessionQueueItemSummary appear only in the generated schema.ts. services/api/endpoints/queue.ts still calls getQueueItemDTOsByItemIds → items_by_ids, the full-item route. So the commit's measured "~66 ms of event-loop time plus ~1.8 MB per request" is not saved by this PR.

What it does add: public API surface, a new schema type, a second sanitizer that must stay in sync with sanitize_queue_item_for_user (they already diverge — the summary sanitizer redacts device, the full one doesn't), and a route to maintain. Either wire queue.ts to it, or drop the commit and land it with its consumer.

---

Non-blocking

• Concurrent custom-node install can delete the winner's directory. install_custom_node_pack does if target_dir.exists(): return … git clone … except Exception: shutil.rmtree(target_dir). Two concurrent POSTs for the same source both pass the exists check; the loser's failed clone rmtrees the directory the winner is cloning into. Admin-only and self-inflicted, but previously impossible. _load_node_pack also mutates the global invocation registry from two threads now.
• require_admin / require_admin_or_default do no blocking work — they only read current_user.is_admin. Declaring them def buys a threadpool round-trip per admin request and nothing else. (get_current_user / get_current_user_or_default / get_current_media_user_or_default genuinely do a users.get, so those conversions are right; note that in single-user mode get_current_user_or_default also returns before touching the DB.)
• The stall shifts rather than vanishes past 40 in-flight blocking requests. anyio's default thread limiter is 40 tokens, and the SQLite layer is one connection behind a process-wide RLock, so 40 concurrent slow searches occupy every worker and later requests — including the now-sync auth dependency — queue behind them. The probe route in test_event_loop_blocking.py (/api/v1/app/version) has no auth dependency, so it wouldn't surface this. Worth a sentence in blocking-work-in-api-routes, which currently says "keeping everything else responsive" without the bound.
• AST guard scope. test_no_blocking_async_routes.py walks only module-top-level nodes of api/routers/*.py, and _awaits_something counts an await that appears inside a nested closure. Fine for a guard — just narrower than "every handler including ones written later" implies.

Attacks that failed

Recording these so the clean areas are legible as checked, not skipped:

• GZip subclass — I expected the super()-then-widen ordering to be wrong. It isn't: Starlette buffers http.response.start and only reads content_type_is_excluded when the first body message arrives, so widening after the base class has set it is sound, and the and not self.content_type_is_excluded guard correctly prevents un-setting the text/event-stream exclusion. Verified end-to-end against a standalone app: png/webp/jpeg/mp4/zip/octet-stream/event-stream pass through, json/html/css/js/svg compress, Vary: Accept-Encoding still added to compressed responses, a StreamingResponse keeps its explicit Content-Length when excluded, identity path unchanged. One consequence worth knowing: a response with no content-type is now uncompressed.
• Events from worker threads — FastAPIEventService.dispatch uses call_soon_threadsafe, so socket.io delivery from threaded handlers is safe.
• DB access outside the lock — the only direct _conn use outside SqliteDatabase.transaction() is the startup-only migrator, so the doc's "single connection behind a process-wide lock" claim holds. That is the precise answer to @keturn's question above.
• Deadlock — no converted handler blocks on a future the event loop must complete; there is no cycle between the threadpool and the loop.
• created_date injection — parameterized (AND DATE(x.created_at) = ?).
• Frontend cache regression — providesTags on the unified endpoint is a strict superset of what the two old endpoints provided (the virtual-date case gains the hashed GalleryItemNameList id), so invalidation can't regress. The selector correctly drops board_id for virtual ids and sends is_intermediate: false, matching the deprecated by-date route's hardcoded False. No stale references to the removed endpoint remain.
• Build hygiene — openapi.json is byte-identical to a fresh generate_openapi_schema.py run; uv lock --check is clean and annotated-doc is properly locked; pins.json doesn't track fastapi, so no sync issue with #9351. 2186 backend tests pass, 0 failures locally.

Worth calling out

The auth-guard repair is the most valuable thing in this PR and is underweighted in the description. Confirmed independently: iter_route_contexts finds 197 APIRoutes where app.routes finds 2. The default-deny guard had been inspecting ~1% of the API and passing. PUBLIC_ROUTES is unchanged, which is the part that matters — the restored guard found no unauthenticated routes, so nothing was papered over to make it pass. That commit stands on its own and deserves the separate title you offer in the merge plan; the FastAPI bump is much easier to justify with that finding in its own changelog entry.

[lstein]

Following up on the action item from my review — I swept all the converted handlers for the same lost-atomicity class, since the AST guard can't see it. One new finding, and it chains into the setup_admin one.

Method

The diff converts 180 functions: 175 route handlers (8 in package A + 167 in the sweep) + the 5 auth dependencies. I pulled them from the diff by name, filtered by AST to the 78 whose bodies contain a mutating call, and read those.

The filter that matters: an async def handler with no await was atomic only against other HTTP requests — never against the session processor, the install thread, or the image-move worker. And atomicity is not lost where the mutation is a single SQL statement, where the read-modify-write sits inside one db.transaction() (the RLock covers it), or where the write re-enforces the check in its own WHERE clause.

So the regression set is narrow: check-then-act across two service calls, where the racing party is another request, and the write does not re-check. That leaves two, one of which I'd already reported.

New finding — delete_user can delete every admin

auth.py:571 is users.get() → count_admins() <= 1 → users.delete(): three separate transactions.

Triggering sequence: exactly 2 admins exist; two concurrent DELETE /api/v1/auth/users/{id} for two different admins. Both read count_admins() == 2, both pass the guard, both delete:

statuses: [204, 204]
admins remaining: set()

Before the change the second request read 1 and got the intended 400.

The consequence is worse than losing the admins. With zero admins, has_admin() is false, so GET /auth/status reports setup_required: true and POST /auth/setup becomes reachable unauthenticated — the instance drops back into first-run setup and whoever reaches it first becomes the new admin. That is the same has_admin() gate finding #1 in my review depends on, and both routes are now racy, so I'd fix them together: one guard inside UserService, inside the transaction that performs the write, rather than in the route.

Worth noting this intersects the already-deferred last-admin-invariant follow-up. The TOCTOU was known there, but it was only reachable from non-HTTP paths; this PR makes it reachable from two concurrent curl commands.

Newly concurrent, low severity

Handler	What changed
custom_nodes.install_custom_node_pack	(already reported) the loser's except handler rmtrees the winner's directory
model_manager.convert_model	get_model → minutes of conversion → delete + install_path. Same-key double-convert is now possible; separately, two different conversions can now run in parallel and compete for RAM/VRAM, which nothing bounds
workflows.update_workflow / delete_workflow / update_workflow_is_public	read existing → authorize → write. The write re-passes user_id, so authorization stays safe at the SQL layer — only the old_is_public in the emitted event can be stale. Cosmetic
model_manager.do_hf_login	global token + HF cache file; last writer wins

Cleared, with the reason

Recording these so the sweep is legible as done rather than sampled:

• create_user — email TEXT NOT NULL UNIQUE, so the check-then-insert race is caught by the constraint and create turns the IntegrityError into a 400.
• images.star_images_in_list / unstar_images_in_list / delete_images_from_list, board_images.add_images_to_board, videos.*, system_prompts.*, style_presets.delete_style_preset — gate on ownership, which no endpoint can change, and/or re-pass user_id into the write.
• app_info.update_runtime_config / set_external_provider_config / reset_external_provider_config — already hold _EXTERNAL_PROVIDER_CONFIG_LOCK.
• image_moves.start_image_move / start_image_move_recovery — the service is already threading.Lock-guarded because it manages a worker thread.
• session_queue.retry_items_by_id — per-item authorize then bulk retry; the only mutable input is item status, which already raced the session processor. Not a regression.
• download_queue.*, client_state.*, boards.create_board, recall_parameters.update_recall_parameters (a single set_by_key, no read-modify-write), model_manager.delete_orphaned_models (per-path result reporting absorbs it).
• The assert_image_move_maintenance_inactive() → loop-of-writes pattern in the images/videos bulk routes: maintenance can begin mid-loop, but that already raced the move worker, which sets the flag from its own thread. Pre-existing.

Pre-existing, found en route — not caused by this PR

update_user has no last-admin guard at all. PATCH /api/v1/auth/users/{id} with {"is_admin": false} or {"is_active": false} against the sole admin succeeds single-threaded, reaching the same zero-admin state with no race needed — count_admins() is called from exactly one place in the codebase, delete_user. Flagging it because it means fixing only the race would not restore the invariant, and both point at the same fix: put the guard in UserService next to the write.

[lstein]

Follow-up to the sweep above, and a correction to one line in it.

Correction: I wrote that update_user has no last-admin guard. That is true of main, but #9360 (open) already adds a route-level count_admins() guard there, so that specific gap is spoken for — I should have checked the open PRs before flagging it as unclaimed. Apologies for the noise.

What was genuinely unclaimed is the TOCTOU itself: both #9360's new guard and the existing delete_user one read count_admins() in their own transaction and then write in another. That is the part this PR makes reachable from two concurrent HTTP requests rather than only across processes, and it is fixed in #9479, opened off main:

• moves the invariant into UserService.update() / UserService.delete(), evaluated on the cursor of the transaction that performs the write, with BEGIN IMMEDIATE so the count is read under the write lock;
• therefore also covers invoke-usermod / invoke-userdel, which construct UserService directly and never reach a route guard;
• keeps the route-level checks as the friendly-400 path — the service is the backstop.

17 new tests, 9 of which fail without the guard, including three that race two threads through a barrier and assert exactly one survives.

#9479 is independent of this PR — the invariant is wrong on main today and the fix is confined to the user service. It does not block #9436. Noting it here only because #9436 is what changes the race from cross-process to two-curl-commands, so the two are worth landing in either order but not forgetting about.

That leaves the two findings in my review unchanged: the setup_admin duplicate-admin race (same shape, same fix location — worth doing in the same pass as #9479 if you'd rather I extend that PR) and the unconsumed queue-summaries endpoint.

[JPPhoto]

@Pfannkuchensack Can you fix this?

• invokeai/app/services/session_queue/session_queue_sqlite.py:1365 expands every client-supplied ID into one SQLite bind, with no batch limit at invokeai/app/api/routers/session_queue.py:236. Posting 32,766 IDs causes OperationalError: too many SQL variables, returned as HTTP 500. Test: POST {"item_ids": list(range(32766))} to /api/v1/queue/default/item_summaries_by_ids; expect controlled rejection or chunking.

Suggestions:

• Consider bounding/chunking IDs and adding this overflow regression test. Keep the earlier @lstein review follow-ups separate: #9479 for admin invariants and the summary-route consumer decision.

Ping me when you're ready for a re-read!

[JPPhoto]

Merge blockers:

• None. The last-admin deletion/demotion race is covered by #9360.

Other findings/issues:

• invokeai/app/api/routers/model_manager.py:1169-1186: conversion lock excludes concurrent delete_model/bulk deletion now running in worker threads. Same-key convert/delete can fail, delete the source mid-conversion, or leave a replacement after HTTP 204. Test: barrier conversion around loader.load_model or installer.install_path, race PUT /models/convert/{key} with DELETE /models/i/{key}; expect serialized outcome and no orphan/replacement.

Suggestions:

• Consider making PR 9360 a merge dependency for PR 9436: its transactional user-service guard addresses the earlier last-admin race. PR 9360 does not address model conversion versus deletion.

[JPPhoto]

Other findings/issues:

• invokeai/app/api/routers/model_manager.py:344-372,493-525,598-618,738-777,1203-1224: Head 6c6e38e fixes the conversion/delete race, but reidentify, record/image updates, and bulk reidentify still bypass the per-key claim. Concurrent conversion can discard metadata or images while the other request reports success. Test: barrier conversion after reading the old config and race each operation; expect serialization and preserved replacement data.

Suggestions:

• Consider marking the conversion/delete finding resolved against 6c6e38e, then extending the claim to these remaining operations.

• Consider keeping #9360 as a merge-order dependency rather than a suggestion; 9436 itself lacks the service-level last-admin guard.