fix: Review sweep

This commit is contained in:
Eric Hare
2026-05-08 17:28:24 -07:00
parent 42fc6ae507
commit 1d7e366498
6 changed files with 231 additions and 32 deletions

View File

@ -1,6 +1,4 @@
# Router for base api
import os
from fastapi import APIRouter
from lfx.services.settings.feature_flags import FEATURE_FLAGS
@ -81,27 +79,19 @@ router_v1.include_router(models_router)
router_v1.include_router(model_options_router)
# Extension reload is Mode A (local-dev / pip-installed) only -- in Mode B/C
# bundle changes require a Docker image rebuild and the in-process reload
# endpoint would mask the real deploy pipeline. Off by default so authenticated
# users on self-hosted / production deployments cannot trigger runtime imports
# without an explicit opt-in via LANGFLOW_ENABLE_EXTENSION_RELOAD.
# Extension reload is Mode A (local-dev / pip-installed) only. The route is
# always mounted; a per-request guard in ``langflow.api.v1.extensions`` reads
# the live ``settings.enable_extension_reload`` and returns 404 when the flag
# is off, which means the route is indistinguishable from "not mounted" on
# production deployments that leave it unset.
#
# Read the env var directly here rather than going through the settings
# service: this module is imported during ``langflow run`` startup, before
# ``--env-file`` has been loaded into the environment. Calling
# ``get_settings_service()`` here would initialize settings prematurely and
# break the documented "env file -> settings" ordering enforced by
# ``__main__.py``. Pydantic settings normalize the same flag from the
# environment when settings are eventually built; this gate just decides
# whether to attach the route.
def _maybe_include_extensions_router() -> None:
raw = os.environ.get("LANGFLOW_ENABLE_EXTENSION_RELOAD", "").strip().lower()
if raw in {"1", "true", "yes", "on"}:
router_v1.include_router(extensions_router)
_maybe_include_extensions_router()
# Mounting unconditionally avoids the import-time / env-file ordering
# coupling: ``langflow.__main__`` imports ``setup_app`` (and hence this
# router module) before ``load_dotenv(env_file)`` runs, so any module-level
# read of ``LANGFLOW_ENABLE_EXTENSION_RELOAD`` would miss the value supplied
# via ``--env-file``. The runtime guard sees the post-env-file value
# because it executes per-request, after settings have been built.
router_v1.include_router(extensions_router)
include_deployment_router(router_v1)

View File

@ -6,7 +6,8 @@ which drives the LE-1018 atomic-swap pipeline against the process-default
LE-1019 (UI) and LE-1020 (migration) will add list / status / migrate
endpoints alongside this one. Mode A only -- in Mode B/C the path is to
rebuild the Docker image, and this endpoint is not registered.
rebuild the Docker image, and the runtime guard at the request layer
short-circuits with 404 when ``LANGFLOW_ENABLE_EXTENSION_RELOAD`` is off.
"""
from __future__ import annotations
@ -15,16 +16,45 @@ from fastapi import APIRouter, Depends, HTTPException, status
from lfx.extension.bundle_registry import get_default_registry
from lfx.extension.reload import ReloadInProgressError, reload_bundle
from lfx.log.logger import logger
from lfx.services.deps import get_settings_service
from langflow.services.auth.utils import get_current_active_user
router = APIRouter(prefix="/extensions", tags=["Extensions"])
def _require_extension_reload_enabled() -> None:
"""Per-request guard for the Mode A reload route.
The route is always mounted (see ``langflow.api.router``) so that
``--env-file`` can opt in to it without forcing route registration
to happen after the env file is loaded. Operators on Mode B/C
deployments leave ``LANGFLOW_ENABLE_EXTENSION_RELOAD`` unset; the
guard returns 404 so the route is indistinguishable from "not
mounted" for those operators.
"""
settings = get_settings_service().settings
if not getattr(settings, "enable_extension_reload", False):
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail={
"code": "extension-reload-disabled",
"message": (
"Extension reload is disabled. Set "
"LANGFLOW_ENABLE_EXTENSION_RELOAD=true to enable it on a "
"local-development install (Mode A)."
),
},
)
@router.post(
"/{extension_id}/bundles/{bundle_name}/reload",
status_code=status.HTTP_200_OK,
dependencies=[Depends(get_current_active_user)],
dependencies=[
Depends(_require_extension_reload_enabled),
Depends(get_current_active_user),
],
)
async def reload_extension_bundle(extension_id: str, bundle_name: str) -> dict:
"""Trigger an atomic-swap reload for a single Bundle.

View File

