Repository navigation
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>
kjlubick
left a comment
There was a problem hiding this comment.
LGTM (assuming this is the same code as internal MR 121)
An unpinned LM Studio pull let every new partial in the repository reach deletion, so cancelling one could remove a quantization the LM Studio app was fetching beside it once that download stalled for a settle window. Without a pinned quantization, a partial is now a candidate only when the CLI's output named its file; if it named none, nothing is deleted. LM Studio cleanup now shares Ollama's retry loop and consecutive-still passes, and neither engine stops at the first failed removal. A failed pre-download snapshot no longer fails the download: it leaves the snapshot nil, which deletes nothing on cancel, as Ollama already did. A download's PID could be reissued while cmd.Wait was still blocked on the pipes of a grandchild after the reap. On Windows the worker now holds a handle to the launcher for as long as it may signal it; on Unix it signals the process group recorded at start instead of looking the group up from a PID that may already name a stranger. Document that a joined pull shares the owning request's lifetime, record the default-models-directory limit on cleanup in services-parity.md, and trim a comment that narrated history. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
relayToEngine wrote every engine:* request from its own goroutine, so an engine:cancel-pull sent right after the engine:action that started the pull could reach engine-manager first. It found nothing claimed, answered as a cancel for nothing, and the download ran to completion. The same held for the remote pull and its cancel. Write engine:action, engine:cancel-pull, engine:remote-pull-model and engine:remote-cancel-pull on the read loop and relay their responses asynchronously, which preserves the client's order through to engine-manager's own read loop. None of them is subject to the port gates or the engine-config lock the general relay applies. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
A cancel the engine-manager acknowledged retired the entry but left the status line on "canceling ...", since only a failure updated it. Report the cancellation, and keep it when the pull's own error-free reply settles afterwards. Signed-off-by: Chris Kelsey <ckelsey@nvidia.com>
| return ok && p.requested.Load() | ||
| } | ||
|
|
||
| func pullKey(engine, model string) string { return engine + "\x00" + model } |
There was a problem hiding this comment.
Why are we using a null byte here? Strings with null bytes give me the heeby jeebies (too much C++ programming), so maybe we can use a different differentiator?
| } | ||
| e.pullMu.Unlock() | ||
| if p == nil { | ||
| return nil |
There was a problem hiding this comment.
I'm not sure about this immediate return. Can't this happen if key is not in e.pulls nor e.pullClaims?
Here's a generated test that fails and I think we want to fix:
// A cancel can also land just after the pull it targets finished, and that one
// has nothing to stop. Holding it for the next pull of the same model made a
// retry report "cancelled" without downloading anything.
func TestPendingCancelTargetsOriginalWhenRetryRegistersFirst(t *testing.T) {
ex := NewExecutor(NewRegistry(), NewReporter(nil), nil, t.TempDir())
releaseOriginal := ex.claimPull("ollama", "demo", true)
defer releaseOriginal()
if err := ex.CancelModelPull(context.Background(), "ollama", "demo"); err != nil {
t.Fatalf("cancel original pull: %v", err)
}
releaseRetry := ex.claimPull("ollama", "demo", true)
defer releaseRetry()
originalRan := false
retryRan := false
retryResult, err := ex.trackedPull(context.Background(), "ollama", "demo", func(ctx context.Context) (json.RawMessage, error) {
retryRan = true
return succeedingPull(ctx)
})
if err != nil {
t.Fatalf("retry pull: %v", err)
}
originalResult, err := ex.trackedPull(context.Background(), "ollama", "demo", func(ctx context.Context) (json.RawMessage, error) {
originalRan = true
return succeedingPull(ctx)
})
if err != nil {
t.Fatalf("original pull: %v", err)
}
if originalRan {
t.Error("original pull started after its cancellation was acknowledged")
}
if !retryRan {
t.Error("retry inherited the original pull's cancellation")
}
assertPullStatus(t, originalResult, "cancelled")
assertPullStatus(t, retryResult, "success")
}
| | `engine:stop` | `{ engine }` | `EngineStatus` | | ||
| | `engine:restart` | `{ engine }` | `EngineStatus` | | ||
| | `engine:action` | `{ engine, action, params }` | the engine's raw response | | ||
| | `engine:cancel-pull` | `{ engine, model }` | `null` once the transfer has stopped and its partial files are cleaned up | |
There was a problem hiding this comment.
Nit: "its partial files are cleaned up " -> "cleanup has settled" since we will intentionally leave some files behind.
| A download is started with `engine:action{pull_model}`, or | ||
| `engine:remote-pull-model` for a pinned peer, and stopped with | ||
| `engine:cancel-pull` or `engine:remote-cancel-pull`. All of them name their | ||
| target in a `model` field: an engine can be downloading several models at once, |
There was a problem hiding this comment.
"several models at once"? I thought it was one at a time in a queue.
|
|
||
| The model-bearing commands — `pullModel`, `cancelModelPull`, `loadModel`, | ||
| `unloadModel`, `deleteModel`, and `setModelExpiry` — carry the target in | ||
| `model`. It is what lets a node run several downloads at once and have each |
There was a problem hiding this comment.
"several downloads at once" - same concern as architecture.md
| spinner. | ||
| completion (clearing the entry and refreshing the model list). LM Studio's CLI | ||
| progress updates the same percentage display. A pull for an engine that already | ||
| has one running is reported as `stage:"queued"` until its turn. |
There was a problem hiding this comment.
Can you clarify that it joins onto an existing download? We don't download it twice, right?
| once the transfer has stopped is shared with another client even by that | ||
| measure, so it survives as well: | ||
|
|
||
| ```mermaid |
There was a problem hiding this comment.
This diagram might want to mention if the existing request is already in progress. From trackedPull "A second request for a download already in flight joins it"
|
|
||
| A download is started with `engine:action{pull_model}`, or | ||
| `engine:remote-pull-model` for a pinned peer, and stopped with | ||
| `engine:cancel-pull` or `engine:remote-cancel-pull`. All of them name their |
There was a problem hiding this comment.
LMStudio has "model", but IIUC, Ollama has {name: model}
| | `ollamaLoadResponseHeaderTimeout` (`executor.go`) | 10min | Only Ollama's local `run_model` action gets the cold-load allowance. | | ||
| | `OLLAMA_LOAD_PENDING_TIMEOUT_MS` (`pending-actions.store.ts`) | 12min | Ollama's Load control stays locked beyond the remote path's 11-minute header budget, while backend success or failure still clears it immediately. | | ||
| | `PENDING_TIMEOUT_MS` | 60s | Unchanged for every other command, including an Ollama delete. | | ||
| | `remoteReadyResponseHeaderTimeout` (`remoteclient.go`) | 11min | Remote start/delete responses can wait on engine readiness, and a cold Ollama model load can withhold headers for minutes, so those calls use the readiness-sized client. Other model actions retain the ordinary 30s budget. | |
There was a problem hiding this comment.
"Other model actions retain the ordinary 30s budget." The docs on "waitsForEngineReadiness" say it's a longer timeout, but I didn't see any timeout in the code.
| engInstallKey = key.NewBinding(key.WithKeys("i"), key.WithHelp("i", "install")) | ||
| engUninstallKey = key.NewBinding(key.WithKeys("u"), key.WithHelp("u", "uninstall")) | ||
| engPullKey = key.NewBinding(key.WithKeys("p"), key.WithHelp("p", "pull model")) | ||
| engCancelPullKey = key.NewBinding(key.WithKeys("c"), key.WithHelp("c", "cancel pull")) |
There was a problem hiding this comment.
Update the README with the new keys?
kjlubick
left a comment
There was a problem hiding this comment.
Still have some questions/concerns about the logic for partial detection.
| } | ||
|
|
||
| func ollamaBlobsDir(st *engineState) string { | ||
| root := st.plat.Runtime.Env["OLLAMA_MODELS"] |
There was a problem hiding this comment.
Should this also incorporate Runtime.LaunchEnv? (sidenote: I was confused about LaunchEnv vs Env so am drafting a PR to add some commentary/examples to registry.go)
| // runs out. A file still busy at the deadline is left in place; that is the | ||
| // only safe outcome, and is not a cleanup failure. A removal error still | ||
| // standing at the deadline is. | ||
| func retryPartialCleanup(pass func(partialSteadyPasses) (busy bool, err error)) error { |
There was a problem hiding this comment.
The indirection makes this a smidge hard to reason about (i.e. tracking control flow here to ollamapull.go back to removeSettledPartials) but I don't have any silver bullet ideas for making it cleaner. If you have any inspiration for making it simpler, I'm all ears but otherwise it's fine as is. Maybe if "pass" had a different name.
| // one that could not be read at all is unknown, which is reported as busy | ||
| // rather than quietly treated as success. | ||
| func quiescentPaths(ctx context.Context, paths []string) (stable []stablePath, busy bool) { | ||
| watching := make(map[string]os.FileInfo, len(paths)) |
There was a problem hiding this comment.
The fact that quiescentPaths starts fresh leaves us with a potential case where an edit happens and we miss it.
sequenceDiagram
participant C as Cleanup
participant F as Partial file
participant W as Another download writer
C->>F: Pass 1 observes size 100
Note over C,F: Three checks see the same metadata
C->>C: Stable count becomes 1
Note over C: Pause between passes
W->>F: Append bytes, size becomes 200
C->>F: Pass 2 starts with baseline size 200
Note over C,F: Three checks see the same new metadata
C->>C: Stable count becomes 2
C->>F: Final check still sees size 200
C->>F: Delete the partial
Do we really need quiescentPaths and partialSteadyPasses? Or does partialSteadyPasses need more info to re-seed quiescentPaths or something?
| } | ||
| output := &lmsDownloadOutput{lastPercent: -1, progress: func(percent int) { | ||
| if ctx.Err() == nil { | ||
| e.emitPullProgress(ProgressEvent{Engine: engine, Model: model, Op: "pull", Stage: "downloading", Percent: percent, Message: model}) |
There was a problem hiding this comment.
I see this emits a pulling and downloading notification but don't see a "completed" one.
| // holds a cancel behind the pull it names has no attempt to wait for, so it | ||
| // must let the cancel through instead of parking it for its whole window, and | ||
| // B must answer a cancel for nothing as a no-op rather than an error. | ||
| func TestRemoteEngineCancelPull(t *testing.T) { |
There was a problem hiding this comment.
Codex suggests this additional case which I think also seems good:
sequenceDiagram
participant A as Initiating node
participant B as Peer engine manager
participant D as Fake download
participant F as Temporary cache
A->>B: Pull model
B->>D: Start blocking download
D->>F: Create partial data
B-->>A: Progress attributed to the model
A->>B: Cancel that model
B->>D: Stop transfer
D-->>B: Transfer stopped
B->>F: Perform cancellation cleanup
F-->>B: Cleanup settled
par Cancellation response
B-->>A: Cancel acknowledged
and Original pull response
B-->>A: Pull result is cancelled
end
We want to make sure we handle the case where a download is active.
| // (`nvpair-engine-manager/remote.go` `remoteProgressFn`), which is what | ||
| // keys a frame to its optimistic entry. An install carries none, and | ||
| // needs none: its progress is tracked per engine. | ||
| const model = stringValue(obj.model) |
There was a problem hiding this comment.
Some existing tests don't include the model (but a message with the model?)
https://github.com/NVIDIA/Personal-AI-Router/blob/7a4d689eb804b979d0114c004bca3397e105b050/desktop/tests/modular/remote-pull-progress.test.ts#L98C1-L104C11
Is that an oversight in the tests?
Does the new behavior need tests?
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.A second review pass found the following, fixed in the last three commits:
lms getoutput named its file, and nothing is deleted when it named none.cmd.Waitcan return up todownloadWaitDelayafter the reap, so a stop signal could still reach a reissued PID in that gap. Windows now holds a handle to the launcher while it may be signalled, and Unix signals the process group recorded at start instead of looking it up from the PID.engine:*request from its own goroutine, so a cancel could reach engine-manager ahead of the pull it names. It now writes the pull and cancel methods in the order the client sent them, which is whynvpair-ui-brokeris bumped.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.After the second review pass, on Windows:
The race-detector run above predates the second pass and was not repeated.
Not yet run:
lms getprints the name of the file it downloads. Unpinned LM Studio cleanup depends on it; if it prints none, cancelling an unpinned pull stops the download and leaves its partial for LM Studio to resume.Risk
Checklist
git commit -s), certifying the Developer Certificate of Origin.services/versions.jsonis written by automation — do not edit it by hand.