Skip to content

Commit db0d854

Browse files
committed
Find GGUF models in publisher/model subfolders, and resume interrupted downloads
- listModels() only ever read files directly inside the configured folder. Tools like LM Studio organize downloads as <publisher>/<model>-GGUF/<file>.gguf one level deeper than that, so pointing the llama.cpp model storage location at a shared root (e.g. an existing Ollama/LM Studio models directory) found nothing. Now walks subfolders (bounded to 6 levels) and reports each model's path relative to the configured root. Sharded models are still grouped correctly per-folder, so two different publishers' same-named shard files no longer collide into one entry. - LocalGgufModel now separates the real relative path (name, used for load/delete) from a display label, since a relative path with subdirectories isn't something you'd want to send to the identity validation you'd normally do on a bare filename — deleteModel's guard against path traversal now checks for ".." segments and containment within the root instead of rejecting any path separator outright, and sibling-shard cleanup happens in the target's own directory rather than the top-level root. - downloadGgufFile() now resumes instead of restarting: an existing <file>.gguf.part is sent as a Range request, and the response is appended rather than overwritten. Falls back to a fresh download if the server ignores the Range header (200 instead of 206) or reports the partial as stale (416). Errors during streaming — a dropped connection being the main one — now deliberately leave the .part file in place instead of deleting it, since that's real resumable progress, not corruption. This also removes the startup sweep of .part files added in the previous commit, since blowing away partial downloads on every launch would defeat the point of being able to resume them. 14 new tests across llamacpp-manager.test.ts and huggingface.test.ts cover subfolder discovery, per-folder shard grouping, nested deletion, path traversal rejection, and the resume/fresh-start/stale-416 download paths.
1 parent 13e9803 commit db0d854

5 files changed

Lines changed: 253 additions & 45 deletions

File tree

