Skip to content

Commit de2017e

Browse files
davidzhaou9g
andauthored
fix lk agent dev with Agents-JS (#900)
* fix `lk agent dev` with Agents-JS JS agents will also support the OnServerStart protocol * Gate the dev-channel flag on the installed agents SDK version (#901) * remove dead nil checks on devSrv, now created unconditionally Also normalizes an em dash in a nearby comment. * move interpreter-based SDK version resolution to pkg/agentfs Pure relocation, no behavior change: the Python/Node binary finders and installed-version probes (plus the embedded node_resolve_version.js) move from cmd/lk into pkg/agentfs as exported functions, with agentfs.ResolveInstalledSDKVersion as the shared entry point, so callers outside the startup checks can reuse the resolver. * pick the dev-channel flag from the installed SDK version An SDK must not be handed a flag it predates: Node's CLI hard-fails on unknown options, so every released @livekit/agents (<= 1.5.0) dies at startup when passed --cli-addr. Resolve the installed agents version via the project's own runtime and choose accordingly — --cli-addr when positively at/above the first supporting release (1.6.6 Python, 1.5.1 Node, placeholders until those releases exist), otherwise the legacy --reload-addr for Python and no flag at all for Node. Undetermined versions always take the safe branch. * remove agent_watcher_test.go * pre-warm node in TestAgentProcessFailSignal to avoid probe timeout On Windows CI runners the first node.exe spawn can take several seconds (Defender scans the binary on first exec), which blows the SDK version probe's 5s timeout inside startAgent — the test then fails with '@livekit/agents not found' at exactly 5.04s before the crash-signal behavior under test ever runs (same flake seen on main). Spawn a throwaway Node process first to absorb the cold start. --------- Co-authored-by: u9g <jason.lernerman@livekit.io>
1 parent cc8d66f commit de2017e

6 files changed

Lines changed: 235 additions & 131 deletions

File tree

cmd/lk/agent_run_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,13 @@ func TestAgentProcessFailSignal(t *testing.T) {
3535
if _, err := exec.LookPath("node"); err != nil {
3636
t.Skip("node not on PATH")
3737
}
38+
// The first node.exe spawn on a Windows CI runner can take several seconds
39+
// (Defender scans the binary on first exec), which blows the SDK version
40+
// probe's 5s timeout inside startAgent and fails the test before the
41+
// crash-signal behavior under test ever runs. Spawn a throwaway Node
42+
// process first to absorb the cold start.
43+
require.NoError(t, exec.Command("node", "-e",
44+
`console.log('pre-warming node so the SDK version probe does not time out on a cold runner')`).Run())
3845

3946
// An agent whose job crashes logs a marker but keeps the process alive;
4047
// Failed() must fire without waiting for exit.

cmd/lk/agent_watcher.go

Lines changed: 67 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,53 @@ func watchExtensions(pt agentfs.ProjectType) map[string]bool {
4141
return map[string]bool{".js": true, ".ts": true, ".mjs": true, ".mts": true}
4242
}
4343

44+
// firstPythonCLIAddrVersion is the first Python livekit-agents release that
45+
// accepts --cli-addr; releases up to and including 1.6.5 only accept the legacy
46+
// --reload-addr.
47+
const firstPythonCLIAddrVersion = "1.6.6"
48+
49+
// firstNodeCLIAddrVersion is the first @livekit/agents release that accepts
50+
// --cli-addr; releases up to and including 1.5.0 reject unknown options, so
51+
// older SDKs must not be passed any dev-channel flag.
52+
const firstNodeCLIAddrVersion = "1.5.1"
53+
54+
// devChannelAddrFlag picks the CLI flag used to hand the agent the dev-channel
55+
// address, or "" when the installed SDK predates any flag it would accept. The
56+
// installed version is resolved via the project's interpreter so symlinked/
57+
// workspace deps and loose constraints report what will actually run.
58+
func devChannelAddrFlag(config AgentStartConfig) string {
59+
return devChannelAddrFlagFor(config.ProjectType, agentfs.ResolveInstalledSDKVersion(config.Dir, config.Entrypoint, config.ProjectType))
60+
}
61+
62+
// devChannelAddrFlagFor implements the flag choice. The flag was renamed
63+
// --reload-addr -> --cli-addr (it now carries more than reloads), but an SDK
64+
// must not be handed a flag it predates, so anything not positively new
65+
// enough — including an undetermined version — falls back to what released
66+
// SDKs accept: the legacy --reload-addr for Python, no flag at all for Node
67+
// (whose CLI hard-fails on unknown options).
68+
func devChannelAddrFlagFor(projectType agentfs.ProjectType, installedVersion string) string {
69+
newEnough := func(minVersion string) bool {
70+
if installedVersion == "" {
71+
return false
72+
}
73+
ok, err := agentfs.IsVersionSatisfied(installedVersion, minVersion)
74+
return err == nil && ok
75+
}
76+
switch {
77+
case projectType.IsPython():
78+
if newEnough(firstPythonCLIAddrVersion) {
79+
return "--cli-addr"
80+
}
81+
return "--reload-addr"
82+
case projectType.IsNode():
83+
if newEnough(firstNodeCLIAddrVersion) {
84+
return "--cli-addr"
85+
}
86+
return ""
87+
}
88+
return ""
89+
}
90+
4491
// agentWatcher watches for file changes and restarts an agent subprocess.
4592
type agentWatcher struct {
4693
config AgentStartConfig
@@ -79,19 +126,21 @@ func newAgentWatcher(config AgentStartConfig) (*agentWatcher, error) {
79126
return nil, fmt.Errorf("failed to setup file watcher: %w", err)
80127
}
81128

82-
// The reload protocol (capture running jobs from the old process, restore
83-
// them in the new one) is Python-only; Node reloads are a plain kill+respawn.
84-
var rs *devServer
85-
if config.ProjectType.IsPython() {
86-
rs, err = newDevServer()
87-
if err != nil {
88-
w.Close()
89-
return nil, err
90-
}
91-
// Append --reload-addr to CLI args so the Python process connects back
92-
config.CLIArgs = append(config.CLIArgs, "--reload-addr", rs.addr())
129+
// The dev server backs two things over one channel: the ServerInfo the agent
130+
// reports on connect (e.g. for the Cloud console link) and the reload protocol
131+
// (capture running jobs from the old process, restore them in the new one). It
132+
// is created for every agent type so ServerInfo works; the job capture/restore
133+
// is Python-only and gated in restart() (Node reloads are a plain kill+respawn).
134+
rs, err := newDevServer()
135+
if err != nil {
136+
w.Close()
137+
return nil, err
93138
}
94139
rs.onServerInfo = config.OnServerInfo
140+
// The agent connects back to this address over the dev channel.
141+
if addrFlag := devChannelAddrFlag(config); addrFlag != "" {
142+
config.CLIArgs = append(config.CLIArgs, addrFlag, rs.addr())
143+
}
95144

96145
return &agentWatcher{
97146
config: config,
@@ -118,9 +167,6 @@ func (aw *agentWatcher) start() error {
118167
// acceptSession waits (in the background) for the next process to connect back on
119168
// the reload channel and hands the connection to a devSession read loop.
120169
func (aw *agentWatcher) acceptSession() {
121-
if aw.devSrv == nil {
122-
return
123-
}
124170
go func() {
125171
conn, err := aw.devSrv.listener.Accept()
126172
if err != nil {
@@ -133,9 +179,12 @@ func (aw *agentWatcher) acceptSession() {
133179
}
134180

135181
func (aw *agentWatcher) restart() error {
136-
// 1. Capture active jobs from the current process (best-effort)
182+
// 1. Capture active jobs from the current process (best-effort). Job
183+
// capture/restore is Python-only; Node reloads are a plain kill+respawn.
137184
if aw.session != nil {
138-
aw.devSrv.captureJobs(aw.session)
185+
if aw.config.ProjectType.IsPython() {
186+
aw.devSrv.captureJobs(aw.session)
187+
}
139188
aw.session.close()
140189
aw.session = nil
141190
}
@@ -181,9 +230,7 @@ func (aw *agentWatcher) Run(done <-chan struct{}) error {
181230
if aw.session != nil {
182231
aw.session.close()
183232
}
184-
if aw.devSrv != nil {
185-
aw.devSrv.close()
186-
}
233+
aw.devSrv.close()
187234
aw.watcher.Close()
188235
}()
189236

@@ -249,7 +296,7 @@ func (aw *agentWatcher) Run(done <-chan struct{}) error {
249296
case <-exitCh:
250297
// Nil the channel so this case won't fire again (nil channels block forever)
251298
exitCh = nil
252-
// Drain any pending debounce don't restart immediately
299+
// Drain any pending debounce - don't restart immediately
253300
if debounceTimer != nil {
254301
debounceTimer.Stop()
255302
debounceTimer = nil

cmd/lk/python_sdk_version_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func setupUvAgentProject(t *testing.T, stubVersion, depSpec string, sync bool) s
7272

7373
func TestResolvePythonAgentVersion_ReadsInstalledVersion(t *testing.T) {
7474
dir := setupUvAgentProject(t, "1.6.7", "livekit-agents", true)
75-
version, notInstalled := resolvePythonAgentVersion(dir, agentfs.ProjectTypePythonUV)
75+
version, notInstalled := agentfs.ResolvePythonAgentVersion(dir, agentfs.ProjectTypePythonUV)
7676
require.Equal(t, "1.6.7", version)
7777
require.False(t, notInstalled)
7878
}
@@ -112,7 +112,7 @@ func TestFindPythonBinary_UVRunDoesNotSyncEnvironment(t *testing.T) {
112112
require.NoError(t, err)
113113
require.NoError(t, os.WriteFile(stubPyproject, []byte(strings.Replace(string(orig), "1.6.7", "9.9.9", 1)), 0o644))
114114

115-
bin, prefixArgs, err := findPythonBinary(dir, agentfs.ProjectTypePythonUV)
115+
bin, prefixArgs, err := agentfs.FindPythonBinary(dir, agentfs.ProjectTypePythonUV)
116116
require.NoError(t, err)
117117
cmd := exec.Command(bin, append(prefixArgs, "-c", `import importlib.metadata as m; print(m.version("livekit-agents"))`)...)
118118
cmd.Dir = dir

cmd/lk/simulate_subprocess.go

Lines changed: 9 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -53,45 +53,6 @@ type AgentProcess struct {
5353
LogPath string
5454
}
5555

56-
// findPythonBinary locates a Python binary for the given project type.
57-
func findPythonBinary(dir string, projectType agentfs.ProjectType) (string, []string, error) {
58-
if projectType == agentfs.ProjectTypePythonUV {
59-
uvPath, err := exec.LookPath("uv")
60-
if err == nil {
61-
// --no-sync: the CLI proxies the environment as it exists on disk
62-
// and must never install or upgrade packages as a side effect.
63-
return uvPath, []string{"run", "--no-sync", "python"}, nil
64-
}
65-
}
66-
67-
// Check common venv locations
68-
for _, venvDir := range []string{".venv", "venv"} {
69-
candidate := filepath.Join(dir, venvDir, "bin", "python")
70-
if _, err := os.Stat(candidate); err == nil {
71-
return candidate, nil, nil
72-
}
73-
}
74-
75-
// Fall back to system python
76-
pythonPath, err := exec.LookPath("python3")
77-
if err != nil {
78-
pythonPath, err = exec.LookPath("python")
79-
if err != nil {
80-
return "", nil, fmt.Errorf("could not find Python binary; ensure a virtual environment exists or Python is on PATH")
81-
}
82-
}
83-
return pythonPath, nil, nil
84-
}
85-
86-
// findNodeBinary locates the Node binary used to run a JS/TS agent.
87-
func findNodeBinary() (string, error) {
88-
nodePath, err := exec.LookPath("node")
89-
if err != nil {
90-
return "", fmt.Errorf("could not find Node binary; ensure node is on PATH")
91-
}
92-
return nodePath, nil
93-
}
94-
9556
// isTypeScriptEntry reports whether the entrypoint is TypeScript source that
9657
// needs Node's type-stripping loader to run directly (no build step).
9758
func isTypeScriptEntry(entry string) bool {
@@ -139,39 +100,16 @@ func checkTypeStrippingSupport(dir, nodeBin string) error {
139100
// equivalent floor from server client settings.
140101
const nodeAgentMinVersion = "1.0.0"
141102

142-
// nodeResolveVersionScript asks Node to report the installed @livekit/agents
143-
// version using its own module resolution paths (so pnpm/workspace symlinks
144-
// and hoisting resolve exactly as they will at runtime). See the source file
145-
// for details.
146-
//
147-
//go:embed node_resolve_version.js
148-
var nodeResolveVersionScript string
149-
150-
// resolveNodeAgentVersion returns the installed @livekit/agents version as Node
151-
// resolves it from fromDir, or "" if it can't be determined.
152-
func resolveNodeAgentVersion(nodeBin, fromDir string) string {
153-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
154-
defer cancel()
155-
cmd := exec.CommandContext(ctx, nodeBin, "-e", nodeResolveVersionScript)
156-
cmd.Dir = fromDir
157-
out, err := cmd.Output()
158-
if err != nil {
159-
return ""
160-
}
161-
return strings.TrimSpace(string(out))
162-
}
163-
164103
// checkNodeSDKVersion gates a Node agent on nodeAgentMinVersion, resolving the
165-
// installed @livekit/agents from the entrypoint's directory so monorepo and
166-
// workspace layouts (where the dep is a workspace:* symlink, not a versioned
167-
// entry in the root package.json) report the version that will actually run.
104+
// installed @livekit/agents via the project's own runtime (see
105+
// agentfs.ResolveInstalledSDKVersion).
168106
func checkNodeSDKVersion(cfg AgentStartConfig) error {
169-
nodeBin, err := findNodeBinary()
170-
if err != nil {
107+
// Surface a missing Node binary as its own error rather than folding it
108+
// into "package not found".
109+
if _, err := agentfs.FindNodeBinary(); err != nil {
171110
return err
172111
}
173-
fromDir := filepath.Dir(filepath.Join(cfg.Dir, cfg.Entrypoint))
174-
version := resolveNodeAgentVersion(nodeBin, fromDir)
112+
version := agentfs.ResolveInstalledSDKVersion(cfg.Dir, cfg.Entrypoint, cfg.ProjectType)
175113
if version == "" {
176114
return fmt.Errorf("@livekit/agents not found; install dependencies and make sure this is a LiveKit agent project")
177115
}
@@ -182,50 +120,12 @@ func checkNodeSDKVersion(cfg AgentStartConfig) error {
182120
return nil
183121
}
184122

185-
// pythonResolveVersionScript prints the installed livekit-agents version, or a
186-
// sentinel when the interpreter runs fine but the package isn't installed —
187-
// distinguishing "dependencies not synced" from probe failures (no
188-
// interpreter, timeout), which exit non-zero.
189-
const pythonResolveVersionScript = `import importlib.metadata as m
190-
try:
191-
print(m.version("livekit-agents"))
192-
except m.PackageNotFoundError:
193-
print("` + pythonAgentNotInstalled + `")`
194-
195-
const pythonAgentNotInstalled = "__NOT_INSTALLED__"
196-
197-
// resolvePythonAgentVersion returns the installed livekit-agents version read
198-
// via the project's interpreter, so any installer (uv, pip, poetry) reports the
199-
// version that will actually run. notInstalled reports that the interpreter ran
200-
// but the package is missing from its environment; version is "" when it can't
201-
// be determined at all (no interpreter, probe failure, etc.).
202-
func resolvePythonAgentVersion(dir string, projectType agentfs.ProjectType) (version string, notInstalled bool) {
203-
pythonBin, prefixArgs, err := findPythonBinary(dir, projectType)
204-
if err != nil {
205-
return "", false
206-
}
207-
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
208-
defer cancel()
209-
args := append(append([]string{}, prefixArgs...), "-c", pythonResolveVersionScript)
210-
cmd := exec.CommandContext(ctx, pythonBin, args...)
211-
cmd.Dir = dir
212-
out, err := cmd.Output()
213-
if err != nil {
214-
return "", false
215-
}
216-
v := strings.TrimSpace(string(out))
217-
if v == pythonAgentNotInstalled {
218-
return "", true
219-
}
220-
return v, false
221-
}
222-
223123
// checkPythonSDKVersion gates a Python agent on thinCLIMinVersion. It prefers
224124
// the installed version (resolved via the interpreter, accurate regardless of
225125
// the package manager and not fooled by a loose version constraint); when
226126
// dependencies aren't installed it falls back to static project-file parsing.
227127
func checkPythonSDKVersion(cfg AgentStartConfig) error {
228-
version, notInstalled := resolvePythonAgentVersion(cfg.Dir, cfg.ProjectType)
128+
version, notInstalled := agentfs.ResolvePythonAgentVersion(cfg.Dir, cfg.ProjectType)
229129
if notInstalled && cfg.ProjectType == agentfs.ProjectTypePythonUV {
230130
// The launch runs `uv run --no-sync`, so a missing package won't be
231131
// installed on the way up — fail fast with the fix instead.
@@ -351,7 +251,7 @@ const thinCLIMinVersion = "1.6.0"
351251
// where the type-stripping flag lets a `.ts` entrypoint run without a build.
352252
func buildAgentCommand(cfg AgentStartConfig) (string, []string, error) {
353253
if cfg.ProjectType.IsNode() {
354-
nodeBin, err := findNodeBinary()
254+
nodeBin, err := agentfs.FindNodeBinary()
355255
if err != nil {
356256
return "", nil, err
357257
}
@@ -368,7 +268,7 @@ func buildAgentCommand(cfg AgentStartConfig) (string, []string, error) {
368268
return nodeBin, args, nil
369269
}
370270

371-
pythonBin, prefixArgs, err := findPythonBinary(cfg.Dir, cfg.ProjectType)
271+
pythonBin, prefixArgs, err := agentfs.FindPythonBinary(cfg.Dir, cfg.ProjectType)
372272
if err != nil {
373273
return "", nil, err
374274
}

0 commit comments

Comments
 (0)