[Bugfix][Frontend] Remove LoRA adapter from the engine on unload

[emerardd]

Purpose

Closes #42633. Closes #54839.

POST /v1/unload_lora_adapter only drops the adapter from the frontend
registry. It never tells the engine, so the adapter keeps its worker-side slot
and CPU cache entry until LRU eviction happens to reclaim it. The adapter
vanishes from /v1/models while still occupying a LoRA slot no request can be
routed to.

OpenAIServingModels.load_lora_adapter already calls
self.engine_client.add_lora(...); the unload path had no counterpart. The Rust
frontend does not have this bug — LoraManager::unload_lora in
rust/src/server/src/lora.rs calls remove_lora before dropping its registry
entry — so the two frontends currently disagree on what unloading means.

Relationship to #42634

@rayowang got here first. #42634 (opened 2026-05-14) identifies the same bug
and takes the same approach this PR takes: add remove_lora to the
EngineClient protocol and call it from unload_lora_adapter before dropping
the registry entry. That design is theirs, and if maintainers prefer to land
#42634 instead, the fix below can be reduced to a review comment there — I would
rather see the bug fixed than see this PR merged.

I am opening this separately because of one behavioral difference that I believe
makes #42634 unmergeable as written, plus two smaller gaps.

The difference that matters: remove_lora() == False is not an error

#42634 turns a falsy return into a 500:

removed = await self.engine_client.remove_lora(lora_request.lora_int_id)
if not removed:
    return create_error_response(
        message=f"Failed to remove lora adapter '{lora_name}' from the engine.",
        err_type="InternalServerError",
        status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
    )

LoRAModelManager.remove_adapter returns False whenever the id is no longer in
_registered_adapters (vllm/lora/model_manager.py:1194), and that state is
reachable in normal operation: when loading an adapter would exceed
max_cpu_loras, LRUCacheWorkerLoRAManager evicts the oldest one
(vllm/lora/worker_manager.py:308-312 → remove_oldest_adapter() →
_registered_adapters.remove_oldest()).

So on --max-cpu-loras 2:

1. Load adapters A, B, C. Loading C evicts A from the worker.
2. POST /v1/unload_lora_adapter {"lora_name": "A"}.
3. remove_adapter returns False, and [LoRA] Remove engine-side adapter on runtime unload #42634 answers 500 — for a request
   that is entirely valid, on an adapter still listed in /v1/models.

That is the exact state this bug produces today, so the eviction path is not a
corner case here; it is the common one. This PR logs the falsy return at debug
level and still removes the registry entry, leaving the endpoint's success
contract unchanged.

Note this also differs from the Rust frontend, which returns NotRemoved in that
case. I think the Rust side has the same latent problem, but that is out of scope
here and I did not want to change two frontends in one PR. Happy to follow up.

Two smaller differences

• [LoRA] Remove engine-side adapter on runtime unload #42634 does not wrap the engine call, so an exception from remove_lora
  escapes to FastAPI as an unhandled 500. This PR catches it and returns a
  structured ErrorResponse, matching how load_lora_adapter already handles
  add_lora failures.
• Test coverage: [LoRA] Remove engine-side adapter on runtime unload #42634 asserts the call happens. This PR adds the evicted-return
  and raising-engine cases, which are the two paths the difference above is
  about.

Changes

• EngineClient gains an abstract remove_lora, mirroring the existing abstract
  add_lora. AsyncLLM is the only implementation and already defines it, so no
  implementer changes. (Same as [LoRA] Remove engine-side adapter on runtime unload #42634; the bot concern raised there about
  breaking subclasses was correctly rebutted by @rayowang.) The protocol
  deliberately exposes only what the entrypoints layer needs — it declares
  add_lora but not list_loras/pin_lora — and the unload path now needs
  remove_lora.
• unload_lora_adapter calls engine_client.remove_lora(lora_int_id) before
  dropping the registry entry, returning a structured 500 on exception and
  treating a falsy return as success.

/v1/unload_lora_adapter is the only unload entry point. The SageMaker
register_unload_adapter_handler in vllm/entrypoints/serve/lora/api_router.py
dispatches to the same method, so this single change covers both routes.

Duplicate-work checks

Per AGENTS.md:

gh issue view 54839 --repo vllm-project/vllm --comments
gh pr list --repo vllm-project/vllm --state open --search "54839 in:body"
gh pr list --repo vllm-project/vllm --state open --search "unload_lora_adapter"

These surface #42634 (addressed above), plus #54830 and #54833, which cite the
bug as motivation for LoRA metrics but do not touch
vllm/entrypoints/openai/models/serving.py. #54839 is itself a duplicate report
of #42633.

Test Plan

tests/entrypoints/serve/lora/test_serving_models.py covers the endpoint with a
MagicMock(spec=EngineClient), so the regression is caught at unit level without
a GPU:

• test_unload_lora_adapter_success — extended to assert the engine is told, via
  remove_lora.assert_awaited_once_with(lora_id).
• test_unload_lora_adapter_success_when_engine_already_evicted — remove_lora
  returns False; the request must still succeed and clear the registry. This is
  the case [LoRA] Remove engine-side adapter on runtime unload #42634 would 500 on.
• test_unload_lora_adapter_engine_error — remove_lora raises; the response
  must be a structured 500 and the adapter must stay registered.

VLLM_USE_PRECOMPILED=1 uv pip install -e .
python -m pytest tests/entrypoints/serve/lora/test_serving_models.py -v
pre-commit run --files vllm/engine/protocol.py \
  vllm/entrypoints/openai/models/serving.py \
  tests/entrypoints/serve/lora/test_serving_models.py

Test Result

On main @ f81eb4193 + this change, Python 3.12, Ubuntu (WSL2):

$ python -m pytest tests/entrypoints/serve/lora/test_serving_models.py -v
...
tests/.../test_unload_lora_adapter_success PASSED                        [ 45%]
tests/.../test_unload_lora_adapter_success_when_engine_already_evicted PASSED [ 54%]
tests/.../test_unload_lora_adapter_engine_error PASSED                   [ 63%]
...
======================= 11 passed, 15 warnings in 4.35s ========================

Reverting only the two vllm/ files and keeping the tests reproduces the bug:

FAILED tests/.../test_unload_lora_adapter_success
FAILED tests/.../test_unload_lora_adapter_success_when_engine_already_evicted
FAILED tests/.../test_unload_lora_adapter_engine_error
E   AttributeError: Mock object has no attribute 'remove_lora'
3 failed, 8 passed, 15 warnings in 4.71s

The AttributeError is the point: with MagicMock(spec=EngineClient) the
attribute does not exist on main, because nothing in the unload path ever
reaches for it.

pre-commit run --files <the three files> passes, including Run mypy for Python 3.10, ruff, typos, Check SPDX headers and Check for forbidden imports.
(The update-dockerfile-graph hook fails in my checkout for an unrelated local
reason — a CRLF working tree makes its shell script unrunnable. It touches none
of these files.)

Model evaluations do not apply: this changes an administrative endpoint's
engine-side cleanup only. No sampling, kernel, or model-output path is touched,
and the endpoint's HTTP response contract is unchanged.

Notes

AI assistance was used to write this change. I reviewed every changed line and
traced the remove_lora path through AsyncLLM → EngineCore → WorkerBase →
LoRAModelManager.remove_adapter to confirm the False semantics described
above.

Credit to @rayowang for #42634, which found this bug and this fix first.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[github-actions]

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run for upstream CI or /amd-ci run for AMD CI only whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use the corresponding /ci run, /ci retry, and /ci cancel commands, or their /amd-ci variants. New commits do not start upstream CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