Add lms launch XYZ - #594
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
github-actions
Bot
added
the
CLA signed
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce23c8c74b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| `Assume "yes": no interactive prompts; auto-confirm model loads and config writes.`, | ||
| ) | ||
| .allowUnknownOption(true) | ||
| .passThroughOptions(true); |
There was a problem hiding this comment.
Commander documents passThroughOptions() as passing through options that come after command arguments; with [tool] as the first argument, lms launch claude --dry-run and lms launch claude --context-length 8192 put those tokens in toolArgs instead of options. Only --model is recovered by peekModelFromArgs, so the other launch-owned flags are silently ignored and forwarded to the tool, including --dry-run/--print-env which should prevent spawning.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed on the branch.
Root cause was passThroughOptions(true): Commander stops parsing options at the first operand (the [tool] argument), so any lms-owned flag placed after the tool name (--dry-run, --print-env, -c, --api-key, ...) was diverted into toolArgs and forwarded to the tool instead of being honored. The peekModelFromArgs hack rescued only --model, which is exactly the inconsistency you flagged.
Change:
- Dropped
passThroughOptions(true), keptallowUnknownOption(true). lms now consumes its known flags wherever they appear; unknown tool flags still flow intotoolArgsin order;--forwards everything after it to the tool verbatim. - Removed the now-obsolete
peekModelFromArgs/stripModelFlag/injectsModelArgmachinery (argPeek.tsdeleted).
Verified against commander@14 that the documented examples now behave correctly:
launch claude --model X --dry-run→{ tool: "claude", dryRun: true, model: "X", toolArgs: [] }(previously--dry-runwas silently forwarded to claude)launch claude --resume abc --print-env→printEnv: true,toolArgs: ["--resume","abc"]launch claude -- --model Y→toolArgs: ["--model","Y"]
One intended tradeoff: a tool flag that collides with an lms flag (e.g. codex -c key=val) is now consumed by lms — use launch codex -- -c key=val to forward it verbatim. This is documented in the [toolArgs...] help.
| function quoteArgIfNeeded(arg: string): string { | ||
| return arg === "" || /\s/.test(arg) ? JSON.stringify(arg) : arg; |
There was a problem hiding this comment.
--print-env is described as shell-eval'able, but this only quotes arguments containing whitespace; shell metacharacters such as $(), backticks, globs, ;, or & remain live. If a caller preserves a literal tool arg like '--flag=$(touch /tmp/pwn)' when invoking lms, the generated eval "$(lms launch ... --print-env)" output executes the substitution instead of passing the literal argument, so the command and all args need real POSIX/PowerShell quoting rather than whitespace-only quoting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed on the branch.
Replaced the whitespace-only quoteArgIfNeeded — and the double-quote env escaping, which also left $/backtick live in PowerShell — with real single-quote quoting per shell:
- POSIX: wrap in
'…', embedded'→'\''(close/escape/reopen). - PowerShell: wrap in
'…', embedded'→'', and prefix the command line with the&call operator (a quoted first token is otherwise a string literal, not an invocation).
Every emitted token (command, args, and env values) is now single-quoted, so $(), backticks, globs, ;, &, |, and whitespace are all inert.
Verified end-to-end: eval-ing the --print-env output in real bash and iex-ing it in real PowerShell with a --flag=$(touch pwned) argument (and a $(...) env value) passes them to the child process as literal strings — no command substitution runs and no file is created, in both shells.
| const models = (await client.system.listDownloadedModels()).filter( | ||
| model => model.architecture?.toLowerCase().includes("clip") !== true, | ||
| ); |
There was a problem hiding this comment.
This interactive list only filters out CLIP models, but the launch path later always loads the selected download with client.llm.load(...). In an installation with downloaded embedding models, those non-LLM entries appear as valid choices; selecting one returns its modelKey and then fails when the LLM namespace tries to load an embedding model, instead of hiding it or reporting that no launchable LLM is available.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed on the branch.
The picker now filters to model.type === "llm" (the same discriminant list.ts and chat/react/hooks.tsx already use), so embedding models are excluded from the launch list instead of being selectable and then failing in client.llm.load(...). The existing CLIP exclusion is kept, and the empty-state message now reads "No downloaded LLMs found."
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb0dbf5695
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "-c", | ||
| `model_providers.${PROVIDER_ID}.base_url=${ctx.openaiBaseUrl}`, | ||
| "-c", | ||
| `model_providers.${PROVIDER_ID}.wire_api=chat`, |
There was a problem hiding this comment.
With the current @openai/codex CLI, this override makes lms launch codex fail while loading its config: OpenAI's Codex config reference says model_providers.<id>.wire_api only supports responses and defaults to it (https://developers.openai.com/codex/config-reference). Since this adapter always emits wire_api=chat before invoking Codex, users with current Codex installs cannot launch the advertised Codex integration; use responses or omit the override.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f37ad72.
You're right — this would have broken lms launch codex for anyone on a current Codex. Current Codex removed Chat Completions support (Feb 2026) and only speaks the Responses API, but the adapter pinned wire_api=chat.
Since LM Studio serves the Responses API at /v1/responses (v0.3.29+), switching to responses is a clean fix rather than a blocker: Codex posts to {base_url}/responses → http://127.0.0.1:1234/v1/responses, which LM Studio implements.
Change (adapters/codex.ts):
model_providers.lmslaunch.wire_api→responses.- Reworded the note so the
-- -c …wire_api=chatoverride is the legacy fallback (pre-2026 Codex + older LM Studio), not the default. - Updated the unit test to assert
wire_api=responses.
model_context_window is unaffected and still emitted.
| await prepared?.cleanup?.(); | ||
| await rm(workDir, { recursive: true, force: true }); |
There was a problem hiding this comment.
These cleanup calls also run after the --print-env branch returns, so the printed shell command can reference state that has already been torn down. For example, aider prints a --model-metadata-file path inside workDir, and droid relies on the temporary settings entry from its cleanup; eval "$(lms launch ... --print-env)" therefore invokes a command whose required file was deleted or whose model entry was restored before the command starts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f37ad72.
The finally teardown (cleanup() + rm(workDir)) ran unconditionally, including after the --print-env early return — so the emitted command referenced an aider --model-metadata-file that had already been deleted, or a droid ~/.factory/settings.json entry that had already been reverted, before the user ever ran it.
Change (index.ts): teardown is now skipped when --print-env is set, so those resources outlive the process and the printed command actually works. Every other path (spawn, --dry-run, errors) still cleans up.
I also threaded a printEnv flag through LaunchContext so droid's note is honest in that mode — it now says the entry is left in place and not auto-reverted, instead of promising it'll be reverted on exit.
| provider: "generic-chat-completion-api", | ||
| }; | ||
| if (ctx.contextLength !== undefined) { | ||
| entry.maxOutputTokens = ctx.contextLength; |
There was a problem hiding this comment.
ctx.contextLength is the model's input/context window, but Factory documents maxOutputTokens as the maximum output tokens for responses (https://docs.factory.ai/cli/byok/overview). Because resolveModelForLaunch normally supplies a context length even when the user did not pass --context-length, every lms launch droid can write very large output caps like 32K/128K while still not conveying a context-window hint, which can make Droid request or display an invalid response budget.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f37ad72.
Confirmed against the Factory BYOK docs: a customModels entry has no context-window field, and maxOutputTokens is the completion cap — so mapping the model's context length onto it advertised a bogus response budget while conveying no context hint.
Change (adapters/droid.ts):
- Dropped the
contextLength → maxOutputTokensmapping, and removed the now-unusedmaxOutputTokensfield fromDroidCustomModel. - Set
supportsContextHint: false, since Factory genuinely exposes no context knob.lms launch droid -c Nnow takes the honest-degradation path: the model is still loaded at N tokens (the effective window) and lms warns that droid has no verified context-length knob, rather than inventing an output cap. - Added a unit test asserting
droid.supportsContextHint === false.
- codex: use wire_api=responses (current Codex removed Chat Completions; LM Studio serves /v1/responses) instead of the now-rejected chat mode - droid: stop mapping context length onto maxOutputTokens (Factory's output cap, not a context window); mark supportsContextHint=false - launch: skip config teardown under --print-env so the emitted command's resources (aider metadata file, droid settings entry) survive
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f37ad72926
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const resolved = await resolveModelForLaunch(client, logger, { | ||
| model: modelQuery, | ||
| contextLength: options.contextLength, | ||
| yes, | ||
| }); |
There was a problem hiding this comment.
When --print-env is used with a model that is not already loaded, this resolution path can call loadModelWithSpinner, whose Spinner writes progress and terminal-clear escape sequences to process.stdout before the shell script is printed. Command substitution such as eval "$(lms launch claude --model X --print-env)" then receives those bytes before the export/command lines and can fail or execute garbage; suppress progress or send it to stderr for the print-env path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b20ba92.
Right — under --print-env, if the requested model wasn't already loaded, resolveModelForLaunch → loadModelWithSpinner ran the Spinner, which writes progress frames plus the \r\x1B[K / cursor-visibility escapes to process.stdout. Those bytes landed ahead of the export … / command lines, so eval "$(lms launch … --print-env)" ingested them.
Change:
Spinnernow takes an optional output stream, defaulting toprocess.stdout— the other three callers (chat,load,logout) are untouched.- Threaded a
printEnvflag throughResolveModelForLaunchOpts→loadModelWithSpinner; in print-env mode the spinner is constructed withprocess.stderr.
stdout on the print-env path is now exactly the shell script. The other status output there — prepared.notes via logger.warn, and the "already loaded at a smaller context" warning — already routes through createLogger, whose Console is wired to process.stderr (logLevel.ts), so nothing else leaks.
| console.info(); | ||
| console.info(chalk.dim(`! "droid" reads its model list from ${filePath}.`)); |
There was a problem hiding this comment.
For interactive lms launch droid --print-env without -y, these console.info calls write the prompt preface to stdout before formatEnvForShell prints the shell command. Because --print-env is meant for command substitution, eval "$(...)" will try to parse this human text as shell input even when the model is already loaded; write this prompt text to stderr/logger like the confirm prompt.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b20ba92.
Confirmed — the two console.info calls (the blank line and ! "droid" reads its model list from …) wrote to stdout, while the confirm prompt immediately below already used { output: process.stderr }. Under lms launch droid --print-env without -y, that preface polluted the command-substitution stdout.
Change (adapters/droid.ts): both are now console.error (stderr), consistent with the prompt. It's unconditional rather than print-env-gated — interactive prompt context belongs on stderr in every mode — so stdout stays clean whether or not --print-env is set.
| args.push("-c", `model_context_window=${ctx.contextLength}`); | ||
| } | ||
| const env: Record<string, string> = { | ||
| OPENAI_API_KEY: ctx.apiKey, // some Codex builds require a non-empty key even though unused |
There was a problem hiding this comment.
When the LM Studio endpoint enforces a bearer token, lms launch codex --api-key ... still won't authenticate: Codex custom providers read the key from the env var named by model_providers.<id>.env_key (see https://developers.openai.com/codex/config-advanced), but this adapter only sets OPENAI_API_KEY and never adds the matching model_providers.lmslaunch.env_key=OPENAI_API_KEY override. The custom provider therefore sends no bearer token for secured local endpoints.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b20ba92.
Right — the adapter set OPENAI_API_KEY in the child env but never told the custom provider to read it, so per the config-advanced docs Codex sent no Authorization header and --api-key couldn't authenticate a secured LM Studio endpoint.
Change (adapters/codex.ts): added -c model_providers.lmslaunch.env_key=OPENAI_API_KEY, naming the env var the adapter already populates, so Codex forwards it as the bearer token. Updated the codex unit test's expected arg list and the no-context-length length assertion (10 → 12).
Keep `--print-env` stdout eval-clean and let Codex authenticate against secured LM Studio endpoints. - Spinner: add an optional output stream (defaults to stdout, so the chat/load/logout callers are unchanged). Thread a `printEnv` flag through `resolveModelForLaunch` -> `loadModelWithSpinner` so model-load progress goes to stderr under `--print-env`, leaving the emitted shell script as the only thing on stdout. - droid: send the interactive prompt preface to stderr (`console.error`), matching the confirm prompt below it, so `eval "$(lms launch droid --print-env)"` never ingests human text as shell input. - codex: add `model_providers.lmslaunch.env_key=OPENAI_API_KEY` so the custom provider actually forwards `--api-key` as the bearer token to a secured endpoint; update the codex unit tests.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b20ba920cf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| opts.contextLength, | ||
| opts.printEnv === true, | ||
| ); | ||
| } catch { |
There was a problem hiding this comment.
When client.llm.load() rejects for a reason other than “model not found” (for example an unloadable model, an invalid requested context, or a runtime/server error), this catch still assumes the artifact is missing and calls the download path. For an already-owned owner/name, downloadArtifact can hit its no-download branch and process.exit(0), so lms launch --model owner/name exits successfully without launching or reporting the real load failure; only fall back to downloading after distinguishing a genuine not-found error.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 092b576.
You're right — the bare catch treated every client.llm.load() rejection as "model not downloaded", so a genuine load failure on an already-owned owner/name — an invalid --context-length, an unloadable/embedding model, or a server/runtime error — fell through to downloadArtifact, hit its no-op branch, and process.exit(0). lms launch --model owner/name then exited 0 without launching or reporting the real failure.
Change (resolveModel.ts): the catch (loadError) now checks whether the artifact is already present before falling back to download — client.system.listDownloadedModels() matched by modelKey (hub models are keyed owner/name, the same discriminant list.ts/getLLM.ts use). If it's already downloaded the failure is real → throw loadError, so it surfaces with a non-zero exit. Only a genuinely-missing owner/name proceeds to the JIT download + retry.
| first with "lms get ${modelQuery}". | ||
| `); | ||
| } | ||
| await downloadArtifact(client, logger, ownerName.owner, ownerName.name, opts.yes); |
There was a problem hiding this comment.
When --print-env is used with a missing owner/name model, this call routes through the normal lms get downloader, whose plan/progress rendering writes directly to process.stdout before the final shell snippet is printed. That makes eval "$(lms launch claude --model owner/name --print-env)" receive download tables/escape sequences as shell input; the model-load spinner is redirected for print-env, but this JIT-download path still pollutes stdout.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 092b576.
Right — the model-load spinner was redirected to stderr for --print-env, but the JIT-download fallback still routed through the lms get planner, which writes plan tables and /cursor escapes straight to process.stdout (and can prompt for confirmation). eval "$(lms launch claude --model owner/name --print-env)" would then ingest that as shell input.
Rather than thread an output stream through the shared get downloader, --print-env now refuses to JIT-download: if the requested owner/name isn't present it throws a UserInputError telling the user to run lms get owner/name first, then re-run with --print-env. Kicking off a multi-GB interactive download inside command substitution (captured stdout, possible confirmation prompt) isn't something to do implicitly anyway. The already-loaded and already-downloaded print-env paths still emit only the shell script, and non-print-env launches are unchanged (they still JIT-download and retry).
- resolveModel: don't treat every model-load failure as a missing model. On a load error for an already-downloaded owner/name (bad --context-length, unloadable/embedding model, server/runtime error), surface the real error instead of falling into downloadArtifact's no-op branch, which process.exit(0)s and silently masked the failure (lms launch --model owner/name exited 0 without launching). Distinguish via client.system.listDownloadedModels() by modelKey. - resolveModel: under --print-env, refuse to JIT-download a genuinely-missing model. The get planner renders plan tables + cursor escapes to stdout (and can prompt for confirmation), which command substitution would ingest. Instruct the user to run 'lms get owner/name' first so print-env stdout stays a clean, eval-able shell script.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 092b576353
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const bind = cfg?.networkInterface; | ||
| // A server bound to 0.0.0.0 (all interfaces) is still reached locally over loopback. | ||
| const host = opts.host ?? (bind === undefined || bind === "0.0.0.0" ? "127.0.0.1" : bind); | ||
| const port = opts.port ?? cfg?.port ?? DEFAULT_SERVER_PORT; |
There was a problem hiding this comment.
When lms launch is pointed at a remote instance with --host but no explicit --port, createClient defaults the SDK connection to port 1234, while this resolver still substitutes the locally persisted REST port from getServerConfig(). If the user's local LM Studio server was last run on a different port, the launched tool is configured with http://<remote>:<local-port>/v1 and the readiness check/start attempt targets the wrong remote port. Only use the persisted config for local launches, or pass through the effective port used by createClient.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ab4a4e4.
Right — with --host set but no --port, createClient connects on DEFAULT_SERVER_PORT (1234) and never reads the locally-persisted port for a remote, but ensureRestServer still substituted cfg.port from getServerConfig(). So the tool was wired to http://<remote>:<local-port>/v1 and the readiness probe/start targeted the wrong remote port.
Change (serverReady.ts): getServerConfig() is now consulted only for local launches (opts.host === undefined). For a remote launch cfg is undefined, so host = opts.host and port = opts.port ?? DEFAULT_SERVER_PORT — exactly mirroring the connection createClient makes — and the local networkInterface no longer leaks into the remote bind either. Local launches are unchanged (opts.port ?? cfg?.port ?? DEFAULT_SERVER_PORT).
| // Deliberately no maxOutputTokens: it is Factory's output-completion cap, not a context-window | ||
| // hint, so mapping the model's context length onto it would advertise a bogus response budget. | ||
|
|
||
| if (!ctx.yes && process.stdin.isTTY === true) { |
There was a problem hiding this comment.
When droid is launched without -y from a non-TTY context, this condition skips the confirmation instead of failing, then falls through to rewrite ~/.factory/settings.json. In particular, lms launch droid --print-env in a script permanently leaves the new custom model entry because print-env teardown is skipped, even though the user did not opt into the --yes auto-confirm behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ab4a4e4.
Right — !ctx.yes && process.stdin.isTTY === true meant a non-TTY launch without -y fell straight through to rewriting ~/.factory/settings.json with no confirmation, and under --print-env (teardown skipped) that entry was left permanently — all without the user opting into --yes.
Change (adapters/droid.ts): the guard is now if (!ctx.yes), and inside it a non-TTY stdin throws a UserInputError asking the user to re-run with -y/--yes (or from an interactive terminal) instead of silently modifying the file. TTY without -y still prompts on stderr, and -y still auto-confirms. It applies to every non-interactive path (spawn, dry-run, print-env), since writing a real user file without consent is the issue in all of them.
- serverReady: don't apply locally-persisted REST config to remote launches. With --host but no --port, createClient connects on DEFAULT_SERVER_PORT (never the locally-saved port), but ensureRestServer still substituted cfg.port, wiring the tool to http://<remote>:<local-port>/v1 and probing the wrong remote port. getServerConfig() is now consulted only for local launches; remote mirrors createClient (opts.port ?? DEFAULT_SERVER_PORT on the requested host). - droid: require confirmation when stdin is non-interactive. Without -y from a non-TTY, prepare() skipped the confirm and rewrote ~/.factory/settings.json anyway (permanently, under --print-env where teardown is skipped). It now throws UserInputError asking for -y instead of silently modifying a real user file.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab4a4e41a6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const modelQuery = options.model; | ||
| const yes = options.yes ?? false; | ||
|
|
||
| await using client = await createClient(logger, options); |
There was a problem hiding this comment.
When a local user supplies --port to let lms launch start/configure the REST endpoint on a non-default port, this passes that port into createClient before ensureRestServer runs. createClient only takes the local findOrStartLlmster path when port is undefined; with a port it probes http://127.0.0.1:<port>/lmstudio-greeting and exits if the REST server is stopped, so ensureRestServer never gets a chance to auto-start it. This makes lms launch claude --port 4321 ... fail from a stopped server despite the command's advertised server auto-start behavior; keep the SDK/control connection on the normal local path and pass the REST port only to the REST readiness/start logic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f1ba673.
Right — --port on a local launch designates the REST endpoint the tool talks to, but it was also handed to createClient, which only takes the findOrStartLlmster path when port is undefined. With a port set it probed http://127.0.0.1:<port>/lmstudio-greeting and process.exit(1)'d when the REST server was stopped, so ensureRestServer never got to auto-start it — lms launch claude --port 4321 failed on a stopped server despite the advertised auto-start.
Change (index.ts): the SDK/control connection now stays on the normal local path — createClient is called with port: options.host === undefined ? undefined : options.port, so a local launch no longer forwards --port to it. ensureRestServer still receives options.port and owns the REST readiness/start on that port. Remote launches (which connect straight to host:port) are unchanged.
| { output: process.stderr }, | ||
| ), | ||
| ); | ||
| return picked.modelKey; |
There was a problem hiding this comment.
In installations with the same model key on multiple devices, or with a model only available on a non-local device, the picker shows a device suffix but then returns only modelKey, discarding picked.deviceIdentifier. The subsequent client.llm.load(...) call therefore lets the SDK choose the preferred/default device (the existing lms load path explicitly passes deviceIdentifier), so selecting a remote/device-specific entry can load the wrong copy or fail even though the chosen entry was available. Return the selected model/device through the resolver and pass deviceIdentifier when loading.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f1ba673.
Good catch — the picker rendered a device suffix but returned only modelKey, dropping picked.deviceIdentifier, so client.llm.load(...) fell back to the SDK's preferred/default device (unlike lms load, which passes deviceIdentifier).
Change (resolveModel.ts): pickModelInteractively now returns the full ModelInfo; resolveModelForLaunch reads picked.deviceIdentifier and threads it through loadModelWithSpinner into client.llm.load(modelKey, { …, deviceIdentifier }). So an interactively-selected device-specific entry (same key on multiple devices, or a non-local device) loads that exact copy. The --model path leaves deviceIdentifier undefined, preserving the SDK's own device resolution.
| async prepare(ctx) { | ||
| const modelConfig: { limit?: { context: number } } = {}; | ||
| if (ctx.contextLength !== undefined) { | ||
| modelConfig.limit = { context: ctx.contextLength }; |
There was a problem hiding this comment.
When ctx.contextLength is known, this emits limit: { context: ... } for the inline OpenCode config. I checked the current OpenCode schema at https://opencode.ai/config.json, and a model limit object requires both context and output; with only context, lms launch opencode produces an invalid model config whenever a context length is available, which can make OpenCode reject or ignore the injected local model limits. Either include a valid output limit or omit the limit block instead of marking this adapter as having a verified context hint.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f1ba673.
Confirmed against https://opencode.ai/config.json — a model limit object requires both context and output, so emitting limit: { context } alone was invalid and OpenCode would reject/ignore it.
We only know the model's loaded context window, not a real output cap, and inventing one would advertise a bogus response budget (the same trap the droid maxOutputTokens fix avoided). So rather than emit a partial/fabricated block, the adapter now omits limit entirely and sets supportsContextHint: false — the LM Studio server still enforces the context length it loaded the model at, and lms launch opencode -c N takes the honest-degradation path (loads at N, warns opencode has no verified context knob). Updated the opencode unit tests accordingly.
- index: keep the REST --port out of createClient for local launches. --port designates the REST endpoint the tool talks to; feeding it to createClient diverted the SDK/control connection off the findOrStartLlmster path to a direct probe that process.exit(1)s on a stopped server, before ensureRestServer could auto-start it (launch --port 4321 failed despite advertised auto-start). Only remote launches keep passing host:port to createClient. - resolveModel: carry the interactively-selected deviceIdentifier into the load. The picker now returns the full ModelInfo; resolveModelForLaunch threads its deviceIdentifier into client.llm.load so multi-device installs load the chosen copy instead of the SDK's preferred/default device. --model stays undefined. - opencode: stop emitting an invalid partial limit block. OpenCode's model limit requires both context and output; we only know context and won't invent an output cap, so we omit limit entirely and set supportsContextHint=false (the server still enforces the loaded window). Tests updated.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1ba673ac7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (originalRaw !== undefined) { | ||
| await writeFile(filePath, originalRaw, "utf-8"); | ||
| } else { | ||
| await rm(filePath, { force: true }); | ||
| } |
There was a problem hiding this comment.
When a non---print-env droid launch exits, this restores the entire original ~/.factory/settings.json snapshot (or deletes the file if it did not exist). If the launched droid process or the user changes Factory settings while that session is running, those later writes are lost on cleanup because this code writes the old snapshot back wholesale. Remove only the LM Studio (lms launch) entry from the current file instead, so unrelated settings changes survive.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4ca0c41.
Right — cleanup wrote the whole pre-launch snapshot back (or deleted the file), so anything droid or the user changed in ~/.factory/settings.json during the session — other custom models, unrelated settings — was clobbered on exit.
Change (adapters/droid.ts): cleanup now re-reads the current file and strips only our "LM Studio (lms launch)" entry via a new removeDroidSettingsEntry helper (symmetric to mergeDroidSettings), so concurrent changes survive. It drops the file only when we created it and nothing else remains, and falls back to the wholesale snapshot restore solely when the current file is missing/unparseable (surgical removal impossible). Added unit tests covering: removes only our entry while keeping session-added models, preserves changed top-level keys, drops customModels when it was only ours, and no-ops when our entry is absent.
- droid: preserve concurrent Factory settings edits on cleanup. The teardown restored the entire pre-launch ~/.factory/settings.json snapshot wholesale, so any customModels/settings the droid session (or user) wrote while running were clobbered on exit. cleanup now re-reads the current file and removes only our 'LM Studio (lms launch)' entry via a new removeDroidSettingsEntry helper, dropping the file only when we created it and nothing else remains, and falling back to the snapshot restore only if the current file is missing/unparseable. Added unit tests for removeDroidSettingsEntry.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ca0c415f5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| const loaded = await client.llm.listLoaded(); | ||
| const existing = await firstMatchingLoaded(loaded, modelQuery); |
There was a problem hiding this comment.
When the interactive picker selects a copy of a model on a specific device, but another instance with the same modelKey is already loaded on a different device, this reuse path ignores deviceIdentifier and firstMatchingLoaded can return the wrong loaded instance before the later device-aware load call runs. Fresh evidence: picked.deviceIdentifier is assigned above and passed only to loadModelWithSpinner, while this early reuse check still matches solely by identifier/model key, so multi-device installs can silently launch against a device the user did not choose.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in bbb4880.
Fair — you caught the gap my round-6 device fix left open: deviceIdentifier was threaded into the load call, but the earlier firstMatchingLoaded reuse check still matched by identifier/modelKey alone, so a same-key copy already loaded on another device would be returned before the device-aware load ran.
Change (resolveModel.ts): firstMatchingLoaded now takes deviceIdentifier. When the user picked a specific device, both the identifier match and the modelKey match additionally require info.deviceIdentifier === deviceIdentifier, so reuse is confined to the chosen device; if nothing matching is loaded there, it falls through to the device-aware load. For --model (deviceIdentifier undefined) the matching is unchanged.
- resolveModel: respect the picked device when reusing an already-loaded model. The round-6 device fix passed deviceIdentifier to the load call, but the earlier firstMatchingLoaded reuse check still matched by identifier/modelKey alone, so a same-key copy loaded on another device could be returned before the device-aware load ran -- silently launching against a device the user didn't pick. firstMatchingLoaded now takes deviceIdentifier and, when the user picked a device, only reuses an instance on that same device (undefined for --model keeps the prior matching).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbb488096f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| return { | ||
| command: COMMAND, | ||
| args: [], |
There was a problem hiding this comment.
When the forwarded Droid invocation is non-interactive, such as lms launch droid --model X -- exec ..., this adapter writes the custom model entry but returns no -m/--model argument, so Droid can keep using its current/default model instead of the LM Studio model that lms launch resolved. Factory's CLI reference documents model selection through -m, --model, so the launch wrapper should pass the injected model id/name (while still allowing user args to override it if needed).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks — the underlying concern is right (the settings entry alone doesn't make droid use the model), but passing --model isn't safe here, so I've addressed it differently.
Factory's droid --model does not reliably accept custom BYOK models: it expects a generated id like custom:<DisplayName-dashified>-<index> (not the plain model id or the displayName), and there's an open bug — Factory-AI/factory#787 — where droid exec --model rejects every custom model id (all three tried forms: the custom:...-index id, the displayName, and an alternate) with "Invalid model". Per that issue only sessionDefaultSettings.model works via the flag, and the custom:...-index id is undocumented and index-dependent, so we can't reliably reproduce it.
So injecting --model <id> would risk breaking lms launch droid with an "Invalid model" error rather than fixing it. Instead (a38dc3c) I added a note directing the user to select the entry through droid's own /model picker — the documented selection path (per the Groq/Factory BYOK guides). If #787 lands and the id format is documented, I'm glad to inject it then; meanwhile user -- <args> still forward verbatim, so anyone with a known-working custom:... id can pass -- --model custom:... themselves.
| ANTHROPIC_DEFAULT_OPUS_MODEL: ctx.model, | ||
| ANTHROPIC_DEFAULT_SONNET_MODEL: ctx.model, | ||
| ANTHROPIC_DEFAULT_HAIKU_MODEL: ctx.model, | ||
| ANTHROPIC_DEFAULT_FABLE_MODEL: ctx.model, |
There was a problem hiding this comment.
If the caller already has CLAUDE_CODE_SUBAGENT_MODEL exported, spawnToolAndWait preserves it because only these keys are overlaid on process.env, and Claude Code documents that variable as the subagent model override. In that environment, subagents can still target a cloud/nonexistent Anthropic model even though the main and tier defaults are pinned here; set this variable to the local model/inherit value or explicitly clear it for the child.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a38dc3c.
Confirmed against code.claude.com/docs/en/model-config — CLAUDE_CODE_SUBAGENT_MODEL overrides subagent/agent-team model resolution ahead of the tier defaults, and since spawnToolAndWait spawns with { ...process.env, ...extraEnv }, a value already exported in the caller's shell survived untouched and could point subagents at a cloud/nonexistent Anthropic model.
Change (adapters/claude.ts): added CLAUDE_CODE_SUBAGENT_MODEL: ctx.model alongside the four tier pins, so subagents resolve to the local model too. Test updated.
| async prepare(ctx) { | ||
| const config = { | ||
| $schema: "https://opencode.ai/config.json", | ||
| model: `lmstudio/${ctx.model}`, |
There was a problem hiding this comment.
OpenCode merges config sources rather than replacing them, and small_model is a separate setting for lightweight/background tasks. Because the inline config only sets the main model, any existing global/project small_model remains in effect, so lms launch opencode can still send those tasks to a previously configured cloud/default provider instead of LM Studio; set small_model to the same lmstudio/... model as part of this injected config.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a38dc3c.
Confirmed on opencode.ai/docs/config — small_model is a separate top-level setting for lightweight/background tasks (e.g. title generation), and OpenCode merges config sources, so a global/project small_model would keep routing those to a previously-configured provider even though the main model is set.
Change (adapters/opencode.ts): the injected config now also sets small_model: lmstudio/${ctx.model} (same provider/model form as model). Test updated.
- claude: pin CLAUDE_CODE_SUBAGENT_MODEL to the local model. It overrides subagent/agent-team model resolution ahead of the tier defaults (code.claude.com/docs/en/model-config), so an inherited value from the caller's shell would otherwise route subagents to a cloud/nonexistent model, since spawnToolAndWait passes the parent env through for keys we don't set. - opencode: also pin small_model. OpenCode merges config sources and small_model (title/lightweight tasks) is separate, so a global/project small_model would keep routing those to a previously configured provider. - droid: do NOT inject --model. Factory's --model does not reliably accept custom BYOK models (open bug Factory-AI/factory#787 rejects them as "Invalid model"), and the custom id format is undocumented/fragile -- passing it would risk breaking launches. Added a note telling the user to select the entry via droid's "/model" instead. - tests updated for the claude subagent var and opencode small_model.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a38dc3cb44
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await client.system.startHttpServer({ | ||
| port, | ||
| cors: false, | ||
| networkInterface: bind ?? "127.0.0.1", |
There was a problem hiding this comment.
When the local REST server is stopped but the persisted server config came from a prior lms server start --bind 0.0.0.0 or a LAN address, bind is that public interface and this auto-starts the server on it for a plain local lms launch. The normal server start path only reuses the saved port and defaults its bind to 127.0.0.1, with a warning if the user explicitly asks for a non-loopback bind; launch should not silently expose the inference server to the network just because the last status file recorded a public bind.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a8bdb36.
Right — after the round-5 change a local launch still derived host/bind from the persisted networkInterface, so if the server was stopped and the saved config came from a prior lms server start --bind 0.0.0.0 (or a LAN address), lms launch would connect over and auto-start the inference server on that public interface — exposing it to the network without the user asking launch to.
Change (serverReady.ts): local launches now ignore the persisted networkInterface entirely — host is always 127.0.0.1 and the auto-start passes networkInterface: "127.0.0.1", reusing only the saved port (matching lms server start, which defaults its bind to loopback). An already-running 0.0.0.0-bound server is still reachable over loopback, so that keeps working; a server bound only to a specific LAN IP now needs an explicit --host <ip>, which is the honest way to opt into a non-loopback endpoint. Remote (--host) launches are unchanged.
- serverReady: never reuse the persisted (possibly public) bind for local launches. When the local REST server was stopped and the saved config recorded a 0.0.0.0/LAN networkInterface, ensureRestServer connected over and auto-started the inference server on that public interface for a plain 'lms launch', silently exposing it. Local launches now always use loopback (host 127.0.0.1) and reuse only the saved port -- matching 'lms server start', which defaults its bind to 127.0.0.1 -- and the auto-start binds 127.0.0.1 explicitly. An already-running 0.0.0.0-bound server is still reachable over loopback; remote (--host) is unchanged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8bdb3613f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // it, a global/project `small_model` would keep routing those tasks to a previously | ||
| // configured provider, so point it at the same local model as the main one. | ||
| small_model: `lmstudio/${ctx.model}`, | ||
| provider: { |
There was a problem hiding this comment.
When lms launch opencode runs with an existing OpenCode config that sets enabled_providers, this inline config adds provider.lmstudio but leaves that inherited allowlist untouched. OpenCode documents that config sources are merged and that enabled_providers allows only the listed providers (https://opencode.ai/docs/config/), so a user/project config such as enabled_providers: ["anthropic"] makes OpenCode ignore the injected LM Studio provider while model points at lmstudio/...; include lmstudio in the inline allowlist for this invocation (and clear an inherited disabled entry if needed).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4d33fd1.
Confirmed on opencode.ai/docs/config — enabled_providers is an allowlist ("only the specified providers will be enabled and all others will be ignored") and disabled_providers takes priority over it. So a user/project enabled_providers: ["anthropic"] (or a disabled_providers containing lmstudio) would make OpenCode drop the injected lmstudio provider even though model/small_model point at lmstudio/....
Change (adapters/opencode.ts): the inline config now sets enabled_providers: ["lmstudio"] and disabled_providers: [], so this session resolves through the injected provider regardless of an inherited allowlist/blocklist. It scopes the launch to lmstudio, which is the intent — everything already points at lmstudio/.... Test updated.
|
|
||
| let prepared: PreparedLaunch | undefined; | ||
| try { | ||
| prepared = await adapter.prepare(ctx); |
There was a problem hiding this comment.
For lms launch droid --dry-run, this calls adapter.prepare(ctx) before the dry-run branch, and the Droid prepare path prompts and writes ~/.factory/settings.json before returning. That means a dry run can still modify a real user config temporarily, expose the injected entry to a concurrent Factory process, or fail in a non-TTY shell unless -y is supplied even though the user only asked to print a plan; defer Droid's write/confirmation until after the dry-run branch or make prepare side-effect-free for dry runs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4d33fd1.
Right — adapter.prepare(ctx) runs before the --dry-run branch, and droid's prepare wrote ~/.factory/settings.json (and prompted, or threw in a non-TTY without -y) before returning, so a plan-only run had real side effects.
Change: threaded a dryRun flag through LaunchContext (alongside printEnv) and set it from --dry-run in index.ts. droid.prepare now short-circuits when ctx.dryRun — before any fs access — returning a preview (command + a "Would write …" note) with no file write, no confirmation prompt, and no cleanup. The real spawn path is unchanged. Added a unit test asserting droid's dry-run prepare is side-effect-free. Other adapters only ever write into the per-launch temp workDir, so they're already side-effect-free for the user.
- opencode: enable the injected provider under an inherited allowlist/blocklist. enabled_providers is an allowlist (others ignored) and disabled_providers takes priority over it, so a user/project 'enabled_providers: ["anthropic"]' (or an lmstudio disable) made OpenCode ignore our provider while model/small_model point at lmstudio/... The inline config now sets enabled_providers: ["lmstudio"] and clears disabled_providers. - launch/droid: make --dry-run side-effect-free. index called adapter.prepare() before the dry-run branch, and droid's prepare wrote ~/.factory/settings.json and prompted (failing in non-TTY without -y) even for a plan-only run. Threaded a dryRun flag through LaunchContext; droid.prepare now returns a preview (no write, no prompt) before any fs access when dryRun is set. - tests: opencode allowlist assertions, droid dry-run side-effect-free test, makeCtx dryRun default.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d33fd1097
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| return; | ||
| } | ||
| const cleaned = removeDroidSettingsEntry(current, DISPLAY_NAME); |
There was a problem hiding this comment.
When ~/.factory/settings.json already contained an "LM Studio (lms launch)" model entry, this cleanup removes that entry from the current file after a normal launch instead of restoring the pre-launch value. A common way to hit this is after a previous --print-env run intentionally left the entry in place; the next non---print-env launch overwrites it during prepare and then deletes it here, corrupting the user's Factory settings. Track whether the entry existed before launch and restore that original entry rather than always removing by display name.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7b5d417.
Good catch — this was a regression in my round-7 surgical cleanup. If ~/.factory/settings.json already held an "LM Studio (lms launch)" entry (classically, one an earlier --print-env run deliberately left in place), a subsequent normal launch's prepare() overwrote it and cleanup then removed it by displayName — deleting the user's version instead of restoring it.
Change (adapters/droid.ts): prepare now captures the pre-launch entry with our displayName (originalEntry) before overwriting. On cleanup, after stripping our launch-time entry, it re-adds originalEntry via mergeDroidSettings when one existed, otherwise just drops the entry we added. So a pre-existing entry is restored to its original value, a fresh one is removed, and unrelated concurrent changes are still preserved.
| enabled_providers: ["lmstudio"], | ||
| disabled_providers: [], |
There was a problem hiding this comment.
This only clears the older provider allow/block lists. OpenCode's current config docs say OPENCODE_CONFIG_CONTENT is merged with existing configs and non-conflicting settings are preserved, while the policies docs say a provider denied by experimental.policies is unavailable even if otherwise configured correctly. With a project config such as deny provider.use on * then allow anthropic, lmstudio remains denied despite this inline provider/model config, so lms launch opencode can still reject or ignore the injected provider. Include an inline policy override for lmstudio in the runtime config too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7b5d417.
Right — experimental.policies is a separate gate ahead of the allowlist. Per the policies docs, it currently governs provider.use, matching is last-match-wins, and an unmatched provider defaults to allowed.
Change (adapters/opencode.ts): the inline config now appends experimental.policies: [{ effect: "allow", action: "provider.use", resource: "lmstudio" }]. Because it's last-match-wins, this re-enables lmstudio under a project-level deny provider.use *. Two honest caveats: policies currently control only which providers OpenCode may use, so this touches no other user protection; and — as the docs note — a user's global deny still outranks a project/injected allow by design (OpenCode preventing a repo from re-enabling a globally-denied provider), so such a user would add lmstudio to their own global policy. Combined with enabled_providers/disabled_providers, the common project-config cases now resolve through LM Studio. Test updated.
Sources: OpenCode policies, config.
- droid: restore a pre-existing settings entry on cleanup instead of deleting it. If ~/.factory/settings.json already had an "LM Studio (lms launch)" entry (e.g. left by an earlier --print-env run), prepare() overwrote it and round-7's cleanup then removed it by displayName, losing the user's original. prepare now captures the pre-launch entry and cleanup restores it (via mergeDroidSettings) when it existed, still surgically removing only our own addition otherwise. - opencode: also override an experimental.policies provider deny. Policies gate provider.use ahead of the allowlist; they are last-match-wins, so the inline config now appends an explicit allow for lmstudio, re-enabling it under a project-level "deny provider.use *". Test updated.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b5d417a51
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| addCommandsGroup("Local models", [chat, get, load, unload, ls, ps, importCmd], "#22D3EE"); | ||
| addCommandsGroup("Serve", [server, log], "#34D399"); | ||
| addCommandsGroup("Launch (Beta)", [launch], "#FB923C"); |
There was a problem hiding this comment.
Registering launch as a pass-through command here exposes it to the pre-Commander version check below (commandArguments.includes("-v") || commandArguments.includes("--version")). For any forwarded tool version flag, such as lms launch codex -- --version or lms launch aider -- -v, the process exits before launch can parse the -- separator, so the advertised verbatim forwarding path cannot invoke the tool.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d37632c.
Right — the pre-Commander shortcut did a blunt commandArguments.includes("-v"|"--version") over all args, so lms launch codex -- --version (or lms launch aider -- -v) matched and printed the lms version + exited before launch ever parsed --, defeating the verbatim forwarding.
Change (src/index.ts): the shortcut now only fires for a -v/--version that appears before the first subcommand token (commandArguments.slice(0, firstSubcommandIndex)). lms -v / lms --version still work; after a subcommand the flag belongs to it, and launch forwards everything past -- to the wrapped tool. As a bonus this also stops lms <anysub> --version from masquerading as the lms version in general.
- index: scope the pre-Commander -v/--version shortcut to top-level flags. It scanned every argument, so a forwarded tool version flag (lms launch codex -- --version, lms launch aider -- -v) printed the lms version and exited before launch could parse '--' and forward it. Now only -v/--version appearing BEFORE the subcommand triggers the shortcut; after a subcommand the flag belongs to it (and launch forwards past '--' to the wrapped tool).
1 participant
lms launch: wire coding CLIs to your local LM Studio server in one commandAdds
lms launch <tool>— a single-command bridge that ensures the LM Studio REST inference server is running, resolves or loads amodel, translates everything into the tool's own configuration surface (env vars / CLI args), and hands the terminal over to the coding
CLI.
What it does
Key design decisions
ensures both are running.
surface, no bare-spawn EINVAL. Always uses 127.0.0.1 (not localhost, which resolves to ::1 first on Windows).
auto-compaction is loading the model at a large context window — exactly what ollama launch documents. We pass -c to client.llm.load()
and set verified per-tool secondary hints where available.
files are cleaned up in a finally block.
a tool without a context knob → warn, don't invent flags.
Module layout
src/subcommands/launch/
index.ts Commander command + orchestration action
types.ts LaunchContext, PreparedLaunch, ToolAdapter interfaces
registry.ts resolveAdapter, catalog formatting, ruled-out tools
serverReady.ts ensureRestServer (two-server thesis)
resolveModel.ts JIT-load/pick model; read back real context length
spawnTool.ts cross-spawn + signals + exit code propagation
argPeek.ts peek --model from passthrough args (read-only)
format.ts shell-detection, env formatting, dry-run output
launch.test.ts Pure function unit tests
adapters/
claude.ts ANTHROPIC_BASE_URL (bare origin), all-tier pinning
copilot.ts COPILOT_PROVIDER_* env vars with offline mode
aider.py --model lm_studio/ + model metadata file
codex.ts Explicit custom provider via -c overrides
opencode.ts Inline OPENCODE_CONFIG_CONTENT config delivery
droid.ts ~/.factory/settings.json merge + backup/restore lifecycle
Supported tools
Not supported: Gemini CLI (no OpenAI-compatible path in stable), Cursor CLI (backend rejects localhost).
Fixes #539