@ -0,0 +1,92 @@
"""Tests for the runtime gate on the Extension reload route.
The route is always mounted (no import-time dependency on the env file
having been loaded yet); a per-request dependency reads the live
``settings.enable_extension_reload`` and returns ``404`` when off. This
keeps Mode B/C deployments indistinguishable from "not mounted" while
allowing ``--env-file LANGFLOW_ENABLE_EXTENSION_RELOAD=true`` to opt
into the route at runtime.
"""
from __future__ import annotations
from unittest.mock import MagicMock
import pytest
from fastapi import HTTPException, status
from langflow.api.v1.extensions import _require_extension_reload_enabled
def _make_settings(*, enable: bool):
settings = MagicMock()
settings.enable_extension_reload = enable
service = MagicMock()
service.settings = settings
return service
def test_guard_raises_404_when_disabled(monkeypatch: pytest.MonkeyPatch) -> None:
"""Guard must return 404 with the typed code when the flag is off."""
monkeypatch.setattr(
"langflow.api.v1.extensions.get_settings_service",
lambda: _make_settings(enable=False),
)
with pytest.raises(HTTPException) as exc:
_require_extension_reload_enabled()
assert exc.value.status_code == status.HTTP_404_NOT_FOUND
assert exc.value.detail["code"] == "extension-reload-disabled"
assert "LANGFLOW_ENABLE_EXTENSION_RELOAD" in exc.value.detail["message"]
def test_guard_passes_when_enabled(monkeypatch: pytest.MonkeyPatch) -> None:
"""Guard must be a no-op when the flag is on."""
monkeypatch.setattr(
"langflow.api.v1.extensions.get_settings_service",
lambda: _make_settings(enable=True),
)
# Must not raise.
_require_extension_reload_enabled()
def test_guard_treats_missing_attribute_as_disabled(monkeypatch: pytest.MonkeyPatch) -> None:
"""Defensive: a settings object missing the field must not enable the route."""
settings = MagicMock(spec=[]) # no attributes
service = MagicMock()
service.settings = settings
monkeypatch.setattr(
"langflow.api.v1.extensions.get_settings_service",
lambda: service,
)
with pytest.raises(HTTPException) as exc:
_require_extension_reload_enabled()
assert exc.value.status_code == status.HTTP_404_NOT_FOUND
def test_route_is_mounted_unconditionally() -> None:
"""The reload route must be registered regardless of env-var state.
Regression: an earlier implementation read ``LANGFLOW_ENABLE_EXTENSION_RELOAD``
at import time, which meant ``--env-file`` could not turn the route on
because ``langflow.__main__`` imports the router before ``load_dotenv``
runs. Mounting unconditionally + runtime guard fixes this.
"""
from langflow.api.router import router
def collect_paths(routes, prefix: str = "") -> list[str]:
out: list[str] = []
for route in routes:
if hasattr(route, "routes"):
out.extend(collect_paths(route.routes, prefix + getattr(route, "prefix", "")))
elif hasattr(route, "path"):
out.append(prefix + route.path)
return out
paths = collect_paths(router.routes)
assert any("/extensions/" in p and "/reload" in p for p in paths), (
f"Extension reload route must be mounted unconditionally; mounted paths: {paths}"
)

View File

