Repository navigation
refactor: production readiness audit - #6
Merged
Merged
Conversation
…egistration durable Two production-breaking defects that a development checkout hides. Packaging: fastapi, uvicorn, websockets, python-multipart and rich were declared as core dependencies but were also named in [tool.poetry.extras]. Poetry treats any dependency named in an extra as extra-only, so the published wheel demoted all five to `extra == "ui"` / `extra == "rich"`. A plain `pip install flowyml` therefore produced an install whose entire web UI died with `ModuleNotFoundError: No module named 'fastapi'`. The extras now carry only genuinely optional packages; `ui` and `rich` are kept as empty extras so existing `pip install flowyml[ui]` invocations keep working. The mirror-image defect: azure-ai-ml, azure-identity and azure-storage-blob were referenced by the `azure` extra but never declared as dependencies, so Poetry dropped them and `pip install flowyml[azure]` installed nothing while the Azure blob store and Azure ML registry import them at runtime. They are now declared optional. mlflow, wandb and keras were declared optional but belonged to no extra at all, making them unreachable by any install command; they now have extras, and `flowyml[all]` is a true superset of every extra. Registry: components registered via the @register_component decorator were lost whenever the global ComponentRegistry was rebuilt. The decorator fires once per process on first import, and Python's module cache makes any later re-import a no-op, so lazy loaders such as _ensure_providers_loaded could not restore them and get_component() returned None for the rest of the process. Registrations are now recorded and replayed into every newly constructed registry, which fixes azure_blob/gcs/s3 and friends disappearing after a reset. Adds reset_component_registry() as the supported way to rebuild. Adds tests/test_packaging_metadata.py, which asserts these invariants against both pyproject.toml and the built distribution metadata, and makes test_load_plugins_invalid_config assert its actual intent (an invalid config registers nothing new) instead of an absolute count that only held because of the registry bug. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
… UI calls
Security. The API was open by default and stayed open when misconfigured:
- AuthMiddleware returned early whenever FLOWYML_ENV was not "production" *or*
FLOWYML_API_TOKEN was unset. Both docker-compose.yml and .env.example ship
FLOWYML_API_TOKEN with an empty value, so following the project's own
deployment guide produced a production instance with no authentication at
all. POST /api/execution/execute importlib-imports and runs arbitrary
modules, so that was unauthenticated remote code execution. Startup now
aborts with an actionable message, and the middleware denies every request
as a second line of defence for hosts that skip lifespan events.
FLOWYML_ALLOW_INSECURE=1 is the deliberate opt-out for operators who
terminate auth at a proxy.
- The login form fell back to the publicly documented password "flowyml" when
FLOWYML_ADMIN_PASSWORD was unset, and issued the fixed string
"dev-token-placeholder" as a session token when FLOWYML_API_TOKEN was unset.
Either one made a production instance trivially accessible. Both are now
refused in production; development keeps its zero-config defaults.
- CORS listed "*" alongside allow_credentials=True. Starlette resolves that
pairing by reflecting the caller's Origin, letting any website issue
credentialed requests to a developer's local instance and read the replies.
Origins are now explicit and configurable via FLOWYML_CORS_ORIGINS.
- Token and password comparisons used ==, which short-circuits on the first
differing byte; they now use secrets.compare_digest. Tokens are no longer
accepted from the query string, where they leak into access logs, browser
history and Referer headers.
TokenManager. Writes were non-atomic, so a crash or a concurrent request
during the write window left a truncated token store that locked every client
out; writes now go through a temp file plus os.replace, with 0600 on the file
and 0700 on its directory. verify_token rewrote the entire store on every
authenticated request to update last_used, making each API call wait on an
fsync and racing concurrent handlers; it is now throttled and mutex-guarded.
create_token validates permissions instead of accepting typos.
Missing endpoints. Two UI features called routes that were never implemented,
and both failed silently:
- The API Tokens page's "Revoke" button issued DELETE /api/execution/tokens/
{name} against a non-existent route and never checked the response, so it
reported success while the token stayed valid. The endpoint now exists and
revokes by a stable opaque id (names are labels and need not be unique), and
the button surfaces failures instead of swallowing them.
- The Settings page's system-information card called /api/execution/info,
which did not exist, so it always rendered its hard-coded fallbacks: version
"0.1.0" (actual: 2.2.0) and database "PostgreSQL" (actual: SQLite). The
endpoint now reports the live version, backend, environment and uptime.
- GET /api/ai/context was hardcoded to /context/run/{run_id} while the UI
builds the path from a generic pageType, so any other page got an
unexplained routing 404; it now shares the POST endpoint's explanatory 400.
Also: the OpenTelemetry console exporter was enabled by default, printing a
multi-line JSON document per span - four or more per HTTP request - which
drowned real log output; it is now opt-in via FLOWYML_OTEL_CONSOLE. Version
strings in the FastAPI app and /api/health no longer report a stale "0.1.0".
Migrated the deprecated on_event startup hook to a lifespan handler and the
deprecated Pydantic class-based Config to ConfigDict.
Tests: adds test_backend_security.py (31 cases), test_execution_endpoints.py
(15 cases) and test_api_alignment.py, which extracts all 107 frontend API call
sites - handling nested template literals - and asserts each resolves to a
backend route that accepts the verb. Also isolates scheduler and leaderboard
tests onto per-test temp directories; they shared fixed filenames in the
working directory and deleted each other's databases under parallel runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
The artifact endpoints turned a stored metadata `path` into a filesystem
operation without checking where that path pointed, and the path was
client-controlled from two directions:
- POST /api/assets/ persisted its free-form `metadata` dict verbatim, so a
client could store {"path": "/etc/hostname"} and read the file back through
GET /{id}/download or /{id}/content. Verified against a running server: the
download returned the file's contents.
- The upload handler assembled its destination from `project`, `run_id`,
`artifact_id` and the uploaded `filename`, all client-supplied. A ".."
component escaped the artifacts directory, and an absolute component
discarded it entirely, since Path("/base") / "/etc/passwd" is "/etc/passwd".
- DELETE /api/assets/{id} passed the same unchecked path to the artifact
store's delete, removing whatever file it named.
Together those were arbitrary file read, write and delete against the host.
Adds flowyml/ui/backend/artifact_paths.py: resolve_within_root() resolves a
candidate against the artifacts root and refuses anything landing outside it,
resolving symlinks first so a planted link cannot point out; sanitize_path_
segment() reduces a client-supplied name to one safe component; and
strip_reserved_metadata() drops server-owned keys from user metadata. Every
download, inline-content, upload and delete handler now goes through them.
Out-of-bounds reads answer 404 rather than 403 so the caller cannot probe for
the existence of files outside the root.
Fixes a functional bug in the same code: download resolved a relative path
against the server's working directory while inline-content resolved it
against the artifacts directory, so downloading a relative-path artifact 404'd
even though viewing it worked. Both now agree.
Adds tests/test_artifact_path_security.py: 28 cases covering the confinement
primitives, all three exploits end-to-end through the API, and the legitimate
upload/download/delete round trip that must keep working.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
… responses Resource exhaustion. Every list endpoint took an unbounded `limit`, so a single `?limit=100000000` made the server materialise an arbitrarily large result set. Asset lineage took an unbounded traversal `depth`, log reads an unbounded `lines`, and step-log reads a negative `offset`. All are now declared with explicit Query bounds, matching the pattern metrics.py already used, so an out-of-range value is rejected as a 422 before any work starts. Information disclosure. 46 handlers raise HTTPException(500, detail=str(e)). For a SQLAlchemy failure that string carries the failing statement and connection details; for a filesystem failure it carries absolute server paths. The global handler echoed unhandled exception text too, with a comment conceding "in prod maybe hide this". Rather than editing 46 call sites, a single exception handler now redacts 5xx detail in production and replaces it with a short reference id that ties the response to the full server-side log entry. Development keeps the detail, where the operator is the developer, and 4xx messages are untouched since those are written for the caller. The middleware's misconfiguration 503 no longer names the missing environment variable either: that told an anonymous client the instance was running unauthenticated. The remedy is logged instead. Also replaces the SPA fallback, which was implemented by overriding the 404 exception handler, with a trailing catch-all route. The old approach turned *every* unmatched path into the SPA shell, so a typo in a fetch URL returned HTML with a 200-shaped body instead of a JSON 404, and the handler only existed when a frontend build was present. Unknown /api and /ws paths now always answer JSON 404s. Adds tests/test_backend_error_handling.py: 28 cases covering redaction in production, detail retention in development, log-correlation references, JSON 404s for unknown API paths, and the pagination bounds on every list endpoint including the valid requests that must keep working. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
…rectly Artifact identity. SQLMetadataStore stores artifact_id in its own column, but list_assets() and load_artifact() returned only the metadata JSON, so any record whose metadata did not happen to repeat the id came back without one. Everything created through POST /api/assets/ is in that category, because the handler builds metadata from the request body and never writes the id into it. The UI composes download and content URLs from that field, so those assets were listed but not retrievable, and the dedupe helper fell back to a synthetic key. Both methods now project the column into the result, which is exactly what query() already did for run_id on runs. A metadata-supplied id still wins, so existing records are unaffected. Model availability. /api/deployments/available-models tested for model files under a hard-coded "/app/artifacts" - the path inside the project's Docker image. Anyone running `flowyml ui` from a pip install keeps artifacts under the configured artifacts directory, so the probe always failed and the deployments page rendered an amber "Missing" badge for every model, making the deploy flow look broken. Resolution now goes through the configured artifacts directory and the shared confinement helper, so an artifact path pointing outside the root reports False instead of probing the host filesystem. The probes also run off the event loop rather than blocking it inside an async handler. Repository is now lint-clean: this also fixes the three pre-existing ruff findings (two blocking-filesystem calls in async handlers, one docstring) and the pathlib findings in the token store added earlier. Adds tests/test_asset_identity_and_availability.py: 11 cases covering id projection in both store methods, the full create/upload/list/download path the UI takes, and model availability reporting including out-of-root paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
… API calls Bundle. The frontend shipped as a single 7.9 MB chunk (2.7 MB gzipped) that every visitor downloaded before the first paint. Three causes, all fixed: - @mlc-ai/web-llm, the in-browser LLM runtime and by far the largest dependency at 5.4 MB, was imported statically by an app-wide context provider. It has exactly one call site, already inside an async function, so it is now imported on demand when a user actually starts the assistant. - The router imported all 24 pages eagerly, so opening the dashboard also downloaded the charting, graph-rendering and syntax-highlighting code for every other screen. Pages are now React.lazy with a Suspense fallback. - AIAssistantPanel rendered unconditionally in MainLayout, pulling the markdown renderer and syntax highlighter into every page load for a panel that starts closed. It now mounts on first open and stays mounted so its transitions still run. Initial load is 394 KB across two chunks; the remaining 7.2 MB is split into 80 chunks fetched per route. manualChunks deliberately pins only React: naming the page-specific libraries forced each into one shared chunk that the entry preloaded even when no initial route needed it, which left 1.5 MB on the critical path. Redirected API calls. 26 call sites requested paths the backend declares with a trailing slash (`/api/runs` vs `/api/runs/`), so every one cost a 307 and a second round trip - including a POST that only survived because 307 preserves the body, and all of which break behind a proxy that does not forward redirects. All now request the declared path. WebSocket. Live step logs built their URL from window.location.host, so in remote-execution mode the socket connected to the wrong origin and silently fell back to polling. A new getWebSocketUrl() mirrors getBaseUrl(); the run id and step name are URL-encoded, and a socket that resolves after the effect is torn down is now closed instead of leaked. Adds the missing /ws entry to the Vite dev proxy, without which live logs never worked under `npm run dev`. Also reverts the SPA catch-all route introduced in the previous commit back to a 404 handler. A catch-all matches before Starlette's redirect_slashes logic, which turned every trailing-slash mismatch into a 404 - a regression caught by driving the real UI in a browser, where five pages were failing to load data. The handler now merges the SPA fallback with 5xx redaction, since Starlette allows one handler per exception type, and still returns JSON 404s for unknown /api and /ws paths. Verified by loading all 19 routes in headless Chromium: every one renders with no console errors, no page errors, no failed requests and no 4xx/5xx API responses. Adds a test asserting no frontend call depends on a trailing-slash redirect - the existing segment-wise matcher normalised slashes away and could not see this class. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
…ntend tests Date crash. 21 call sites formatted API timestamps with `format(new Date(value))`. date-fns raises `RangeError: Invalid time value` for an unparseable date, so one malformed record replaced an entire page with the error boundary. Reproduced against a running server: inserting a single run row with `start_time: "unknown"` took down the whole Dashboard. The inverse is equally wrong - `new Date(null)` is the Unix epoch, so a missing timestamp rendered as a real "Jan 1, 1970" date. All 21 now use the repository's existing `formatDate` helper, which returns "-" for anything unparseable. Verified by re-loading every route with the malformed record still present: all 19 render clean. Remote mode. AuthContext issued login, logout and session checks with a bare fetch(), and ErrorBoundary reported client errors the same way. A bare fetch always targets the page's own origin, so in remote-execution mode - where the API lives on another host - authentication could never work. They now go through fetchApi, with credentials: 'include' on the session endpoints so the cookie survives a cross-origin request. Testing. The frontend had no test runner and CI never touched it: no build check, no unit tests, and the workflow's path filter meant frontend changes did not even trigger a run, so a broken build surfaced only at release time when the wheel is packaged with a prebuilt bundle. Adds vitest with 19 unit tests covering formatDate's behaviour on every malformed input, and fetchApi / getWebSocketUrl in both local and remote modes, and adds a frontend CI job that runs them plus a production build. Adds tests/test_frontend_source_hygiene.py, five source-level guards in the Python suite for defects that already reached the running UI: raw date formatting, WebSocket URLs built from window.location.host, bare fetch() of an API path, eagerly imported routes, and a static import of the LLM runtime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
Three endpoints place a client-supplied string into a pip argument vector:
POST /api/plugins/install, POST /api/plugins/uninstall/{plugin_id} and
POST /api/deployments/dependencies/install. subprocess runs without a shell, so
there was no shell injection, but an argument vector carries its own hazards:
- Option injection. pip cannot distinguish a package named
"--index-url=http://evil/" from the option of that spelling, so an
unvalidated name could point the installer at an attacker-controlled index,
install from a local path, or enable --editable. The dependency installer was
the widest opening: its "allow direct package names too" branch appended any
string the caller sent.
- Self-removal. `pip uninstall -y flowyml` - or fastapi, or pydantic - would
disable the server while it was handling the request.
Adds flowyml/utils/packages.py, which validates against the PEP 508 grammar
(name, optional extras, optional version specifiers) and refuses anything that
reads as an option, URL, path, or second argument. Uninstall additionally
refuses a list of distributions the running server depends on, compared after
PEP 503 normalisation so pydantic_core and pydantic-core are the same name.
Requirements are re-validated inside the background install task, which runs
decoupled from the request that queued it.
Also fixes the dependency installer invoking a bare `pip`, resolved from PATH.
That can install into a different interpreter than the one serving requests, so
the endpoint reported success while the import kept failing. It now uses
sys.executable -m pip, as the other two call sites already did.
Adds tests/test_package_name_validation.py: 61 cases covering accepted
specifiers, option injection, URLs and paths, shell metacharacters, uninstall
protection including normalisation tricks, and the HTTP endpoints themselves -
including that a legitimate framework install still works.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
…N818 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
…onestly Wrong database. SQLiteMetadataStore is an alias for SQLMetadataStore, and calling it with no arguments opens the default local SQLite file, ignoring FLOWYML_DATABASE_URL. Five routers did that: experiments, leaderboard, metrics and evaluations constructed one directly, and templates built a fresh store - and therefore a fresh SQLAlchemy engine and connection pool - on every request, falling back to the default file whenever the variable was unset. docker-compose.yml points FLOWYML_DATABASE_URL at Postgres, so in the deployment this project documents, pipelines wrote experiments, metrics and evaluation results to Postgres while those endpoints read a SQLite file inside the container: permanently empty, and discarded on every restart. The Experiments page, the Leaderboard and the model-metrics views were therefore blank no matter how much data existed. assets.py had already been fixed - its comment reads "Use the main configured store (PostgreSQL in Docker deployment)" - but the others were missed. All now use get_store(). Silent storage failures. Four list handlers caught every exception and answered 200 with an empty collection plus an `error` key nothing reads. The UI does `data.runs || []`, so a database outage rendered the same "no runs yet" empty state as a new installation: no alert, no error status, invisible until a user asked where their data went. They now raise 500, which the application-wide handler logs with a correlation id and redacts in production. Making the backend honest is only half of it - the UI still swallowed the result. Adds fetchJson(), which throws an ApiError carrying the server's message instead of handing back a Response that callers took straight to .json(), and wires failure toasts into the runs, experiments, traces and projects pages. Adds an ErrorState component for pages that want an inline failure state rather than a toast. Verified end to end by breaking the store behind GET /api/runs/ in a live server: the API returns 500, the traceback is logged with a reference id, and the UI shows "Could not load runs: could not connect to server: Connection refused" where it previously showed an empty list. Adds tests/test_metadata_store_consistency.py (an AST-based guard so no backend module can construct a store directly again, plus a test that an experiment written to the configured store is visible through the API) and tests/test_list_endpoints_fail_loudly.py (a broken store must never produce a 200 or an empty collection, while genuinely empty results still succeed). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
POST /api/runs/{run_id}/steps/{step_name}/logs appends caller-supplied text to
a file whose path is assembled from the two URL parameters, and the matching
GET reads it back. Neither was confined.
Starlette matches a path parameter against a single segment, so an encoded
%2F is rejected during routing - but ".." is an ordinary segment. Verified
against a running server:
curl --path-as-is -X POST http://host/api/runs/../steps/x/logs \
-d '{"content":"INJECTED"}'
returned 200 and created .flowyml/logs/x.log, outside the runs directory, with
attacker-controlled content. The read endpoint and the dead-step scanner in the
same router, plus the AI context log reader, built their paths the same way.
Adds resolve_run_log_path() alongside the artifact confinement helpers: each
component is reduced to a single safe segment and the result is verified to sit
under the runs root, so neither traversal nor an absolute value escapes. All
four call sites now use it. A traversal attempt lands harmlessly at
runs/unknown-run/logs/ instead of writing outside the tree, and ordinary log
streaming is unchanged.
Adds tests/test_run_log_path_security.py: 18 cases covering traversal in each
component independently and together, absolute paths, empty components, the
guarantee that distinct runs do not collapse into one file, the original
exploit through the HTTP layer, and the log round trip that must keep working.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
dependencies.get_store() memoises a SQLAlchemy engine for the process, which is correct in production - one connection pool, not one per request - but nothing reset it between tests. BaseTestCase points config.metadata_db at a temporary directory per test, so a store built during one test kept serving every later test in the same worker, and rows written by one appeared in another's queries. This only became visible once the metrics router started using the shared store instead of constructing its own, which is the behaviour the Postgres deployment needs. conftest already clears the step registry, stack registry, stack manager and plugin config for the same reason; the store cache now joins them. Verified stable across three consecutive parallel runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
AuthMiddleware is a Starlette BaseHTTPMiddleware, which only processes scopes
of type "http". WebSocket handshakes pass straight through it, so in a
production deployment /ws/runs/{run_id}/logs and its per-step sibling accepted
any connection while every HTTP route demanded a bearer token. Verified against
the app: GET /api/runs/ answered 401 with no credentials, and the same client
opened a WebSocket successfully. Pipeline step logs routinely carry data paths,
hostnames and occasionally credentials, so that was a straightforward read
bypass for anyone who could reach the port.
Adds is_websocket_authorized(), which applies the same rules as the HTTP
middleware: development stays open, FLOWYML_ALLOW_INSECURE still opts out, a
missing server token fails closed, and comparison is constant-time. Browsers
cannot set request headers on a WebSocket handshake, so the existing HttpOnly
session cookie is accepted alongside an Authorization header for CLI and SDK
clients. A token in the query string is deliberately not accepted, matching the
HTTP decision: URLs end up in access logs and Referer headers.
Rejection closes the socket *before* accepting, so the handshake itself fails
with HTTP 403. Accepting first and closing afterwards would leave the client
believing it had connected.
Adds tests/test_websocket_security.py: 24 cases covering both endpoints across
missing, wrong, header and cookie credentials, malformed Authorization values,
the query-string rejection, fail-closed behaviour when the server token is
unset, the development and opt-out paths, and an end-to-end check that a posted
log still reaches a subscribed client.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
Model-explorer sessions live in the server process rather than the database, which is appropriate for interactive scratch state - but nothing ever expired them. A session was created for every model a user opened and removed only by an explicit delete, and every prediction and parameter sweep appended to a list that only grew. A long-running server accumulated all of it until it ran out of memory, and the growth was driven entirely by ordinary UI use. Sessions are now held in an OrderedDict capped at MAX_SESSIONS with least-recently-used eviction, so an abandoned browser tab cannot hold memory indefinitely while an actively used session survives. Per-session prediction and sweep history is capped at MAX_HISTORY_PER_SESSION, keeping the most recent entries, which is what the UI displays. Recording against an evicted session is a no-op rather than resurrecting it. Adds tests/test_explorer_session_bounds.py: 10 cases covering both caps, oldest-first eviction, that activity protects a session from being evicted by age, that the newest history is what survives truncation, and that ordinary create/read/delete/list still work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
…params _heartbeat_timestamps and _step_metrics record, per run and per step, when a step last reported in and what it reported. _cleanup_heartbeats was written to release that state but was never called from anywhere, so both dictionaries gained an entry per run and per step for the lifetime of the process: a server executing pipelines continuously leaked memory in proportion to how much work it had done. Cleanup now runs when a run is stopped and when a heartbeat observes a terminal run status. That alone is not sufficient - a crashed worker never reports anything again - so entries whose newest heartbeat is older than HEARTBEAT_RETENTION_SECONDS are pruned opportunistically on the next recorded heartbeat, with MAX_TRACKED_RUNS as a hard ceiling that evicts the least-recently-active runs. Dead-step detection is unaffected: its window is far shorter than the retention period. Also bounds the two remaining unbounded query parameters, found by an AST sweep for every defect class in this audit: the leaderboard's `n` and the schedule history's `limit` both accepted any integer. An AST sweep of the backend now reports no unconfined filesystem paths, no unbounded pagination parameters, no handlers returning 200 with an error payload, no bare `pip` invocations, and no unbounded in-memory state. Adds tests/test_heartbeat_state_bounds.py: 11 cases covering explicit cleanup, age-out for runs that stop reporting, the ceiling backstop, and the dead-step detection that the state exists to support. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
The suite failed intermittently with FileExistsError from
Path('.flowyml/cache').mkdir(parents=True, exist_ok=True) - a call that should
never raise that error. It does when the directory is removed between mkdir's
failure and its is_dir() recheck: with exist_ok=True, pathlib swallows
FileExistsError only if the path is a directory at the moment it looks, and
re-raises otherwise.
conftest deletes the cache directory between tests to stop cached step outputs
leaking across them, and the default cache path is relative, so every parallel
worker shared one directory and each worker's delete raced every other worker's
create. The failure surfaced in whichever test happened to be constructing a
CacheStore at the time, which made it look unrelated to caching.
Each worker now gets its own temporary cache directory via FLOWYML_CACHE_DIR,
which the config layer already honours. This also stops the suite from deleting
a developer's real .flowyml/cache.
Reproduced with `-n 16 --dist=load` (roughly one failure in six runs before the
change); ten consecutive runs at the same contention pass afterwards.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
`flowyml --version` printed "0.1.0" for release 2.2.0, and `flowyml status` showed the same. The literal lived in a place the release automation never touches: [tool.semantic_release].version_variables covers flowyml/__init__.py and the sidebar component, not the CLI, so the number had been wrong since the first release after 0.1.0. Anyone diagnosing a deployment from that output was reading a number that could not have been right. Both call sites now derive from flowyml.__version__, so there is nothing left to keep in sync. Adds tests/test_version_consistency.py, which checks the package version against pyproject and asserts that the CLI, /api/health, the OpenAPI document, /api/execution/info and the frontend sidebar all report it - and that the CLI's version_option is not a literal again. Verified against a clean install: building the wheel and installing it into a fresh virtualenv now yields `flowyml, version 2.2.0`, a UI backend that imports with all 108 routes, and all 19 pages rendering in a browser with no console errors, JS errors or failed requests. That import is the failure this branch started from: before the packaging fix the same install raised ModuleNotFoundError: No module named 'fastapi'. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
The auth changes on this branch make FLOWYML_ENV=production fail closed, so an operator following the existing docs would hit a startup refusal with no explanation of what changed or why. The reference material now matches the behaviour. .env.example and docker-compose.yml carry FLOWYML_ADMIN_PASSWORD, FLOWYML_ALLOW_INSECURE and FLOWYML_CORS_ORIGINS - previously undocumented even though the code read the first two - along with a note on why an empty FLOWYML_API_TOKEN is no longer accepted as "authentication off", and a one-liner for generating a token. infra/DEPLOYMENT.md marks FLOWYML_API_TOKEN and FLOWYML_ADMIN_PASSWORD as required in production, adds a "Failing closed" section quoting the actual startup error, and notes that an unset FLOWYML_DATABASE_URL silently falls back to a container-local SQLite file - the misconfiguration that made the Experiments and Leaderboard pages look permanently empty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
A run can live in the globally configured metadata store or in a project's own
store, so a lookup by id has to consult both. runs.py did; ai_context.py did
not. Each router carried its own copy of the same iteration loop and they
drifted, which showed up in the UI as a run detail page rendering perfectly
while the AI-context request behind it answered 404 for the very run on screen.
Reproduced against a running server with a real pipeline run:
GET /api/runs/{id} returned 200 and GET /api/ai/context/run/{id} returned 404
for the same id.
iter_metadata_stores() and find_run_across_stores() now live in dependencies,
next to get_store(), and runs.py, assets.py, experiments.py and ai_context.py
all delegate to them - removing three near-identical copies along with the
opportunity to drift again. A project store's name is still attached to runs
that do not already carry one, and an explicit project on the record wins.
Adds tests/test_cross_store_run_lookup.py: 9 cases covering lookup in each
store, project annotation, and - the point of the change - that the run detail
and AI context endpoints agree about whether a given run exists, for runs in
the global store, in a project store, and for one that exists nowhere.
Verified in a browser against a real pipeline run: the run detail page now
loads with no failed requests, where it previously logged a 404.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
…up failures /api/config answered 401 in production, which broke the bootstrap the login page depends on. The frontend reads it to resolve the API base URL, so in remote-execution mode - where the API lives on a different origin than the page - the browser could not learn where to send the login request in the first place. The endpoint is named get_public_config and returns only deployment topology: execution mode, server and UI URLs, and which services are remote. No credentials, and specifically not the API token. It is now in the public path set alongside /api/health, and a test asserts the response never contains the token. Startup refusal is now logged before it is raised. An ASGI server wraps a lifespan failure in its own traceback, so the message telling the operator which variable is missing was buried a dozen frames down. It appears as a plain ERROR line first. Verified in a browser against a production-configured server: an unauthenticated visitor is redirected to /login, the previously accepted default password "flowyml" is rejected, a correct password lands on a rendered dashboard, and no JS errors occur. Unauthenticated requests to /api/runs/, /api/assets/, /api/execution/tokens and /api/execution/execute all answer 401, while /api/config and /api/health answer 200. With no credentials configured, uvicorn exits non-zero and never binds the port. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
…ake pre-commit pass Three things blocked a merge. poetry.lock was stale. Changing the dependency and extras tables in pyproject.toml invalidated it, and `poetry check --lock` failed. Every CI job runs `poetry install --with dev --all-extras`, which refuses an inconsistent lock, so all four jobs died at the install step before running a single test. The lock is regenerated and consistent; resolution succeeds, and `--all-extras` now installs 316 packages against 289 before - the newly reachable azure, mlflow, wandb, keras and kfp extras, on top of the torch and tensorflow bulk that already dominated. Python 3.10 was broken. Two of the new test files imported `tomllib`, which is stdlib only from 3.11, so both would fail at import on the 3.10 leg of the test matrix. They now use `toml`, already a core dependency and available on every supported version. Every changed file - and every file in the package - is now parsed against the 3.10 grammar as a check. pre-commit failed, and had been failing before this branch: on main, ruff reported 4 errors and ruff-format wanted to rewrite 16 files. The hooks pin ruff v0.1.11, which formats differently from current releases, so the whole tree is now formatted by the pinned version and the lint errors are fixed. The one that could not be auto-fixed is A003 on `StackCache.set`, a false positive: the name is only ever reached as `cache.set(...)`, `get`/`set` is the interface the class implements and its own docstring examples use, and later ruff releases removed the rule for exactly this reason. It carries a targeted noqa with that rationale rather than a renamed public method. Two hooks were also rewriting generated files. `trailing-whitespace` stripped whitespace from inside the minified frontend bundle - and minified JS contains multi-line template literals where a trailing space is part of the string, so the hook was silently changing what the shipped program does. `pretty-format-json` reformatted package-lock.json, which npm rewrites back on the next install. Both now exclude the generated paths, matching the exclude `check-added-large-files` already carried. Verified: pre-commit passes all 12 hooks; all four CI test groups pass sequentially as the workflow runs them (smoke 27, integration 41, unit 1284, coverage 1284); the suite passes under `-n 8`; the 19 frontend vitest cases pass; the bundle builds; and all 19 UI routes render clean in a browser after the reformat and rebuild. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
…_mode option The unit-test workflow installed --all-extras, which after the packaging fixes on this branch meant 316 packages: gcp, aws, azure, kfp, sagemaker, mlflow, wandb and the genai stack on top of torch, tensorflow and a set of CUDA wheels. An AST audit of every import in tests/ finds exactly four optional modules - torch, tensorflow, keras and sklearn - and nothing else. tensorflow and scikit-learn already come from the dev group and keras ships inside tensorflow, so `pytorch` is the only extra the suite needs. The genai tests pass without langchain, langgraph or openai installed (they mock the clients), and no test imports mlflow, wandb, azure, kfp, sagemaker or google.cloud at all. The three test jobs now install 188 packages instead of 316 - and instead of the 289 they installed before this branch, so CI is lighter than it was rather than heavier. No coverage is lost: the extras' own correctness is asserted by tests/test_packaging_metadata.py, and the release workflow still performs a full --all-extras install, which is where a broken extra actually matters. Also removes `asyncio_mode = "auto"` from the pytest config. pytest-asyncio is not a dependency, so the option was silently unknown and emitted a PytestConfigWarning on every run. It was also a trap: a future bare `async def test_` would have been collected and never awaited. The suite's four async tests are driven by anyio's pytest plugin through `@pytest.mark.anyio`, which works because anyio ships with starlette - verified by breaking an assertion inside one and confirming it fails rather than passing vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
The `regexp` input in PRECOMMITS.yml was written with doubled backslashes
inside a single-quoted YAML scalar, which has no backslash escapes. Every
`\\` therefore reached JavaScript as a literal backslash, so the class
`[a-z,A-Z,0-9,\\-,\\_,\\/,:]` contained a `\` (0x5C) to `,` (0x2C) range
and `new RegExp` threw before reading any title:
SyntaxError: Invalid regular expression: Range out of order in
character class
The job died in five seconds whatever the pull request was called, and the
`pre-commit run` step after it never executed - so the workflow verified
nothing at all.
Replace it with the pattern the repository already uses for commit
messages (the `validate-commit-msg` hook), so one convention governs both
and a title accepted locally is accepted in CI. Scope stays optional and
the type list matches the hook, which is what the history on `main`
actually uses.
tests/test_ci_workflow_config.py compiles the pattern, asserts it stays
identical to the hook entry, and checks it against real titles from
`main` plus malformed ones.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1CErYvpggWAogJ9MDVqj4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.