app/src/huggingface.test.ts

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
2+
import * as fs from "node:fs";
3+
import * as os from "node:os";
4+
import * as path from "node:path";
5+
import { downloadGgufFile } from "./huggingface";
6+
7+
function fakeResponse(opts: { status: number; headers?: Record<string, string>; chunks?: string[] }) {
8+
const chunks = (opts.chunks ?? []).map((c) => new TextEncoder().encode(c));
9+
let i = 0;
10+
return {
11+
ok: opts.status >= 200 && opts.status < 300,
12+
status: opts.status,
13+
headers: { get: (name: string) => opts.headers?.[name.toLowerCase()] ?? null },
14+
body: {
15+
getReader: () => ({
16+
read: async () => (i < chunks.length ? { done: false, value: chunks[i++] } : { done: true, value: undefined }),
17+
}),
18+
},
19+
} as unknown as Response;
20+
}
21+
22+
describe("downloadGgufFile", () => {
23+
let dir: string;
24+
let destPath: string;
25+
26+
beforeEach(() => {
27+
dir = fs.mkdtempSync(path.join(os.tmpdir(), "hf-download-test-"));
28+
destPath = path.join(dir, "model.gguf");
29+
});
30+
31+
afterEach(() => {
32+
vi.unstubAllGlobals();
33+
});
34+
35+
it("downloads fresh, reports progress, and renames the .part file to the final name on success", async () => {
36+
const progress: { receivedBytes: number; totalBytes: number | null }[] = [];
37+
vi.stubGlobal(
38+
"fetch",
39+
vi.fn(async () => fakeResponse({ status: 200, headers: { "content-length": "10" }, chunks: ["hello", "world"] }))
40+
);
41+
42+
await downloadGgufFile("org/model", "model.gguf", destPath, (p) => progress.push(p));
43+
44+
expect(fs.existsSync(destPath)).toBe(true);
45+
expect(fs.existsSync(destPath + ".part")).toBe(false);
46+
expect(fs.readFileSync(destPath, "utf8")).toBe("helloworld");
47+
expect(progress.at(-1)).toEqual({ receivedBytes: 10, totalBytes: 10 });
48+
});
49+
50+
it("resumes from an existing .part file using a Range request instead of starting over", async () => {
51+
fs.writeFileSync(destPath + ".part", "hello");
52+
const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => {
53+
expect((init?.headers as Record<string, string>).Range).toBe("bytes=5-");
54+
return fakeResponse({ status: 206, headers: { "content-range": "bytes 5-9/10" }, chunks: ["world"] });
55+
});
56+
vi.stubGlobal("fetch", fetchMock);
57+
58+
await downloadGgufFile("org/model", "model.gguf", destPath, () => {});
59+
60+
expect(fs.readFileSync(destPath, "utf8")).toBe("helloworld");
61+
});
62+
63+
it("discards a stale partial and starts over when the server ignores the Range request", async () => {
64+
fs.writeFileSync(destPath + ".part", "OLD-STALE-DATA");
65+
vi.stubGlobal(
66+
"fetch",
67+
vi.fn(async () => fakeResponse({ status: 200, headers: { "content-length": "5" }, chunks: ["fresh"] }))
68+
);
69+
70+
await downloadGgufFile("org/model", "model.gguf", destPath, () => {});
71+
72+
expect(fs.readFileSync(destPath, "utf8")).toBe("fresh");
73+
});
74+
75+
it("keeps the .part file (doesn't delete it) when the connection drops mid-stream, so a retry can resume", async () => {
76+
vi.stubGlobal("fetch", vi.fn(async () => ({
77+
ok: true,
78+
status: 200,
79+
headers: { get: () => "20" },
80+
body: {
81+
getReader: () => ({
82+
read: async () => {
83+
throw new Error("connection reset");
84+
},
85+
}),
86+
},
87+
} as unknown as Response)));
88+
89+
await expect(downloadGgufFile("org/model", "model.gguf", destPath, () => {})).rejects.toThrow("connection reset");
90+
expect(fs.existsSync(destPath + ".part")).toBe(true);
91+
expect(fs.existsSync(destPath)).toBe(false);
92+
});
93+
94+
it("keeps the .part file and throws when the stream ends short of the expected size", async () => {
95+
vi.stubGlobal(
96+
"fetch",
97+
vi.fn(async () => fakeResponse({ status: 200, headers: { "content-length": "100" }, chunks: ["short"] }))
98+
);
99+
100+
await expect(downloadGgufFile("org/model", "model.gguf", destPath, () => {})).rejects.toThrow(/incomplete/);
101+
expect(fs.existsSync(destPath + ".part")).toBe(true);
102+
});
103+
104+
it("discards a partial and retries once when the server responds 416 (range no longer satisfiable)", async () => {
105+
fs.writeFileSync(destPath + ".part", "stale");
106+
const fetchMock = vi
107+
.fn()
108+
.mockResolvedValueOnce(fakeResponse({ status: 416 }))
109+
.mockResolvedValueOnce(fakeResponse({ status: 200, headers: { "content-length": "5" }, chunks: ["fresh"] }));
110+
vi.stubGlobal("fetch", fetchMock);
111+
112+
await downloadGgufFile("org/model", "model.gguf", destPath, () => {});
113+
114+
expect(fs.readFileSync(destPath, "utf8")).toBe("fresh");
115+
expect(fetchMock).toHaveBeenCalledTimes(2);
116+
});
117+
});