@ -57,6 +57,16 @@ export default defineConfig(({ mode }) => {
"import.meta.env.LANGFLOW_MCP_COMPOSER_ENABLED": JSON.stringify(
envLangflow.LANGFLOW_MCP_COMPOSER_ENABLED ?? "true",
),
// Mode A only: gates the palette Bundle-header Reload action.
// ``feature-flags.ts`` reads this via ``import.meta.env``; without an
// entry here Vite never substitutes the value at build time, so the
// flag stays undefined for both ``vite dev`` and ``vite build`` and
// the Reload UI never appears even when extension templates carry
// the metadata. Default off so production builds keep parity with
// the backend's default-off ``LANGFLOW_ENABLE_EXTENSION_RELOAD``.
"import.meta.env.LANGFLOW_EXTENSION_RELOAD_ENABLED": JSON.stringify(
envLangflow.LANGFLOW_EXTENSION_RELOAD_ENABLED ?? "false",
),
},
plugins: [
react(),

View File

@ -597,14 +597,7 @@ def dev_command(
typer.echo("")
typer.echo("Launching: " + " ".join(shlex.quote(part) for part in cmd))
env = os.environ.copy()
# Force eager loading so dev-extension components show up in the
# palette within AC #4's 5s budget. Use unconditional assignment
# (not setdefault) so a developer who has lazy loading exported in
# their shell does not silently lose dev components from the palette
# -- the dev workflow needs eager loading regardless of the global
# default.
env["LANGFLOW_LAZY_LOAD_COMPONENTS"] = "false"
env = _build_dev_launch_env(os.environ)
# Replace the current process so Ctrl-C in the launched langflow
# propagates without an extra pty/job-control hop. When ``langflow``
@ -616,6 +609,36 @@ def dev_command(
os.execvpe(cmd[0], cmd, env) # noqa: S606
def _build_dev_launch_env(base_env: os._Environ[str] | dict[str, str]) -> dict[str, str]:
"""Build the env dict handed to the launched ``langflow run`` process.
Centralizes the per-flag rationale so the contract is testable in one
place and a missing flag is caught by a focused unit test rather than
only manifesting as a runtime UX gap.
Flags set:
- ``LANGFLOW_LAZY_LOAD_COMPONENTS=false`` (always, overriding the
author's shell): dev components must appear in the palette
eagerly so the AC's 5-second budget holds.
- ``LANGFLOW_ENABLE_EXTENSION_RELOAD=true`` (setdefault): the
backend route only mounts a usable handler when this is set;
``setdefault`` so an author intentionally testing the off path
can pre-export ``=false``.
- ``LANGFLOW_EXTENSION_RELOAD_ENABLED=true`` (setdefault): the
frontend palette gate (``ENABLE_EXTENSION_RELOAD`` in
``feature-flags.ts``) is wired through ``import.meta.env`` and
read by Vite at build time; setting it here only affects a
``vite dev`` server launched from the same shell. Listed for
parity with the backend flag so an author who runs both halves
locally gets the Reload button without hand-editing ``.env``.
"""
env = dict(base_env)
env["LANGFLOW_LAZY_LOAD_COMPONENTS"] = "false"
env.setdefault("LANGFLOW_ENABLE_EXTENSION_RELOAD", "true")
env.setdefault("LANGFLOW_EXTENSION_RELOAD_ENABLED", "true")
return env
def _build_langflow_run_argv(extra_args: list[str]) -> list[str]:
"""Build the argv list that exec's ``langflow run``.

View File

@ -256,6 +256,60 @@ def test_dev_refuses_non_directory(
assert "not a directory" in (result.stderr or result.stdout)
# ---------------------------------------------------------------------------
# dev launch env: contract for the env vars handed to ``langflow run``
# ---------------------------------------------------------------------------
def test_dev_launch_env_enables_reload_and_eager_loading() -> None:
"""The launched langflow inherits flags that make 'edit -> Reload' work.
Without ``LANGFLOW_ENABLE_EXTENSION_RELOAD=true`` the backend route is
not registered as a usable handler and the documented author loop
404s. Without ``LANGFLOW_LAZY_LOAD_COMPONENTS=false`` dev components
miss the palette's 5-second budget. Both must be set unconditionally
or via setdefault per the helper's contract.
"""
from lfx.cli._extension_commands import _build_dev_launch_env
env = _build_dev_launch_env({})
assert env["LANGFLOW_LAZY_LOAD_COMPONENTS"] == "false"
assert env["LANGFLOW_ENABLE_EXTENSION_RELOAD"] == "true"
assert env["LANGFLOW_EXTENSION_RELOAD_ENABLED"] == "true"
def test_dev_launch_env_overrides_author_lazy_loading() -> None:
"""``LANGFLOW_LAZY_LOAD_COMPONENTS=true`` in the author shell must not win.
The dev workflow always wants eager loading; an author whose shell
exports lazy loading for normal langflow use should not silently
lose dev components from the palette.
"""
from lfx.cli._extension_commands import _build_dev_launch_env
env = _build_dev_launch_env({"LANGFLOW_LAZY_LOAD_COMPONENTS": "true"})
assert env["LANGFLOW_LAZY_LOAD_COMPONENTS"] == "false"
def test_dev_launch_env_respects_author_reload_off_path() -> None:
"""An author testing the off path can pre-export the disable flag.
The reload flags use setdefault so an explicit ``=false`` exported
in the shell survives the helper's defaulting.
"""
from lfx.cli._extension_commands import _build_dev_launch_env
env = _build_dev_launch_env(
{
"LANGFLOW_ENABLE_EXTENSION_RELOAD": "false",
"LANGFLOW_EXTENSION_RELOAD_ENABLED": "false",
}
)
assert env["LANGFLOW_ENABLE_EXTENSION_RELOAD"] == "false"
assert env["LANGFLOW_EXTENSION_RELOAD_ENABLED"] == "false"
# ---------------------------------------------------------------------------
# extension reload (LE-1018) -- argument validation
# ---------------------------------------------------------------------------