feat(cli): discover AgentServer in standalone CLI#1735
feat(cli): discover AgentServer in standalone CLI#1735rosetta-livekit-bot[bot] wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 9cb8aec The changes in this PR will be included in the next version bump. This PR includes changesets to release 34 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| ).action(async (entrypoint: string | undefined, command: Command) => { | ||
| const opts = { ...program.opts(), ...command.opts() } as RunOptions; | ||
| await runDiscoveredServer({ entrypoint, production: true, opts }); |
There was a problem hiding this comment.
🔴 Commander action handler receives options object instead of Command instance due to positional argument
In Commander v12, action handlers for commands with positional arguments receive parameters in the order (arg1, options, command). The start [entrypoint] command has one positional arg, so the handler receives 3 arguments: (entrypoint, optionsObject, commandInstance). However, the handler only declares two parameters (entrypoint, command), which means command actually receives the plain options object {logLevel: 'info'} (the 2nd arg), not the Command instance (the 3rd arg). When command.opts() is subsequently called, it throws TypeError: command.opts is not a function because plain objects don't have an .opts() method. This contrasts with the existing cli.ts which correctly handles commands WITHOUT positional arguments using (...[, command]) to skip the options object and receive the Command instance.
| ).action(async (entrypoint: string | undefined, command: Command) => { | |
| const opts = { ...program.opts(), ...command.opts() } as RunOptions; | |
| await runDiscoveredServer({ entrypoint, production: true, opts }); | |
| ).action(async (entrypoint: string | undefined, _options: unknown, command: Command) => { | |
| const opts = { ...program.opts(), ...command.opts() } as RunOptions; | |
| await runDiscoveredServer({ entrypoint, production: true, opts }); | |
Was this helpful? React with 👍 or 👎 to provide feedback.
| this.#opts.wsURL = opts.wsURL || this.#opts.wsURL; | ||
| this.#opts.apiKey = opts.apiKey || this.#opts.apiKey; | ||
| this.#opts.apiSecret = opts.apiSecret || this.#opts.apiSecret; | ||
| this.#opts.workerToken = opts.workerToken || this.#opts.workerToken; | ||
| this.#opts.logLevel = opts.logLevel || this.#opts.logLevel; |
There was a problem hiding this comment.
🟡 updateOptions skips workerToken Cloud deployment constraints enforced by constructor
The AgentServer constructor (agents/src/worker.ts:309-323) enforces Cloud deployment constraints when workerToken is set: it resets loadFunc to defaultCpuLoad and loadThreshold to the production default. However, updateOptions at line 393 simply assigns opts.workerToken without replicating these constraints. If a user creates an AgentServer without a workerToken (allowing custom loadFunc/loadThreshold) and the CLI later provides --worker-token via updateOptions, the server will run in Cloud mode with potentially incompatible custom load settings, bypassing the safety checks the constructor was designed to enforce.
Prompt for agents
In agents/src/worker.ts, the updateOptions method at line 390-394 sets workerToken without applying the same Cloud deployment constraints that the constructor applies at lines 309-323. When workerToken is newly set via updateOptions, the method should also check and reset loadFunc to defaultCpuLoad and loadThreshold to Default.loadThreshold(this.#opts.production) with appropriate log warnings, mirroring the constructor logic. This ensures that Cloud deployment constraints are always enforced regardless of whether the workerToken was provided at construction time or later via updateOptions.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const server = await discoverAgentServer(entrypoint); | ||
| server.updateOptions({ | ||
| apiKey: opts.apiKey, | ||
| apiSecret: opts.apiSecret, | ||
| logLevel: opts.logLevel, | ||
| workerToken: opts.workerToken, | ||
| wsURL: opts.url, | ||
| }); | ||
| await runAgentServer({ logLevel: opts.logLevel, production, server, watch: false }); |
There was a problem hiding this comment.
🚩 Production mode mismatch between CLI and discovered AgentServer
The start command in livekit-agents.ts:149 always passes production: true to runAgentServer. This affects SIGINT/SIGTERM drain behavior and log formatting. However, the discovered AgentServer instance was constructed by the user's module with its own production flag (which determines loadThreshold, numIdleProcesses, and port defaults at agents/src/worker.ts:222-246). The updateOptions method doesn't include production in its allowed fields. This means if a user constructs their AgentServer with production: false defaults and then runs livekit-agents start, the server will have dev-mode pool settings (0 idle processes, infinite load threshold) while the CLI assumes production behavior (drain on SIGINT). This might be intentional (letting users fully configure their exported server), but it could surprise users who expect start to imply full production defaults.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Verification
Notes
Ported from livekit/agents#6024
Original PR description
No description.