app/src/huggingface.ts

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ export interface DownloadProgress {
5555
// of the app just not knowing about it.
5656
export const PARTIAL_DOWNLOAD_SUFFIX = ".part";
5757

58+
function parseContentRangeTotal(headerValue: string | null): number | null {
59+
// "bytes 12345-67890/98765" — the part after the slash is the full size.
60+
const match = headerValue?.match(/\/(\d+)$/);
61+
return match ? Number(match[1]) : null;
62+
}
63+
5864
export async function downloadGgufFile(
5965
modelId: string,
6066
filename: string,
@@ -64,19 +70,48 @@ export async function downloadGgufFile(
6470
): Promise<void> {
6571
const fs = await import("node:fs");
6672
const url = `https://huggingface.co/${modelId}/resolve/main/${encodeURIComponent(filename)}`;
73+
const partPath = destPath + PARTIAL_DOWNLOAD_SUFFIX;
74+
75+
// A .part file left over from a dropped connection or a force-quit is
76+
// real progress, not garbage — resume it with a Range request instead of
77+
// re-downloading from byte zero every time.
78+
let existingBytes = fs.existsSync(partPath) ? fs.statSync(partPath).size : 0;
79+
const headers: Record<string, string> = {};
80+
if (token) headers.Authorization = `Bearer ${token}`;
81+
if (existingBytes > 0) headers.Range = `bytes=${existingBytes}-`;
82+
6783
let res: Response;
6884
try {
69-
res = await fetch(url, { headers: token ? { Authorization: `Bearer ${token}` } : undefined });
85+
res = await fetch(url, { headers });
7086
} catch (err) {
7187
throw new Error(`Couldn't reach Hugging Face: ${(err as Error).message}`);
7288
}
89+
90+
if (existingBytes > 0 && res.status === 416) {
91+
// Our partial is already >= what the server has now (stale, or the
92+
// remote file changed) — it can't be resumed, so start clean once.
93+
fs.rmSync(partPath, { force: true });
94+
return downloadGgufFile(modelId, filename, destPath, onProgress, token);
95+
}
96+
let resuming = existingBytes > 0 && res.status === 206;
97+
if (existingBytes > 0 && !resuming) {
98+
// Server ignored the Range request and is sending the whole file
99+
// from the start — appending that to the stale partial would
100+
// corrupt it, so discard the partial and treat this as fresh.
101+
existingBytes = 0;
102+
}
103+
73104
if (!res.ok || !res.body) throw new Error(`Failed to download "${filename}" (HTTP ${res.status}).`);
74105

75-
const totalBytes = Number(res.headers.get("content-length")) || null;
76-
let receivedBytes = 0;
77-
const partPath = destPath + PARTIAL_DOWNLOAD_SUFFIX;
78-
const writeStream = fs.createWriteStream(partPath);
106+
const contentLength = Number(res.headers.get("content-length")) || null;
107+
const totalBytes = resuming
108+
? (parseContentRangeTotal(res.headers.get("content-range")) ?? (contentLength !== null ? existingBytes + contentLength : null))
109+
: contentLength;
110+
111+
let receivedBytes = existingBytes;
112+
const writeStream = fs.createWriteStream(partPath, { flags: resuming ? "a" : "w" });
79113
const reader = res.body.getReader();
114+
onProgress({ receivedBytes, totalBytes });
80115

81116
try {
82117
while (true) {
@@ -89,32 +124,21 @@ export async function downloadGgufFile(
89124
});
90125
}
91126
} catch (err) {
92-
writeStream.end();
93-
fs.rmSync(partPath, { force: true });
127+
// Deliberately not deleting partPath — a dropped connection here
128+
// leaves real, resumable progress on disk for the next attempt.
129+
// Waited out rather than fire-and-forget so the partial bytes are
130+
// actually flushed to disk before this function returns.
131+
await new Promise<void>((resolve) => writeStream.end(() => resolve()));
94132
throw err;
95133
}
96134
await new Promise<void>((resolve, reject) => {
97135
writeStream.once("error", reject);
98136
writeStream.end(() => resolve());
99137
});
100138
if (totalBytes !== null && receivedBytes !== totalBytes) {
101-
fs.rmSync(partPath, { force: true });
102-
throw new Error(`Download of "${filename}" was incomplete (got ${receivedBytes} of ${totalBytes} bytes).`);
139+
throw new Error(
140+
`Download of "${filename}" was incomplete (got ${receivedBytes} of ${totalBytes} bytes) — try downloading it again to resume.`
141+
);
103142
}
104143
fs.renameSync(partPath, destPath);
105144
}
106-
107-
// Leftover *.gguf.part files can only come from a download that never
108-
// finished (crash, force-quit, killed process) — there's no resume support,
109-
// so they're permanently unusable. Called once at startup rather than left
110-
// for the user to notice a phantom download stuck at some old percentage.
111-
export async function cleanupIncompleteDownloads(modelsDir: string): Promise<void> {
112-
const fs = await import("node:fs");
113-
const path = await import("node:path");
114-
if (!fs.existsSync(modelsDir)) return;
115-
for (const f of fs.readdirSync(modelsDir)) {
116-
if (f.toLowerCase().endsWith(PARTIAL_DOWNLOAD_SUFFIX)) {
117-
fs.rmSync(path.join(modelsDir, f), { force: true });
118-
}
119-
}
120-
}

app/src/llamacpp-manager.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,33 @@ describe("listModels", () => {
6464
const names = listModels(dir).map((m) => m.name);
6565
expect(names).toEqual(["finished.gguf"]);
6666
});
67+
68+
it("finds models nested in publisher/model subfolders, e.g. LM Studio's layout", () => {
69+
const modelDir = path.join(dir, "bartowski", "Some-Model-GGUF");
70+
fs.mkdirSync(modelDir, { recursive: true });
71+
fs.writeFileSync(path.join(modelDir, "some-model.gguf"), "x".repeat(20));
72+
const models = listModels(dir);
73+
expect(models).toHaveLength(1);
74+
expect(models[0].name).toBe("bartowski/Some-Model-GGUF/some-model.gguf");
75+
expect(models[0].sizeBytes).toBe(20);
76+
});
77+
78+
it("groups shards separately per subfolder instead of merging same-named shards across models", () => {
79+
const dirA = path.join(dir, "pub", "Model-A-GGUF");
80+
const dirB = path.join(dir, "pub", "Model-B-GGUF");
81+
fs.mkdirSync(dirA, { recursive: true });
82+
fs.mkdirSync(dirB, { recursive: true });
83+
fs.writeFileSync(path.join(dirA, "weights-00001-of-00002.gguf"), "x");
84+
fs.writeFileSync(path.join(dirA, "weights-00002-of-00002.gguf"), "x");
85+
fs.writeFileSync(path.join(dirB, "weights-00001-of-00002.gguf"), "x");
86+
fs.writeFileSync(path.join(dirB, "weights-00002-of-00002.gguf"), "x");
87+
const models = listModels(dir);
88+
expect(models).toHaveLength(2);
89+
expect(models.map((m) => m.name).sort()).toEqual([
90+
"pub/Model-A-GGUF/weights-00001-of-00002.gguf",
91+
"pub/Model-B-GGUF/weights-00001-of-00002.gguf",
92+
]);
93+
});
6794
});
6895

