Skip to content

CLI: Rename unsloth connect to unsloth start#6613

Merged
wasimysaid merged 55 commits into
unslothai:mainfrom
NilayYadav:rename-connect-to-start
Jul 3, 2026
Merged

CLI: Rename unsloth connect to unsloth start#6613
wasimysaid merged 55 commits into
unslothai:mainfrom
NilayYadav:rename-connect-to-start

Conversation

@NilayYadav

Copy link
Copy Markdown
Collaborator

The unsloth connect command has been renamed to unsloth start and expanded to launch multiple coding agents (Claude Code, OpenAI Codex, OpenClaw, OpenCode, Hermes) against a running Unsloth Studio server.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request renames the CLI command unsloth connect to unsloth start across the entire codebase, updating CLI entry points, helper scripts, tests, and frontend components. It also introduces a new UI section in the frontend settings to display and copy the coding agent command. Feedback on the changes suggests fixing a comma splice in the newly added English translation string to improve grammatical correctness and readability.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread studio/frontend/src/i18n/locales/en.ts Outdated
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dacc19461e

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const KEY_PLACEHOLDER = "sk-unsloth-YOUR_KEY";
const MODEL_FALLBACK = "unsloth/gemma-4-E4B-it-GGUF:UD-Q5_K_XL";

const AGENT_COMMAND = "unsloth start claude";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include the selected Studio URL in the agent command

When this panel is showing a non-default local port or a tunneled/remote base URL, the copied agent command still contains neither UNSLOTH_STUDIO_URL nor --api-key. find_studio_server() defaults to http://127.0.0.1:8888 unless UNSLOTH_STUDIO_URL is set, and _agent_api_key() refuses automatic minting for non-loopback servers without an explicit key, so users following this snippet can connect to the wrong local server or fail instead of launching against the server shown in the UI. Build the snippet from the selected base/available key or show the required env vars for these cases.

Useful? React with 👍 / 👎.

The API keys panel showed a hardcoded `unsloth start claude`. `unsloth start`
defaults to 127.0.0.1:8888 and only mints a key for a loopback server, so a
non-default port or a tunnel/remote base would target the wrong server or fail
to mint. Build the command from the panel base/key (and emit a key for
non-loopback), matching the other snippets in the panel.
Avoids breaking existing scripts and docs that still call `unsloth connect`.
The test builds _SameTaskStreamingResponse via __new__, so set the attribute
that __call__ now reads.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

@shimmyshimmer

Copy link
Copy Markdown
Member

Pushed three commits to wrap up the feedback here.

1. The coding-agent command now targets the server the panel is showing.
The API keys panel was emitting a hardcoded unsloth start claude. Since unsloth start discovers the server from UNSLOTH_STUDIO_URL (default 127.0.0.1:8888) and only auto-mints a key for a loopback server, copying that command against a non-default port or a tunnel/remote base would hit the wrong server or fail to mint. It now builds the command from the panel's base/key (and appends --api-key for a non-loopback base), the same way the other snippets do. The logic sits in a small agent-command.ts so it is unit-testable.

2. unsloth connect still works as a hidden deprecated alias for unsloth start, so existing scripts and docs do not break after the rename.

3. Fixed an unrelated pre-existing failure in Repo tests (CPU). test_same_task_response_closes_body_iterator_on_send_disconnect builds _SameTaskStreamingResponse via __new__ and never set _unstarted_cleanup, which __call__ now reads. Stubbed it alongside the other attributes the test sets.

The grammar note on codingAgentsHint is already two sentences in the current text.

Verified locally: unsloth_cli tests (58 passed), the stream-cancel suite (25 passed), unsloth connect/unsloth start both resolve to the five agents, frontend typecheck and production build pass, eslint and prettier clean, i18n parity clean, and the command builder was checked across default-local, non-default port, tunnel, and remote bases on both unix and windows.

@danielhanchen

Copy link
Copy Markdown
Member

Pushed three commits to wrap up the review feedback.

