fix(themes): move the theme-detail target stats off the event loop (#5963)

[chenmingwei23]

What is the problem?

api_theme_detail (GET/PUT/DELETE /api/themes/{slug}) still makes seven inline exists()/is_dir() call expressions across six condition sites on the asyncio event loop. On a local disk these are microseconds; on a UNC data home (KIROCREW_HOME on an SMB share) every stat is a network round trip that can block for as long as the network takes. These sites became reachable on Windows when #5943 removed the theme-pack 501 gate; #5943 offloaded the asset routes' resolver and tracked the detail route's remaining stats as debt in #5963.

Why this issue matters to the user

The event loop is shared by every request the gateway serves. One theme-detail request against a slow SMB data home stalls chat streaming, approvals, and every other dashboard route until the stat returns -- a whole-gateway hang triggered by a single settings page visit.

How our fix solves it

All target stats now ride one discovery_executor() hop: a small synchronous helper (_stat_targets) returns (target.exists(), dir_target.is_dir()), and every method branch consumes the captured pair. One hop replaces six, matching how the route already offloads its reads/writes and how #5943 offloaded _resolve_theme_asset at the asset/overlay/topbar routes. Branch-for-branch behavior parity with main was verified (same status codes, same ordering, same error paths); the DELETE-dir branch keeps its off-loop is_dir() re-check under the per-slug install lock. The DELETE-file unlink gains missing_ok=True: the stat now rides an earlier hop, so a concurrent delete winning the race is answered as the idempotent ok it is, rather than escaping as a 500.

What tests we did

• New TestApiThemeDetailStatsOffLoop (3 parametrized cases, GET/PUT/DELETE): spies on Path.exists/Path.is_dir for the handler's two target paths and fails if any such stat runs on the loop thread. Mutation-verified: inlining the helper turns all three red (2 target stat(s) ran on the event loop). Watched paths are built through the handler's own resolver so the assertion survives Darwin's /tmp symlink.
• Full themes suites green: 343 passed (test_dashboard_themes_coverage.py + test_theme_install.py).
• isort/flake8 clean; black + brand gates pass; mypy shows only 4 pre-existing errors in untouched files (identical on clean base via stash A/B).
• Full backend pytest: 69,974 passed; 86 failures + 2 errors are host-environment classes (AF_UNIX path length, CPU-cap assertions, sandbox floors) with zero overlap with this diff, reproduced identically on clean base.

Any other suggestions on the work

Pre-push review (GPT 5.6 + Opus): both no blocking findings. Three advisories folded in before opening (Darwin-safe watched paths, lock-comment narrowing, missing_ok on the unlink). One suggestion declined: short-circuiting the second stat for GET/DELETE when the first suffices -- the issue explicitly asks for the stats grouped as one logical check in a single hop, and one extra stat inside an already-offloaded worker costs no loop time. The pre-existing PUT-vs-DELETE recreate race noted by Opus is unchanged by this diff and is left for its own issue if it earns one.

Closes #5963

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 0d9f5dcd472575bf6f2f87cadab241a8e33270f2 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Real event-loop stall, fixed at the right layer with the same off-loop discipline #5943 established, TOCTOU races handled (missing_ok, locked re-check).

[DESIGN-REVIEWED] 0d9f5dc

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 0d9f5dcd472575bf6f2f87cadab241a8e33270f2 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 0d9f5dc

Verdict parsed from the review's SHA-scoped output markers for commit 0d9f5dcd472575bf6f2f87cadab241a8e33270f2.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 0d9f5dcd472575bf6f2f87cadab241a8e33270f2: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 0d9f5dcd472575bf6f2f87cadab241a8e33270f2 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 0d9f5dc

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 0d9f5dcd472575bf6f2f87cadab241a8e33270f2: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 0d9f5dcd472575bf6f2f87cadab241a8e33270f2 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The review contract is a first-principles gate: inventory the change, test each item for necessity, duplication, and root-cause depth. I've read the intent file, the patch, and the surrounding handler; the one thing worth surfacing is a counted, unfixed sibling of the same root cause in the same file.

First-Principles-Verdict: CONCERNS

The fix itself is derived and minimal, but the same file still runs an on-loop mkdir + exists() in api_themes_create — an unmentioned sibling of the exact root cause.

What this change ships

Intent: stop a theme-detail request from freezing the whole gateway when the data home is on a network share — a FIX.

1. Detail-route target stats run on a worker thread, one hop — justified (defect chain fix(themes): enable theme pack routes on Windows #5943→Move the remaining six api_theme_detail stats off the event loop #5963).
2. Deleting a theme that a concurrent delete already removed now succeeds instead of erroring — declared, justified consequence of item 1.
3. DELETE/GET now always stat both targets (one extra stat off-loop) — declared, defended, harmless.
4. New off-loop spy test, mutation-verified — justified.

No new public surface, config keys, or exported symbols; the change reuses the existing discovery_executor() mechanism rather than inventing one.

Watch

• Point patch with 1 counted unfixed sibling. Grepped \.exists\(\)|\.is_dir\(\) across themes.py: every hit is off-loop except api_themes_create — themes_path.mkdir(parents=True, exist_ok=True) and target.exists() at src/kiro_crew/dashboard/handlers/themes.py:166-168 run on the event loop, the identical SMB-blocking class this PR fixes. The description says what Move the remaining six api_theme_detail stats off the event loop #5963 covered but not what is left. Smaller-still fix: the line-168 pre-check is a second spelling of the authoritative in-lock check _create_locked already runs (line 189, same 409) — delete it and move the mkdir into _create_locked, which removes the sibling by subtraction.

[FIRST-PRINCIPLES-REVIEWED] 0d9f5dc

[NicholasRBowers]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix with clear root cause — theme-detail handler stats (exists/is_dir) moved off the event loop onto the discovery executor, matching the asset routes' established off-loop discipline; handler + regression test only.

[chenmingwei23]

🤖 Kiro Crew Auto-Pipeline [operator: chenmingwei23#de330d0c]: Disposition for the First Principles CONCERNS (advisory, head 0d9f5dc) -- accepted and deferred. Verified in live code: api_themes_create does run mkdir + target.exists() on the loop (themes.py ~165-168), the same SMB-blocking class this PR fixes for the detail route. It is outside #5963's declared scope (the issue names the six detail-route stats), and folding it in would grow a converged single-purpose diff. Filed as #6198 with the review's subtraction-shaped fix (drop the duplicate pre-check, move the mkdir into _create_locked) so it lands as its own minimal change. Design Review PASS and both hard gates are clean on this head; no code change, so no re-roll.