1 Commits

Author SHA1 Message Date
ly-wang19
25d9ac0a43
fix(skills): offload blocking filesystem IO in get_custom_skill_history (#3563)
* fix(skills): offload blocking filesystem IO in get_custom_skill_history

The GET /api/skills/custom/{name}/history handler ran its storage probes and the
per-skill .history read directly on the asyncio event loop:
get_or_new_skill_storage(), custom_skill_exists(), get_skill_history_file().exists()
and read_history() are all blocking filesystem IO. make detect-blocking-io flagged
the existence probe (routers/skills.py:224) as DIRECT_ASYNC.

Move the whole read into a nested sync function run via asyncio.to_thread; a None
return signals 404 (distinct from an empty history list). Behavior is unchanged.

Per the blocking-io-guard SOP:
- Candidate: get_custom_skill_history (FILE_METADATA, DIRECT_ASYNC) -> FIX+ANCHOR.
- Re-scan: the finding no longer appears for this handler.
- Anchor: tests/blocking_io/test_skills_router.py drives the real handler against a
  real on-disk skill + history; teeth verified red (pre-fix) -> green (post-fix)
  under make test-blocking-io.

Scoped to this self-contained read handler. rollback_custom_skill and update_skill
also touch blocking IO but interleave it with awaits (security scan / cache refresh)
and do a read-modify-write, so offloading them needs the asyncio.Lock serialization
treatment (cf. #3552) and is left as a separate fix unit.

* test: trim dead skills history setup

* fix(skills): use the user-scoped storage accessor in the offloaded history read

The merge with main left the offloaded reader calling get_or_new_skill_storage,
which is not defined in this module (ruff F821), so lint failed and the handler
would raise NameError at runtime. Use _get_user_skill_storage(config) — the same
accessor every other handler in this router uses.

Also update the regression test for the current route signature: the handler is
now admin-only and takes a Request, so the test supplies request.state.user
(mirroring tests/blocking_io/test_channel_runtime_config_store.py) and seeds the
history through the same user-scoped accessor.

---------

Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-07-24 22:33:16 +08:00