1. Frontend loopback check now matches the CLI's 127.0.0.0/8 rule. agent-command.ts only recognized 127.0.0.1, but the CLI's is_loopback_url treats the whole 127.0.0.0/8 block as loopback. So a server shown as http://127.0.0.5:8888 got a spurious --api-key in the copied command, which skips the CLI auto-mint path. The bare-command shortcut still fires only for the canonical default host (127.0.0.1/localhost/::1 on :8888), so a non-default loopback like 127.0.0.5 correctly emits UNSLOTH_STUDIO_URL instead of collapsing to the bare command.

2. unsloth_cli/commands/connect.py is back as a deprecated shim. The hidden Typer alias kept unsloth connect working on the CLI, but deleting the module broke direct Python imports of unsloth_cli.commands.connect / connect_app. The shim re-exports start, with a small test asserting connect_app is start_app.

3. Made the touched frontend files biome-clean by wrapping the long codingAgentsHint string and ordering the buildAgentCommand import.

Verified locally: unsloth_cli/tests/test_start.py 56 passed / 3 skipped, test_stream_cancel_registration_timing.py 25 passed, frontend typecheck and production build pass, and a command-builder matrix (default-local, non-default port, tunnel, remote, 127/8, ipv6, unix and windows) all match the expected output.

One item left before merge: this branch is about 20 commits behind main, so merging as-is would revert unrelated current-main work (the release-desktop workflow hardening, the install.sh cleanup, and the /p preview router in studio/backend/main.py). Please rebase onto origin/main, resolving the en.ts and stream-cancel test conflicts in favor of main, before we merge.

danielhanchen and others added 2 commits June 25, 2026 08:22
# Conflicts:
#	tests/studio/test_stream_cancel_registration_timing.py
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 87a9f64bb9

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