6996
describe("deleteModel", () => {
@@ -83,4 +110,23 @@ describe("deleteModel", () => {
83110
it("rejects a path-traversal attempt", async () => {
84111
await expect(deleteModel(dir, "../evil.gguf")).rejects.toThrow(/Invalid model file name/);
85112
});
113+
114+
it("deletes a model nested in a subfolder, and only its own shards", async () => {
115+
const modelDir = path.join(dir, "pub", "Model-GGUF");
116+
fs.mkdirSync(modelDir, { recursive: true });
117+
fs.writeFileSync(path.join(modelDir, "weights-00001-of-00002.gguf"), "x");
118+
fs.writeFileSync(path.join(modelDir, "weights-00002-of-00002.gguf"), "x");
119+
const otherDir = path.join(dir, "pub", "Other-GGUF");
120+
fs.mkdirSync(otherDir, { recursive: true });
121+
fs.writeFileSync(path.join(otherDir, "weights-00001-of-00002.gguf"), "x");
122+
123+
await deleteModel(dir, "pub/Model-GGUF/weights-00001-of-00002.gguf");
124+
125+
expect(fs.readdirSync(modelDir)).toEqual([]);
126+
expect(fs.readdirSync(otherDir)).toEqual(["weights-00001-of-00002.gguf"]);
127+
});
128+
129+
it("rejects a traversal attempt disguised inside a subfolder path", async () => {
130+
await expect(deleteModel(dir, "pub/../../evil.gguf")).rejects.toThrow(/Invalid model file name/);
131+
});
86132
});

app/src/llamacpp-manager.ts

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,12 @@ export async function dispose(): Promise<void> {
226226
}
227227

228228
export interface LocalGgufModel {
229-
// Real file name of the representative shard (part 1, or the lowest part
230-
// present) — this is what gets passed back to loadModel/deleteModel.
229+
// Path of the representative shard (part 1, or the lowest part present)
230+
// relative to the configured models folder, forward-slash separated
231+
// (e.g. "bartowski/Some-Model-GGUF/some-model.gguf") — this is what gets
232+
// passed back to loadModel/deleteModel. Tools like LM Studio organize
233+
// downloads as <publisher>/<model>-GGUF/<file>.gguf, so a flat single
234+
// folder isn't enough to find them.
231235
name: string;
232236
// What to show in the UI. Same as `name` for a normal single-file model;
233237
// for a multi-part one it's a synthetic "(N parts)" label instead, since
@@ -286,16 +290,30 @@ export function groupShardedModels(files: RawGgufFile[]): LocalGgufModel[] {
286290
return [...standalone, ...grouped];
287291
}
288292

293+
// Bounds the recursive scan below — comfortably covers real-world layouts
294+
// like LM Studio's <publisher>/<model>-GGUF/<file>.gguf (2 levels deep)
295+
// without walking into an unrelated, arbitrarily deep folder someone
296+
// accidentally pointed this setting at.
297+
const MAX_SCAN_DEPTH = 6;
298+
299+
function walkGgufFiles(root: string, dir: string, depth: number): RawGgufFile[] {
300+
if (depth > MAX_SCAN_DEPTH) return [];
301+
const files: RawGgufFile[] = [];
302+
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
303+
const full = path.join(dir, entry.name);
304+
if (entry.isDirectory()) {
305+
files.push(...walkGgufFiles(root, full, depth + 1));
306+
} else if (entry.isFile() && entry.name.toLowerCase().endsWith(".gguf")) {
307+
const relative = path.relative(root, full).split(path.sep).join("/");
308+
files.push({ name: relative, path: full, sizeBytes: fs.statSync(full).size });
309+
}
310+
}
311+
return files;
312+
}
313+
289314
export function listModels(modelsDir: string): LocalGgufModel[] {
290315
if (!fs.existsSync(modelsDir)) return [];
291-
const files = fs
292-
.readdirSync(modelsDir)
293-
.filter((f) => f.toLowerCase().endsWith(".gguf"))
294-
.map((f) => {
295-
const full = path.join(modelsDir, f);
296-
return { name: f, path: full, sizeBytes: fs.statSync(full).size };
297-
});
298-
return groupShardedModels(files);
316+
return groupShardedModels(walkGgufFiles(modelsDir, modelsDir, 0));
299317
}
300318

301319
// Model paths currently kept warm in modelCache — used for the
@@ -307,10 +325,14 @@ export function listLoadedModels(): string[] {
307325

308326
export async function deleteModel(modelsDir: string, name: string): Promise<void> {
309327
const root = path.resolve(modelsDir);
310-
const target = path.resolve(root, name);
311-
if (path.basename(name) !== name || !name.toLowerCase().endsWith(".gguf")) {
328+
// `name` may now be a relative path with subfolders (see LocalGgufModel),
329+
// so unlike before this can't reject on path separators — instead it
330+
// rejects ".." segments and requires the resolved path to still land
331+
// inside `root`, which is what actually prevents escaping the directory.
332+
if (path.isAbsolute(name) || name.split(/[\\/]/).includes("..") || !name.toLowerCase().endsWith(".gguf")) {
312333
throw new Error("Invalid model file name.");
313334
}
335+
const target = path.resolve(root, name);
314336
if (target === root || !target.startsWith(root + path.sep)) {
315337
throw new Error("Invalid model file name.");
316338
}
@@ -334,14 +356,17 @@ export async function deleteModel(modelsDir: string, name: string): Promise<void
334356
fs.rmSync(target, { force: true });
335357

336358
// A multi-part model's sibling shards live under the same name pattern
337-
// in the same directory — leaving them behind would orphan otherwise-
338-
// unusable files that just sit there confusing the next listModels() call.
339-
const shardMatch = name.match(SHARD_PATTERN);
359+
// in the same directory as the target — leaving them behind would orphan
360+
// otherwise-unusable files that just sit there confusing the next
361+
// listModels() call.
362+
const targetDir = path.dirname(target);
363+
const targetBasename = path.basename(target);
364+
const shardMatch = targetBasename.match(SHARD_PATTERN);
340365
if (shardMatch) {
341366
const [, base, , , ext] = shardMatch;
342367
const siblingPattern = new RegExp(`^${escapeRegExp(base)}-\\d+-of-\\d+${escapeRegExp(ext)}$`, "i");
343-
for (const f of fs.readdirSync(root)) {
344-
if (f !== name && siblingPattern.test(f)) fs.rmSync(path.join(root, f), { force: true });
368+
for (const f of fs.readdirSync(targetDir)) {
369+
if (f !== targetBasename && siblingPattern.test(f)) fs.rmSync(path.join(targetDir, f), { force: true });
345370
}
346371
}
347372

app/src/main.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -767,10 +767,6 @@ app.whenReady().then(async () => {
767767
await ollama.start();
768768
llamacpp.setModelCacheLimit(settingsStore.getSettings().llamaCppMaxCachedModels ?? 2);
769769
await llamacpp.setGpuBackend(settingsStore.getSettings().llamaCppGpuBackend ?? "auto");
770-
// Leftover *.gguf.part files from a download that never finished last
771-
// session (crash, force-quit) — there's no resume support, so they can
772-
// only ever be dead weight; clear them before the model list is ever read.
773-
await huggingface.cleanupIncompleteDownloads(getLlamaCppModelsDir());
774770
setupAutoUpdater(() => mainWindow);
775771
void connectEnabledMcpServers();
776772
scheduler.init((provider, model, prompt) => completePrompt(provider as ProviderId, model, prompt));

0 commit comments

Comments
 (0)