fix(agent): trust the resolved livekit-agents version in the SDK check; add Windows test runner#894
fix(agent): trust the resolved livekit-agents version in the SDK check; add Windows test runner#894u9g wants to merge 6 commits into
Conversation
u9g
changed the title
|
Should we perhaps just check the actual installed version instead? This would save us from parsing any requirements/lock files and guarantee that this is the version that will be used: uv run python -c \
'from importlib.metadata import version; \
print(version("livekit-agents"))' |
|
|
||
| // pythonResolveVersionScript prints the installed livekit-agents version, or | ||
| // exits non-zero if it isn't importable. | ||
| const pythonResolveVersionScript = `import importlib.metadata as m; print(m.version("livekit-agents"))` |
There was a problem hiding this comment.
Let's make sure this isn't executing any livekit-agents code. Otherwise, this could be a security vulnerability.
Let's make sure importlib is only reading metadata
There was a problem hiding this comment.
Well, the interpreter starts, and the interpreter runs all .pth files of all dependencies which means all those files will be ran, but this is just a check before running uv run, so I don't see what the vulnerability could be here.
The cloud build (for create and deploy subcommands on the lk cli) resolves dependencies from the project/lock files, so uv.lock is the source of truth for what will actually run in deployment; the locally installed version could differ (or not exist) and would be the wrong thing to check. |
checkUvLock matched a `livekit-agents = "x"` line that never appears in uv.lock. uv.lock uses poetry.lock's TOML layout ([[package]] / name / version), so the resolved version was never found, and the SDK version check fell back to the pyproject.toml constraint floor — reading `>=1.0` as 1.0 and rejecting it as older than the minimum (e.g. 1.6.0) even when the lock resolved a newer version. Use the same [[package]] block regex as checkPoetryLock so the resolved version is read and preferred, matching how the Node lockfile parsers already read the installed version. Also replaces the test's fake uv.lock fixture with the real format and adds a regression test for the pyproject-floor-vs-resolved-version case.
…check lk agent start/dev/console gated the Python SDK version by parsing project files, which can't know the resolved version from a loose constraint (and misread e.g. >=1.0 as 1.0). Resolve the installed version via the project interpreter (importlib.metadata) like the Node path already does, so the check reflects what will actually run regardless of installer. Falls back to static file parsing when dependencies aren't installed (e.g. before a sync). uv reads the existing env with --no-sync so the pre-flight check never syncs/downloads.
Combine the two Windows cross-builds into a single zig cross-build job in windows.yaml that produces lk.exe plus a test binary per package. Both the unit-test run (windows-tests) and the session e2e run (windows-session-e2e) depend on it, so the expensive cgo compile happens once. Move the Windows jobs out of session-e2e.yaml (now Linux/macOS only) and fold the standalone test-windows.yaml in.
u9g
force-pushed
the
fix/uv-lock-sdk-version-parsing
branch
from
02e3e10 to
1ecdb6e
Compare
Totally fair, I've misinterpreted the scope initially.
I'd argue we should just add |
The CLI proxies the project's local environment; `uv run` implicitly re-locking and installing packages when launching an agent is a surprising side effect (and can change dependency versions mid-debug). Move --no-sync into findPythonBinary so both the version pre-flight and the launch run against the environment as it exists on disk. Since the launch no longer installs missing dependencies, the version probe now distinguishes "interpreter ran but livekit-agents isn't installed" and the pre-flight fails fast with a `uv sync` hint instead of letting the agent die with ModuleNotFoundError.
The launch path now uses `uv run --no-sync`; deps come from the explicit `uv sync` step in CI.
Stopping only signaled the spinner goroutine, which could sleep up to 80ms before clearing the line — so an error printed right after landed on the leftover "Starting agent" text. Block the stop function until the clear escape has been written.
This is a great point, I did it. |
3 participants
The Python agent SDK-version check (
lk agent start/dev/console, and the build-time check) could reject a project whose installedlivekit-agentsactually satisfies the minimum. This PR makes the check trust the resolved/installed version, and adds Windows test coverage for the package.1. Parse the resolved version from
uv.lock(pkg/agentfs)checkUvLockmatched alivekit-agents = "x"line that never appears inuv.lock— the file uses poetry.lock's TOML layout ([[package]]/name/version). So the resolved version was never found and the check fell back to thepyproject.tomlconstraint, misreading the lower bound (>=1.0→1.0) and failing it against the minimum (e.g. 1.6.0). Now it uses the same[[package]]regex ascheckPoetryLock, so the resolved version is read and preferred.2. Resolve the installed version for the run check (
cmd/lk)lk agent start/dev/consolenow resolves the installedlivekit-agentsvia the project interpreter (importlib.metadata), the way the Node path already does — accurate regardless of installer (uv/pip/poetry) and not fooled by a loose version constraint. For uv it reads the existing environment with--no-sync, so the pre-flight check never syncs or downloads. When dependencies aren't installed (e.g. before a sync), it falls back to static project-file parsing (the uv.lock fix above, then constraints).3. Windows CI (
.github/workflows/windows.yaml)test.yamlcovers Linux/macOS Go tests andsession-e2e.yaml'se2ejob covers Linux/macOS session e2e, but Windows ran essentially no Go tests: mingw GCC can't compile the WebRTC APM C++ (MSVC SEH), and the native cgo link overflows the command-line limit. This adds a singlewindows.yamlthat — like.goreleaser.yaml— cross-compiles on Linux with zig and runs natively onwindows-latest:cross-buildjob produceslk.exeplus a test binary per package (discovered viago list, currentlycmd/lk,pkg/agentfs,pkg/bootstrap,pkg/util).windows-tests(needscross-build) runs every package's tests from its directory so cwd-relative fixtures resolve; runs on PRs, no secrets.windows-session-e2e(needscross-build) drives the session lifecycle against the prebuiltlk.exe; skipped on PRs since it needs LiveKit secrets.Both consumers depend on the one
cross-build, so the expensive cgo compile happens once. The Windows jobs were moved out ofsession-e2e.yaml(now Linux/macOS only) into this workflow. Windows is amd64-only here, matching the prior session-e2e Windows arm;build.yamlstill builds windows/arm64 for release.Tests
pkg/agentfs: realuv.lockfixture (replacing a fake one that only matched the broken regex) + a regression test that a loose pyproject floor doesn't override a satisfying resolved version.cmd/lk:resolvePythonAgentVersion/checkPythonSDKVersionexercised against a real uv project with a local stublivekit-agentspackage (known version, no network, no real SDK) — reads installed version, rejects too-old, installed beats a loose constraint, and falls back to file parsing when not installed. These run on Linux, macOS, and Windows.Note
This is the first time the full Go suite runs on Windows, so the initial run may surface pre-existing Windows-specific issues in tests unrelated to this change.