// The bare `unsloth start` only auto-discovers 127.0.0.1:8888 (localhost/::1 alias it).
function isDefaultLocalHost(host: string): boolean {
return host === "127.0.0.1" || host === "localhost" || host === "::1";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Strip IPv6 brackets before loopback checks

URL.hostname is bracketed for IPv6 literals (for http://[::1]:9000 it is [::1]), so this check never treats Studio's IPv6 loopback URL as loopback. In that local IPv6/non-default-port case the command builder falls through and appends --api-key sk-unsloth-YOUR_KEY once the reveal card is dismissed, even though the CLI can auto-mint for ::1; copying the shown command then fails instead of connecting. Normalize [::1] to ::1 before calling these predicates.

Useful? React with 👍 / 👎.

@danielhanchen

Copy link
Copy Markdown
Member

Decided to drop unsloth connect entirely instead of keeping it as a hidden alias. unsloth start is the only command going forward.

Removed in 6e543d7:

  • the hidden connect Typer alias in unsloth_cli/__init__.py
  • the unsloth_cli/commands/connect.py re-export shim
  • the matching shim test

After this, unsloth connect returns "No such command" and unsloth start lists the five agents (claude, codex, openclaw, opencode, hermes). unsloth_cli/tests/test_start.py passes (55 passed, 3 skipped). The branch is rebased on the pre-commit.ci autofix and up to date with main, so it stays conflict-free.

@danielhanchen

Copy link
Copy Markdown
Member

Reverting my last call: keeping unsloth connect for backwards compatibility after all. It is registered as a hidden alias that routes to the same app as unsloth start, so old unsloth connect <agent> usage keeps working while unsloth start is the canonical command.

In 89b2cdc: re-added the hidden connect alias in unsloth_cli/__init__.py (same start_app object, hidden = True). I did not bring back the unsloth_cli/commands/connect.py Python module shim, since this is only about the CLI command routing.

Verified: unsloth start and unsloth connect both resolve to the five agents (claude, codex, openclaw, opencode, hermes), top-level --help shows start and hides connect, and test_start.py passes (55 passed, 3 skipped).

@danielhanchen

Copy link
Copy Markdown
Member

What unsloth start does

unsloth start <agent> points a local coding agent at a running Unsloth Studio server and launches it, so the agent talks to the model Studio is serving (via the local llama-server / OpenAI-compatible API) instead of a hosted provider.

Agents it can launch

Command Agent Notes
unsloth start claude Claude Code sets ANTHROPIC_BASE_URL / ANTHROPIC_AUTH_TOKEN / ANTHROPIC_MODEL
unsloth start codex OpenAI Codex writes a ~/.codex profile; needs a GGUF model on llama-server
unsloth start openclaw OpenClaw writes ~/.openclaw/openclaw.json
unsloth start opencode OpenCode writes the opencode config
unsloth start hermes Hermes (Nous Research) writes ~/.hermes/config.yaml

What it does under the hood

  1. Discovers the Studio server from UNSLOTH_STUDIO_URL (default http://127.0.0.1:8888). If none is running, it tells you to start one.
  2. Gets an API key: for a local loopback server it mints one automatically and remembers it per server; for a remote server you pass --api-key (or UNSLOTH_API_KEY).
  3. Resolves the model: --model loads that model on the server, otherwise it uses whatever Studio currently has loaded.
  4. Writes that agent's config/env and launches the agent's binary, forwarding any extra args straight through.

Shared flags

  • --model, -m: model to use; defaults to the one loaded in Studio.
  • --api-key: Studio API key (auto-minted locally, required for remote). Also reads UNSLOTH_API_KEY.
  • --launch / --no-launch: --no-launch prints the env and command instead of running it (handy for remote shells or WSL).
  • Trailing args pass straight to the agent, e.g. unsloth start claude -p "...".

Workflow and examples

# 1) Start a Studio server serving a model (one terminal)
unsloth run -hf unsloth/gemma-3-270m-it-GGUF
# or open the Studio app and load a model from the dropdown

# 2) Launch a coding agent against it (another terminal)
unsloth start claude          # Claude Code on the loaded model
unsloth start codex           # OpenAI Codex (GGUF model required)
unsloth start openclaw
unsloth start opencode
unsloth start hermes

# Use a specific model (loads it on the server if needed)
unsloth start claude --model unsloth/Qwen3-4B-GGUF

# Print env + command instead of launching (remote shells / WSL)
unsloth start claude --no-launch

# Point at a remote or tunneled Studio
UNSLOTH_STUDIO_URL=https://abc.trycloudflare.com unsloth start claude --api-key sk-unsloth-...

# Pass extra flags straight to the agent
unsloth start claude -p "summarize this repo"

# Backwards compatible: the old name still routes to the same thing
unsloth connect claude

The Studio API-keys panel (Settings -> API) also shows a ready-to-copy unsloth start command targeting whatever server it is displaying.

@danielhanchen

Copy link
Copy Markdown
Member

Made unsloth start <agent> carry the model-load flags from unsloth run, so you can control how a model loads when --model triggers a load (commit fbfb400):

  • --gguf-variant
  • --max-seq-length / --context-length
  • --load-in-4bit / --no-load-in-4bit
  • --tensor-parallel / --no-tensor-parallel

They are forwarded to /api/inference/load. A bare --model with no extra flags sends the same payload as before, so existing behavior is unchanged.

unsloth start claude --model unsloth/Qwen3-4B-GGUF --gguf-variant UD-Q4_K_XL --context-length 8192

Intentionally not included: the server-startup flags (--host, --port, --frontend, --api-only, --cloudflare, --secure, --parallel, --enable-tools / --disable-tools, --api-key-name). unsloth start attaches to an already-running Studio server, so those only make sense on unsloth run, which boots it. The free-form llama-server passthrough (unsloth run's trailing args) is also not forwarded, because trailing args on unsloth start go to the coding agent.

All five agents and the unsloth connect alias get these. test_start.py passes (56 passed, 3 skipped), including a new test asserting the forwarded load payload.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cded5f9aed

ℹ️ 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".


// Non-default server: set the URL; non-loopback also needs an explicit key.
let cmd = bare;
if (!loopback && key) cmd += ` --api-key ${key}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid passing placeholder keys to remote agents

When the panel is using a non-loopback base (Cloudflare tunnel or LAN URL) after the reveal card is dismissed, UsageExamples passes sk-unsloth-YOUR_KEY as key, and this line turns that placeholder into an explicit --api-key. Running the copied command then saves and sends the placeholder, so unsloth start fails against /v1/models instead of either auto-minting locally or prompting for a real key; only append --api-key when a real revealed key is available, or show a template that clearly must be edited.

Useful? React with 👍 / 👎.

) -> dict:
models = _loaded_models(base, key)
match = next((m for m in models if m["id"] == requested), None)
if requested and match is None:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor load options for already loaded models

When the requested model id is already present in /v1/models, this condition skips /api/inference/load entirely, so the newly added load knobs are silently ignored. For example, if unsloth/foo-GGUF is loaded with one GGUF variant or context length, unsloth start claude --model unsloth/foo-GGUF --gguf-variant Q8_0 --context-length 8192 still connects to the old loaded instance; the backend's load path already compares these settings and would reload when they differ, but it is never reached here.

Useful? React with 👍 / 👎.

…daction

Three review findings:

1. CI could leak a live key. agent-guides-drive.sh printed the raw
   'unsloth start --no-launch' transcript (which carries export UNSLOTH_API_KEY /
   ANTHROPIC_AUTH_TOKEN lines) to the Actions log on both the failure path and the
   success path before redact() ran. Add cat_redacted() and use it for those two
   prints, so the key is scrubbed on the way to the log while the on-disk file stays
   intact for the env parsing that follows.

2. Outages masqueraded as bad keys. _key_accepted caught a broad Exception and
   returned False, so a 5xx or timeout while checking a cached key looked like a
   rejection: it discarded a good key and minted extra ones (local) or reported 'no
   saved key' (remote). Only treat HTTP 401/403 as a rejection; let other errors
   propagate so a real outage surfaces.

3. Codex preflight could leave the auto-started server up. _require_gguf_for_codex
   runs after _connect may have auto-started Studio but before _run installs its
   teardown finally, so a preflight rejection (e.g. a transformers-backend model) left
   the server holding the port/GPU until the atexit backstop. Tear it down explicitly
   at the point of failure.

Tests: a 5xx on a saved key surfaces without minting; a non-GGUF codex preflight
tears down the auto-served server.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc2734e371

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const host = normalizeHost(url.hostname);
const loopback = isLoopbackHost(host);
// Default local server (127.0.0.1/localhost:8888): bare command auto-discovers it.
if (isDefaultLocalHost(host) && url.port === DEFAULT_STUDIO_PORT) return bare;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve UNSLOTH_STUDIO_URL for IPv6 loopback

When the API panel's base is http://[::1]:8888 (for example, Studio bound to IPv6 loopback), this branch returns the bare unsloth start claude command even though the CLI default probes http://127.0.0.1:8888. Users copying the command from that local IPv6 server will fail to find Studio, or could attach to a different server on IPv4; keep emitting UNSLOTH_STUDIO_URL for ::1 instead of treating it as the bare default.

Useful? React with 👍 / 👎.

Comment thread unsloth_cli/commands/start.py Outdated
# home-scope the session to leave the user's real ~/.pi untouched. The key
# rides in the config rather than the env.
write_pi_config(base, key, entry, home / ".pi" / "agent" / "models.json")
env = {"HOME": str(home)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Override Pi's config-dir env for session config

When the user's shell already has PI_CODING_AGENT_DIR set, Pi ignores the HOME relocation here: upstream Pi's getAgentDir() reads process.env[ENV_AGENT_DIR] (for Pi, PI_CODING_AGENT_DIR) before falling back to homedir(). Because _launch preserves that inherited variable, the child can skip the Unsloth-written models.json and start with the user's config instead, so the unsloth provider/key may be missing; set or clear PI_CODING_AGENT_DIR for this session as well.

Useful? React with 👍 / 👎.

# Auto-start a local server only for an interactive launch with a model to serve, and
# only for a loopback target (never stand in for an explicit remote UNSLOTH_STUDIO_URL).
if serve and launch and model and is_loopback_url(expected):
return expected, _start_studio_server(expected, model, load or LoadOptions())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Normalize the auto-start URL before waiting on it

If UNSLOTH_STUDIO_URL is a loopback URL with no explicit port, e.g. http://127.0.0.1, this passes that unnormalized base into _start_studio_server; the child is launched on parsed.port or 8888, but readiness still polls the original port-80 URL via _studio_healthy(base), so unsloth start ... --model ... waits until the 15-minute timeout even though the server came up on 8888. Normalize expected to the actual host:port before starting/polling.

Useful? React with 👍 / 👎.

Comment thread unsloth_cli/commands/start.py Outdated
*_yolo_command_flags("pi", yolo),
*ctx.args,
]
install_hint = "npm install -g @earendil-works/pi-coding-agent"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add Pi's documented --ignore-scripts install flag

When Pi is missing and the user accepts the auto-install prompt, _install_agent executes this hint as-is. Pi's current install docs and README document the npm path with --ignore-scripts as part of their supply-chain hardening, but this command omits it, so accepting the prompt runs package lifecycle scripts during a global install instead of following the upstream safe install recipe.

Useful? React with 👍 / 👎.

… install recipe

Four review findings:

1. Pi ignored the session config when PI_CODING_AGENT_DIR was already set. Pi's
   getAgentDir() reads process.env.PI_CODING_AGENT_DIR before falling back to
   $HOME/.pi/agent, so a value inherited from the user's shell sent Pi to their real
   config and skipped our provider/key (the HOME relocation alone was not enough). Pin
   PI_CODING_AGENT_DIR at the session's .pi/agent dir; it is path-valued so the WSL
   bridge translates it automatically.

2. Pi install hint dropped Pi's documented --ignore-scripts. Pi's README installs with
   'npm install -g --ignore-scripts @earendil-works/pi-coding-agent' and notes it needs
   no install scripts, so accepting the prompt now follows that safe recipe.

3. Auto-start ignored a portless UNSLOTH_STUDIO_URL. unsloth run binds to
   'parsed.port or 8888', so http://127.0.0.1 launched the child on 8888 but the health
   poll (and the returned base) still used port 80, stalling until the startup timeout.
   Normalize the base to host:8888 (IPv6-safe) before starting and polling.

4. API-panel command mistook IPv6 loopback for the bare default. The bare 'unsloth
   start' only probes 127.0.0.1:8888 on the IPv4 stack, so http://[::1]:8888 must carry
   an explicit UNSLOTH_STUDIO_URL. Drop ::1 from the bare-default host set while keeping
   it a loopback host (URL emitted, no key needed).

Tests: PI_CODING_AGENT_DIR is set to the session dir; _effective_base normalizes
portless/IPv6 bases; a portless UNSLOTH_STUDIO_URL auto-serves on :8888.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d165401d92

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const host = normalizeHost(url.hostname);
const loopback = isLoopbackHost(host);
// Default local server (127.0.0.1/localhost:8888): bare command auto-discovers it.
if (isDefaultLocalHost(host) && url.port === DEFAULT_STUDIO_PORT) return bare;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve localhost in the copied agent command

When Studio reports http://localhost:8888 (for example from --host localhost, especially on systems where localhost resolves to IPv6), this branch returns the bare unsloth start claude. The CLI's bare default only probes http://127.0.0.1:8888 unless UNSLOTH_STUDIO_URL is set, so copying the shown command can miss the server displayed in the UI or attach to a different IPv4 process. Only elide the URL for the literal 127.0.0.1:8888 default, or keep UNSLOTH_STUDIO_URL for localhost too.

Useful? React with 👍 / 👎.

Comment on lines +367 to +368
expected = _effective_base(expected)
return expected, _start_studio_server(expected, model, load or LoadOptions())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject HTTPS loopback auto-serve targets

When UNSLOTH_STUDIO_URL is an HTTPS loopback URL and no Studio is running, this branch still auto-starts unsloth run without any TLS mode but keeps expected as https://.... The readiness loop then probes the plain HTTP child over HTTPS, so unsloth start ... --model ... waits the full startup timeout before tearing it down. Restrict auto-serve to HTTP loopback URLs or normalize the returned base to the actual HTTP endpoint.

Useful? React with 👍 / 👎.

danielhanchen and others added 2 commits July 2, 2026 04:47
…command

From a fresh multi-reviewer pass over the merged head plus the latest Codex bot review:

1. Load knobs now always consult the server. _resolve_model matched on model id alone,
   so --gguf-variant / --context-length / --no-load-in-4bit / --tensor-parallel were
   silently ignored whenever the id was already loaded (asking for UD-Q4_K_XL kept a
   Q8_0 serving). With any explicit knob the CLI defers to /api/inference/load, whose
   already-loaded dedup answers without reloading when variant and settings match, so a
   second session running the same command still attaches without evicting the first.

2. OpenCode --yolo and the session model pin now ride in OPENCODE_CONFIG_CONTENT. A
   project's own opencode.json outranks OPENCODE_CONFIG, so a repo config could silently
   override the session model and the --yolo permission block; OPENCODE_CONFIG_CONTENT
   outranks project config. The API key stays in the private file, never in printed env.

3. The --no-launch recipe's last line is a self-contained one-liner (inline VAR=value
   assignments before the command, conflicting vars blanked). People copy just the last
   line, and a bare codex/claude there ran against the user's real ~/.codex or Anthropic
   credentials with zero isolation, e.g. inheriting a pre-existing damaged ~/.codex
   state DB and blaming the recipe. The CI drive script scrubs the key from the one
   'invoking:' echo this adds.

4. The auto-serve log is 0600 and the parent handle is closed. It sat world-readable in
   the shared tempdir under a predictable name while carrying the minted sk-unsloth-
   key from the unsloth run banner.

5. _key_accepted fails with a clean message on outages. Non-auth errors (5xx, network,
   timeout) surfaced as a raw traceback; 401/403 still mean a rejected key.

6. _effective_base strips URL paths, and https loopback targets never auto-serve.
   http://127.0.0.1:8888/studio polled /studio/api/health (404) and https://127.0.0.1
   polled the wrong scheme, both spinning until the 15-minute startup timeout.

7. API-panel command: only literal 127.0.0.1:8888 earns the bare command. localhost can
   resolve to ::1, which the bare CLI never probes, so it keeps UNSLOTH_STUDIO_URL.

8. CI artifact sweep covers redacted-configs/ and agent-workdir/, not just logs/.

Tests: 125 CLI tests pass (new coverage for each fix), 156 backend tests pass, ruff
clean. Adds an unsloth connect alias regression test.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8629227120

ℹ️ 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".

# pin (and --yolo permissions) would silently lose to a repo config. Carry the
# settings that must win in OPENCODE_CONFIG_CONTENT, which outranks project
# config; the API key stays in the private file, never in the printed env.
inline_config: dict = {"model": f"unsloth/{entry['id']}"}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep the OpenCode provider in the winning config layer

Fresh evidence: this fix moves only model (and yolo permissions) into OPENCODE_CONFIG_CONTENT; the provider.unsloth endpoint/key still lives only in the lower-precedence OPENCODE_CONFIG file written above. OpenCode's config docs say OPENCODE_CONFIG is loaded before project opencode.json, while OPENCODE_CONFIG_CONTENT is the later runtime override, so in any repo that already defines provider.unsloth the project provider can replace this Studio base/key and model: unsloth/... will still target the project's provider instead of the current Studio session. Use a collision-proof provider id or put the provider override in the same higher-precedence layer.

Useful? React with 👍 / 👎.

Comment on lines +803 to +804
typer.echo(" ".join(_powershell_quote(arg) for arg in command))
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Make Windows recipes' final command self-contained

On native Windows --no-launch, this branch prints the $env: setup lines and then echoes only the bare agent command before returning, so copying just the last line skips the relocated config/key env (CODEX_HOME, OPENCODE_CONFIG, ANTHROPIC_AUTH_TOKEN, etc.). With the new session-scoped config dirs, that partial copy runs against the user's real agent config or credentials instead of the Studio session; emit a PowerShell-scoped one-liner like the POSIX path or otherwise make the final command include the env.

Useful? React with 👍 / 👎.

danielhanchen and others added 2 commits July 2, 2026 12:32
Pi paints inline from wherever the cursor sits: its first render assumes a
clean screen instead of clearing or entering the alternate screen itself
(current Pi never emits a clear at startup). Launched under unsloth start,
that left the session starting mid-scroll beneath the connection output.
Clear the screen (click.clear, cross-platform, no-op without a TTY) right
before the Studio banner so Pi opens exactly one line down on a clean
viewport. Launch path only: --no-launch recipes and piped output are never
wiped, and alternate-screen agents are left alone.
@danielhanchen

Copy link
Copy Markdown
Member

Pi now opens on a clean screen (260da17)

Feedback from testing: unsloth start pi left Pi starting inline mid-scroll, beneath the connection output, instead of taking over a fresh screen.

Root cause: current Pi never clears at startup. Its first TUI paint explicitly assumes a clean screen (no clear sequence, no alternate screen; verified against Pi 0.79.10 and 0.80.3 by capturing raw pty output and Pi's own redraw debug log, which shows only "first render" with no clear). Bare pi looks clean because a shell prompt usually sits on a fresh line; under unsloth start the connect output pushed Pi's inline paint mid-screen.

Fix: the launcher now hands Pi the clean screen its first render assumes. Right before the Studio <base> · model <id> banner, the Pi launch path clears the terminal via click.clear, which is cross-platform (Windows/Mac/Linux) and a no-op when stdout is not a TTY, so piped output, CI transcripts, and --no-launch recipes are never wiped. Alternate-screen agents (claude, codex) manage the terminal themselves and are untouched.

Verified end to end in tmux against a live Studio: scrollback markers gone from the viewport, banner on line 1, Pi's UI directly below, and a real tool-call turn completed in the session. 128 CLI tests pass including three new ones (Pi launch clears exactly once before exec, --no-launch never clears, claude launch never clears).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 987cf40922

ℹ️ 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".

Comment thread .github/scripts/agent-guides-install.sh Outdated
Comment on lines 94 to 97
# No start.py recipe; the agent's documented package name. The CLI moved
# from the now-deprecated @mariozechner scope to @earendil-works (the old
# scope is frozen, so installing it would test a stale Pi against the API).
npm_retry "@earendil-works/pi-coding-agent" \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Mirror Pi’s start install hint in CI

Now that unsloth start pi has a real install hint (npm install -g --ignore-scripts @earendil-works/pi-coding-agent in start.py), this branch is no longer a hand-written exception. The CI installer still calls npm_retry without that flag; I checked npm install --help, which lists [--ignore-scripts] for install, so this cell both runs package lifecycle scripts and stops exercising the exact recipe users are prompted to run. Pass the flag through for Pi so guide drift is caught.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in f7f4cff: npm_retry passes flags through and the pi cell installs with --ignore-scripts, exactly matching the start.py hint; the stale no-recipe comment is refreshed too. Validated on a staging clone where the connection (pi) cell passed with the flagged install.

Hermes refuses to initialize when the served model's context window is
under 64,000 tokens, and a second copy of the same check rejects the
compression model mid-session. write_hermes_config previously pinned the
real window, so any small local model (e.g. 40,960) failed at startup
with manual config.yaml instructions.

For windows below the floor the recipe now claims 65,536 in
model.context_length, scales compression.threshold so compaction still
fires at 90% of the real window, and sets
auxiliary.compression.context_length to cover the mid-session check.
Windows at or above the floor keep the exact previous behavior.
@danielhanchen

Copy link
Copy Markdown
Member

Pushed 5f158fc: unsloth start hermes now handles Hermes' 64K context floor itself.

Hermes refuses to initialize when the served model reports a context window under 64,000 tokens, and the same check runs a second time against the compression model mid-session. The recipe previously pinned the real window, so any smaller local model (for example Qwen3-1.7B at 40,960) failed at startup and pointed the user at manual config.yaml edits.

For windows below the floor, write_hermes_config now:

  • claims 65,536 in model.context_length (the override Hermes' own error message suggests)
  • scales compression.threshold so compaction still fires at 90% of the real window (0.5625 for a 40,960 model)
  • sets auxiliary.compression.context_length to cover the mid-session compression-model check

Windows at or above the floor keep the exact previous behavior, and the config stays idempotent.

Verified end to end against a llama-server serving Qwen3-1.7B at 40,960 context: a fresh session now initializes with no floor error and completes interactive file tool turns; before the change the same setup reproduced the hard init refusal. CLI test suite passes (129), and the Local Agent Guides connection matrix stays green, so its model-choice comment no longer needs to cite the Hermes floor.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5f158fccf5

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +604 to +608
loaded = _http_json(
"POST",
f"{base}/api/inference/load",
key,
payload,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Force non-GGUF reloads when load settings change

Fresh evidence: this POST is still not enough for an already-active transformers/non-GGUF model, because _load_model_impl returns already_loaded for that path based only on backend.active_model_name (studio/backend/routes/inference.py:3051-3054) before comparing max_seq_length or load_in_4bit. In that context, unsloth start ... --model same --context-length 8192 can print that it is ensuring the requested settings but still attach the agent to the old context/quantization; force a reload/unload for non-GGUF load overrides or make the backend guard compare these forwarded options.

Useful? React with 👍 / 👎.

The pi cell predates the pi recipe in start.py and still installed the
package with lifecycle scripts enabled, so CI stopped exercising the
exact command users are prompted to run. npm_retry now passes extra
flags through, the pi branch mirrors the install hint verbatim, and the
stale no-recipe comment is refreshed.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7f4cff30a

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const host = normalizeHost(url.hostname);
const loopback = isLoopbackHost(host);
// Default local server (127.0.0.1/localhost:8888): bare command auto-discovers it.
if (isDefaultLocalHost(host) && url.port === DEFAULT_STUDIO_PORT) return bare;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep HTTPS loopback URLs explicit

When the panel base is https://127.0.0.1:8888, this condition still returns the bare unsloth start ... command because it checks only host and port. The CLI's bare default probes http://127.0.0.1:8888, so copying the recipe from a local HTTPS endpoint can either miss the displayed server or attach to a different plain-HTTP process; only elide UNSLOTH_STUDIO_URL for the literal http://127.0.0.1:8888 default.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6e653f8: the bare-command elision now also requires the http: scheme, so an HTTPS loopback on the default port keeps its explicit UNSLOTH_STUDIO_URL.

Comment thread unsloth_cli/commands/start.py Outdated
shutil.rmtree(path, ignore_errors = True)
else:
path = _agents_config_root() / agent
shutil.rmtree(path, ignore_errors = True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid deleting active no-launch homes

When --no-launch is used, the printed recipe points the agent at this stable per-agent directory (CODEX_HOME, HERMES_HOME, OPENCLAW_STATE_DIR, etc.). If the user runs unsloth start <same-agent> --no-launch again while a previously printed command is still running, this unconditional removal deletes that live session's config/state home, which can make agents that keep sessions or SQLite/state under that directory fail or lose state mid-run; use a per-recipe directory or avoid deleting a home that may be active.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6e653f8: the stable no-launch home is no longer wiped on re-run; it is created if missing and reused, since every config writer already merges idempotently into an existing home and a previously printed recipe may still be running an agent whose sessions or sqlite state live there. Test pins that a marker file survives a second run.

Comment thread unsloth_cli/commands/start.py Outdated
load = load._replace(gguf_variant = variant)
base, server = _require_studio(model, load, serve = serve, launch = launch)
try:
key = _agent_api_key(base, api_key)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Ignore stale env keys for auto-started servers

When no Studio is running and the user has UNSLOTH_API_KEY exported for some other server, _require_studio can auto-start a fresh local server for --model, but this line still forces that stale key into the first /v1/models request. The new local server can mint/use its own local key, yet the agent launch fails with auth errors solely because an unrelated key was present in the environment; for server is not None, validate the explicit key before using it or fall back to the local mint/cache path.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6e653f8: for a server this run auto-started, an explicit or exported key is validated first and, if the fresh server rejects it, the launch falls back to the local mint/cache path instead of failing (and the foreign key is never remembered for that base). A key the fresh server accepts, e.g. one persisted in the Studio home's auth db, is honored exactly as before. Both cases are covered by new tests.

The empty-string guards ran after appending /config.toml or /config.yaml,
so they could never fire: crosscheck_contract silently skipped its
contract checks and patch_hermes_tools died on the root path with a bare
traceback. Check the raw variable first and guide_fail with the real
cause.
@danielhanchen

Copy link
Copy Markdown
Member

Pushed bc7498b: hardened the guide-drive script's relocation-var handling. The empty-string guards ran after appending /config.toml or /config.yaml, so a missing CODEX_HOME or HERMES_HOME in the connect output could never trip them; crosscheck_contract silently skipped its contract checks and patch_hermes_tools died on the root path with a bare traceback. The raw variable is now checked first and fails with the real cause. Validated on a staging clone: all six connection cells green, Windows and macOS parity green.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@wasimysaid
wasimysaid merged commit b8400f4 into unslothai:main Jul 3, 2026
43 of 45 checks passed
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants