Conversation
Model downloads now report live progress and can be canceled, for both Ollama and LM Studio, on local and remote nodes. The row holds "Canceling" until the backend confirms the transfer has stopped and its files are settled. Cancellation removes only the partial files that download created. Both engines funnel every client on a machine into one cache, so naming a file is not the same as owning it: each pull records the partial files already present before it starts and considers only those that appear afterwards, and a file still growing once the transfer stops is treated as shared and preserved. Completed models, other quantizations, and unrelated downloads are never touched, and a cancellation the LM Studio CLI never confirmed deletes nothing. Only a cancellation someone requested removes files; a pull interrupted by shutdown, a dropped remote connection, or the action timeout leaves its partial data resumable. Downloads are serialized per engine so cleanup cannot race another download, and a queued download can be canceled or retried before it starts. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
… not own cleanupLMSPartials treated a partial as this pull's whenever it differed from the pre-download snapshot. Bytes gained during the pull are not evidence of ownership: another client writing into the same repository directory produces exactly the same difference, and the model hub pulls unpinned ids, so quant is empty and the quantization filter excludes nothing. Every partial in the repository was reachable. Match Ollama's rule instead - presence in the snapshot disqualifies a path outright - and treat a nil snapshot as a failed inspection that deletes nothing rather than as an empty directory. A cancelled resume now keeps its own partial, since nothing on disk tells a resume target from another client's file. That costs disk the next attempt reuses, where the alternative costs a live download its bytes. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
A peer answering cancel_pull interrupts the CLI, waits for it to acknowledge, and removes partial files before it writes a header. The ordinary 30s response-header budget can expire while that is still in progress, so the initiator saw a transport timeout from a cancel the peer was carrying out. Route controlCancelPullPath through the readyHTTP pool, alongside the other endpoints whose peer withholds its header while doing slow work in its own handler. Being cut off never undid the cancellation - that is latched before the peer starts waiting - so the damage was confined to reporting: a cancel that was succeeding came back as failed, and the row rolled back to "Downloading" under a transfer that was stopping. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
…nded The peer joins a duplicate pull onto the download already in flight, so two requests can share one attempt. Their release closed attempt.accepted on the first one to end, which freed a waiting cancel while the other was still on its way to the peer. The cancel then arrived at a peer with nothing registered, was answered as a cancel for nothing, and left the transfer running under a row stuck on "Canceling". Close accepted only once refs reaches zero. Acceptance keeps its own closer, so the peer taking the pull still frees the cancel immediately, however many requests are outstanding. Size remotePullGateWindow to the pull's pre-header budget while here. The old 10s could elapse with the pull legitimately still in flight, producing the same cancel-for-nothing. The window is a backstop, not the mechanism: dial, handshake and header chain only on success, so a pull that never lands releases the gate itself well inside the new value. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
…partial A partial this pull created can still be shared, because another client can coalesce onto it afterwards. Cleanup left a file alone while it was moving, but one quiescence check per pass was not enough once the retry loop ran several of them: each pass observes its own window, so every extra attempt was another independent chance for a writer that merely paused to look finished. A transfer does pause, which made the loop - there to catch up with Ollama's asynchronous blob release - likeliest to delete a shared file exactly when it tried hardest. Carry a per-path count of consecutive still passes across one cancellation and delete only at partialCleanupSteadyPasses. Any movement, or the path dropping out of the candidate set, resets the run, so a file now has to hold still across the whole run to qualify. This pull's own released partial pays one extra pass for it. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
Two defects in the same path, both about the evidence a cancellation acts on. The download output was shared without synchronisation. The exec copy goroutine writes lmsDownloadOutput while the goroutine handling the cancellation reads it to decide whether any partial file may be deleted, and a process the worker gives up on keeps its writer alive past the call that started it - so the reader in pullLMSModel and stopLMSDownload writing cancelled back to false genuinely overlap. Guard every mutable field with a mutex and read them through state(), which returns one consistent snapshot so a caller cannot pair a stale completed with a fresh cancelled. The prompt answer and the progress callback run outside the lock, since each reaches outside the writer and the stdin write can block on the child. Clearing cancelled did not hold anyway. The process is still running, so its next redraw could set the flag again and re-arm deletion of files nothing confirmed had stopped. disown() withdraws the confirmation for good, and the abandonment path now branches on the reap result it discarded, so a process that outlasts the kill is reported as its own outcome. Separately, both engines folded a cleanup failure into the pull's result, which reported a failed cancellation for a transfer that had genuinely stopped and failed the pull's own request with it. Leftover bytes are the vendor's to resume, so this denied the one outcome that did happen and left the row on "Canceling" over a download that was already gone. Warn and keep the cancellation. The race needs -race to demonstrate, which cannot run here (CGO_ENABLED=0, no C compiler), so the concurrency test asserts nothing by itself and is there for CI. The disown and cleanup-failure behaviours are asserted directly. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
The model is what scopes a pull: it keys the claim a cancel is matched against, and it is streamOp's progress filter. Params that named none satisfied the old check purely by being non-empty, then left req.Model empty - which made claimPull a no-op and emptied the filter. An empty filter disables filtering, so the initiator received every other model's pull progress on that engine stamped with its own opID, the one thing the comment above the guard says must not happen. Resolve the model from params first, then require one. That also retires the params fallback in the failure path, which can no longer be reached. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
Neither cancellation signal addresses the CLI alone. On Unix it goes to the whole download process group; on Windows it becomes a console control event for everything sharing the launcher's console. So a signal sent after the process is reaped does not merely miss - the OS is free to have reissued that PID, and the signal reaches a stranger. The Windows guard did not close the window it was written for. isDownloadLauncher verified the image and closed its process handle on return, and only afterwards did the helper call AttachConsole(pid). Holding a handle is what keeps Windows from reusing a PID, so the check proved only what had been true a moment earlier. Return the handle from openDownloadLauncher instead and close it once the attach has happened. Unix had no guard at all. Both signal sites in stopLMSDownload now go through helpers that decline once the process is reaped, which covers Windows too. The flag is an atomic set before Wait's result is published: cmd.ProcessState cannot serve, because Wait writes it on one goroutine while the cancelling goroutine would read it - trading this race for another. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
The list was wrong in both directions. A cancel that hit callTimeout was read as a finished cancel. The engine-manager acknowledges one only after the transfer has stopped and its partial files are cleaned up, so the reply can be slower than the call, and retiring the entry there dropped the only cancel target for a download that was still running - a second press reported no active download on the engine while the transfer carried on. Treat the deadline as the call giving up rather than an outcome, the way the pull path already does, and leave the entry cancellable. Nothing then retired an entry whose own request had outlived that same ignored deadline, so a finished download stayed selectable for the rest of the session. Retire on the terminal progress frame, which the engine-manager emits on failure precisely so a client that stopped waiting still converges. The two halves interlock: ignoring the deadline keeps a download cancellable while it runs, and the terminal frame is what ends it. A successful pull that outlasts callTimeout on an engine whose stream emits no terminal frame still leaks its entry. The residue is a stale cancel target, which the engine-manager already answers harmlessly, and closing it means emitting a terminal success frame - a backend contract change rather than a TUI one. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
Cancelling a download was one-shot. The first attempt latched an entry in `cancelingPulls`, and `setModelPullCanceling` refused any later cancel whose key was already there, so once a request stopped being awaited the row sat on "Canceling" with nothing behind it. That entry is reachable. `MODULAR_CANCEL_PULL_TIMEOUT_MS` is 90s, but a peer answering `engine:remote-cancel-pull` is served by the readiness pool, whose header budget is eleven minutes — deliberately, because cutting a peer off mid-cancel is worse than waiting. A peer that takes longer than 90s to stop the transfer and settle its partial files is therefore ordinary, not exceptional. The desktop stops awaiting, keeps the row in "Canceling" (correct: the cancel really is still running), and the only other thing that clears the row is the pull's own RPC settling, which is bounded at six hours. Until then the download keeps running and the user has no way to ask again. The fix separates the two jobs that one map was doing. `cancelingPulls` keeps only the job it is named for: remembering the status the download had before the first cancel, so a rejection can restore it. It now records that on the first cancel alone. Recording it again on a retry would capture "canceling" as the status to restore, and a later rejection would put the row back into the very state it was being told the backend refused to enter. The guard against concurrent cancels moves to the supervisor, which is the layer that knows when a request is actually outstanding. It is held across the await and released in a `finally`, so a mashed button still sends one request while a timed-out one leaves nothing latched behind. Both cancel buttons drop `canceling` from `disabled`. Without that the retry is unreachable and the rest of this change is invisible: the row is the user's only handle on the download. The label still reads "Canceling…" because that remains true, and repeat clicks are absorbed by the guard above rather than by greying out the control. The comment on `MODULAR_CANCEL_PULL_TIMEOUT_MS` claimed the 90s sits above the peer's header budget. That stopped being true when the remote cancel moved onto the readiness pool earlier in this branch; it now describes why the desktop deliberately gives up first and relies on retry instead of matching a budget measured in minutes. Each part was verified by reverting it alone: restoring the old refusal fails the re-issue case, making the status memory unconditional restores a rejected row to "canceling", and moving the release out of `finally` leaks the guard into every later cancel. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
The hand-maintained documents the architecture rules require updating never gained the cancellation surface this branch adds. `frontend-api.md` listed every other model command but not `cancelModelPull`, and neither it, `services-backend.md`, nor `architecture.md` mentioned `engine:cancel-pull`, `engine:remote-cancel-pull`, or the `model` field they target. `service-contracts:check` cannot catch this: it validates the generated `services-api.md` alone, which already had the methods. `model` is worth stating rather than implying. It is what separates these from the engine-wide commands, and it is why an engine can have several downloads running and cancel, or track progress on, any one of them. Each document gets the part it is responsible for. `frontend-api.md` adds the command and says what a caller sees: no state returned, the row moves to Canceling, the pull's own settling clears it. `services-backend.md` adds the missing `engine:pull-progress` row to the notification table and explains why the reply is only an acknowledgement. `architecture.md` describes cancellation as a request rather than a result. All three record the budget asymmetry, because it looks like a bug otherwise: the peer is deliberately given far longer to answer than the desktop waits, so the desktop giving up first means the cancel is still running, and the row stays in Canceling and remains retryable. Prettier is not run over these files. Both `services-backend.md` and `architecture.md` already fail `prettier --check` at HEAD, and formatting them would rewrite two unrelated tables and an emphasis marker. The added table row is aligned to the existing column widths by hand. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
The cancellation path this branch adds had no cross-process coverage. Its unit tests stub the peer, so nothing exercised a cancel travelling from one engine-manager to another over the pin-gated ec mTLS surface the download itself uses. The case chosen is a cancel for a download nobody started, because it is where the new ordering gate is easiest to get wrong. The gate holds a cancel until the pull it names has reached the peer; with no pull registered there is nothing to wait for, so it has to let the cancel through rather than park it for its full window. The assertion is simply that a response arrives well inside that window, and that node B answers a cancel for nothing as a no-op instead of an error. The second case covers the other end: a cancel naming no model has no target, and is refused with an invalid-params code before anything reaches the peer. Asserting the code rather than merely an error is what keeps it honest — dropping the check does not make the request succeed, it makes the peer reject it, which would still leave an error behind. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
This was referenced Sep 23, 2026
kjlubick
approved these changes
Sep 23, 2026
kjlubick
left a comment
kjlubick
left a comment
Collaborator
There was a problem hiding this comment.
LGTM (assuming this is the same code as internal MR 121)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Model downloads now report live progress and can be canceled, for both Ollama and LM Studio, on local and remote nodes. The row holds "Canceling" until the backend confirms the transfer has stopped and its files are settled, rather than dropping back to "Downloading" the moment a cancel request's own budget elapses.
Cancellation removes only the partial files that download created. Both engines funnel every client on a machine into one cache — Ollama blobs are content-addressed, and the LM Studio app downloads into the same repository directory — so naming a file is not the same as owning it. Each pull records the partial files already present before it starts and considers only those that appear afterwards; a file still growing once the transfer has stopped is treated as shared and preserved. Completed models, other quantizations, and unrelated downloads are never touched, and a cancellation the LM Studio CLI never confirmed deletes nothing.
Only a cancellation someone requested removes files. A pull interrupted by application shutdown, a dropped remote connection, or the action timeout leaves its partial data resumable.
Downloads are serialized per engine so cleanup cannot race another download. A request for an engine that is already downloading is reported as queued and starts when its predecessor finishes; a second request for a download already in flight joins it instead of failing. A queued download can be canceled or retried before it starts.
The terminal interface gains the same cancellation the desktop has.
Release intent
Changelog title
Cancel model downloads and see their progress
Changelog body
Bumps
Scope
In scope:
engine:cancel-pullandengine:remote-cancel-pullJSON-RPC methods, and the matchingPOST /v1/models/cancel-pullroute on the cluster remote-control surface.engine:pull-progressandengine:remote-progress, so a progress frame can be keyed to the download it belongs to.Out of scope:
Review fixes
A review pass over this branch found nine correctness defects, each fixed in its own commit with a test that fails without it. Grouped by what could go wrong:
Destructive or incorrect cleanup
Cancellation reported wrongly, or not reaching its target
Concurrency and process safety
AttachConsole, and a reaped process is never signalled.Input validation
The last of these is worth calling out because the fix spans three layers.
MODULAR_CANCEL_PULL_TIMEOUT_MSis 90s, but a peer answering a cancel is served by the readiness pool, whose header budget is far longer — deliberately, since cutting a peer off mid-cancel is worse than waiting. A peer taking longer than 90s to stop a transfer and settle its files is therefore ordinary. The desktop stops awaiting, correctly keeps the row in "Canceling", and the only other thing that clears the row is the pull's own RPC, bounded at six hours. The bridge refused any further cancel for that download and the button was disabled, so the download kept running with nothing the user could do. The in-flight guard now lives in the supervisor and is held only while it is actually awaiting a reply, the status memory is recorded on the first cancel alone so a rejected retry cannot restore the row to "Canceling", and the button stays clickable.Validation
Run on Windows against this branch:
Manual, on real engines:
services/testsnow includes a two-node case that drivesengine:remote-cancel-pullfrom one real engine-manager to another across the pin-gatedecmTLS surface.The Go race detector cannot run on this Windows host, which has CGO disabled and no C compiler, so it was run in a throwaway
golang:1.25container instead:One engine-manager test,
TestUninstallTerminatesRunningInstance, fails inside the container: process-ownership detection reads the engine as externally managed there. It fails identically at the merge base, so it is a container artifact rather than a regression.Not yet run:
Risk
Checklist
git commit -s), certifying the Developer Certificate of Origin.services/versions.jsonis written by automation — do not edit it by hand.