diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c68409f79..249e07b6a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `omp tiny-models download` JSON/text failures to include the worker-side download error instead of collapsing every worker failure to `ok:false`. ([#3839](https://github.com/can1357/oh-my-pi/issues/3839)) + ## [16.2.6] - 2026-06-29 ### Changed diff --git a/packages/coding-agent/src/cli/tiny-models-cli.ts b/packages/coding-agent/src/cli/tiny-models-cli.ts index 02a3be475..b2efc23b0 100644 --- a/packages/coding-agent/src/cli/tiny-models-cli.ts +++ b/packages/coding-agent/src/cli/tiny-models-cli.ts @@ -28,12 +28,21 @@ interface ProgressReporter { interface DownloadResult { model: TinyLocalModelKey; ok: boolean; + error?: string; } function writeLine(text = ""): void { process.stdout.write(`${text}\n`); } +function downloadErrorSummary(error: string | undefined): string | undefined { + return error + ?.split(/\r?\n/) + .map(line => line.trim()) + .find(line => line.length > 0) + ?.replace(/^Error:\s*/, ""); +} + export function resolveModels(model: string | undefined): TinyLocalModelKey[] { if (!model) return [DEFAULT_TINY_TITLE_LOCAL_MODEL_KEY]; // `all` is a prefetch convenience: skip models that fail before load (unsupported @@ -101,10 +110,15 @@ async function downloadOne(modelKey: TinyLocalModelKey, json: boolean | undefine const label = getTinyLocalModelSpec(modelKey)?.label ?? modelKey; if (!json && !process.stdout.isTTY) writeLine(`Downloading ${label} (${modelKey})...`); const progress = makeProgressReporter(modelKey, json); - const ok = await tinyTitleClient.downloadModel(modelKey, { onProgress: progress.onProgress }); - progress.finish(ok); - if (!json && !process.stdout.isTTY) writeLine(ok ? `Downloaded ${label}.` : `Failed to download ${label}.`); - return { model: modelKey, ok }; + const result = await tinyTitleClient.downloadModel(modelKey, { onProgress: progress.onProgress }); + progress.finish(result.ok); + const error = downloadErrorSummary(result.error); + if (!json && !process.stdout.isTTY) { + writeLine(result.ok ? `Downloaded ${label}.` : `Failed to download ${label}${error ? `: ${error}` : ""}.`); + } else if (!json && !result.ok && error) { + writeLine(`${label} failed: ${error}`); + } + return result.error ? { model: modelKey, ok: result.ok, error: result.error } : { model: modelKey, ok: result.ok }; } export async function runTinyModelsCommand(command: TinyModelsCommandArgs): Promise { diff --git a/packages/coding-agent/src/tiny/title-client.ts b/packages/coding-agent/src/tiny/title-client.ts index 5ea9c9bab..b2d31b6d3 100644 --- a/packages/coding-agent/src/tiny/title-client.ts +++ b/packages/coding-agent/src/tiny/title-client.ts @@ -29,7 +29,12 @@ import type { TinyTitleProgressEvent, TinyTitleWorkerInbound, TinyTitleWorkerOut type PendingRequest = | { kind: "generate"; modelKey: TinyTitleLocalModelKey; resolve: (title: string | null) => void } | { kind: "complete"; modelKey: TinyMemoryLocalModelKey; resolve: (text: string | null) => void } - | { kind: "download"; modelKey: TinyLocalModelKey; resolve: (ok: boolean) => void }; + | { kind: "download"; modelKey: TinyLocalModelKey; resolve: (result: TinyTitleDownloadResult) => void }; + +export interface TinyTitleDownloadResult { + ok: boolean; + error?: string; +} export interface TinyTitleDownloadOptions { signal?: AbortSignal; @@ -269,21 +274,21 @@ export class TinyTitleClient { } } - async downloadModel(modelKey: string, options: TinyTitleDownloadOptions = {}): Promise { - if (!isTinyLocalModelKey(modelKey)) return false; - if (options.signal?.aborted) return false; + async downloadModel(modelKey: string, options: TinyTitleDownloadOptions = {}): Promise { + if (!isTinyLocalModelKey(modelKey)) return { ok: false }; + if (options.signal?.aborted) return { ok: false }; const unsubscribe = options.onProgress ? this.onProgress(options.onProgress) : undefined; try { const worker = this.#ensureWorker(); const id = String(++this.#nextRequestId); - const { promise, resolve } = Promise.withResolvers(); + const { promise, resolve } = Promise.withResolvers(); this.#addPending(id, { kind: "download", modelKey, resolve }); const abort = (): void => { const pending = this.#pending.get(id); if (pending?.kind !== "download") return; this.#deletePending(id); - pending.resolve(false); + pending.resolve({ ok: false }); }; options.signal?.addEventListener("abort", abort, { once: true }); try { @@ -294,11 +299,12 @@ export class TinyTitleClient { this.#deletePending(id); } } catch (error) { + const message = error instanceof Error ? error.message : String(error); logger.debug("tiny-title: local model download failed", { modelKey, - error: error instanceof Error ? error.message : String(error), + error: message, }); - return false; + return { ok: false, error: message }; } finally { unsubscribe?.(); } @@ -314,7 +320,7 @@ export class TinyTitleClient { for (const pending of this.#pending.values()) { this.#emitProgress({ modelKey: pending.modelKey, status: "error" }); if (pending.kind === "generate" || pending.kind === "complete") pending.resolve(null); - else pending.resolve(false); + else pending.resolve({ ok: false }); } this.#pending.clear(); this.#refed = false; @@ -379,7 +385,7 @@ export class TinyTitleClient { return; } if (message.type === "downloaded") { - if (pending.kind === "download") pending.resolve(true); + if (pending.kind === "download") pending.resolve({ ok: true }); return; } if (message.type === "completion") { @@ -389,8 +395,8 @@ export class TinyTitleClient { logger.debug("tiny-title: worker returned error", { error: message.error }); this.#markFailedModel(pending); this.#emitProgress({ modelKey: pending.modelKey, status: "error" }); - if (pending.kind === "generate" || pending.kind === "complete") pending.resolve(null); - else pending.resolve(false); + if (pending.kind === "download") pending.resolve({ ok: false, error: message.error }); + else pending.resolve(null); void this.terminate(); } @@ -407,7 +413,7 @@ export class TinyTitleClient { for (const pending of this.#pending.values()) { this.#emitProgress({ modelKey: pending.modelKey, status: "error" }); if (pending.kind === "generate" || pending.kind === "complete") pending.resolve(null); - else pending.resolve(false); + else pending.resolve({ ok: false, error: error.message }); } this.#pending.clear(); void this.terminate(); diff --git a/packages/coding-agent/test/issue-1940-repro.test.ts b/packages/coding-agent/test/issue-1940-repro.test.ts index c89aed721..4568d1b48 100644 --- a/packages/coding-agent/test/issue-1940-repro.test.ts +++ b/packages/coding-agent/test/issue-1940-repro.test.ts @@ -171,10 +171,30 @@ describe("issue #3291 — tiny-model downloads keep the worker referenced", () = worker.emit({ type: "downloaded", id: downloadRequestId }); - expect(await download).toBe(true); + expect(await download).toEqual({ ok: true }); expect(worker.unrefCalls).toBe(1); } finally { await client.terminate(); } }); + + it("returns the worker error for failed download requests", async () => { + let downloadRequestId = ""; + const worker = new FakeTinyWorker(message => { + if (message.type === "download") downloadRequestId = message.id; + }); + const client = new TinyTitleClient(() => worker); + + try { + const download = client.downloadModel("lfm2-700m"); + + expect(downloadRequestId).not.toBe(""); + worker.emit({ type: "error", id: downloadRequestId, error: "Error: runtime install failed" }); + + expect(await download).toEqual({ ok: false, error: "Error: runtime install failed" }); + expect(worker.terminated).toBe(true); + } finally { + await client.terminate(); + } + }); }); diff --git a/packages/coding-agent/test/tiny-models-cli.test.ts b/packages/coding-agent/test/tiny-models-cli.test.ts index c81e49006..363d1eeba 100644 --- a/packages/coding-agent/test/tiny-models-cli.test.ts +++ b/packages/coding-agent/test/tiny-models-cli.test.ts @@ -1,6 +1,11 @@ -import { describe, expect, it } from "bun:test"; -import { resolveModels } from "@oh-my-pi/pi-coding-agent/cli/tiny-models-cli"; +import { afterEach, describe, expect, it, spyOn, vi } from "bun:test"; +import { resolveModels, runTinyModelsCommand } from "@oh-my-pi/pi-coding-agent/cli/tiny-models-cli"; import { TINY_LOCAL_MODELS } from "@oh-my-pi/pi-coding-agent/tiny/models"; +import { tinyTitleClient } from "@oh-my-pi/pi-coding-agent/tiny/title-client"; + +afterEach(() => { + vi.restoreAllMocks(); +}); describe("tiny-models download model resolution", () => { it("excludes load-blocked models from `all` so the bulk prefetch stays green", () => { @@ -25,4 +30,24 @@ describe("tiny-models download model resolution", () => { if (!blocked) return; expect(resolveModels(blocked.key)).toEqual([blocked.key]); }); + + it("includes worker error details in JSON failures", async () => { + const output: string[] = []; + spyOn(process.stdout, "write").mockImplementation((chunk: string | Uint8Array) => { + output.push(typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk)); + return true; + }); + spyOn(tinyTitleClient, "downloadModel").mockResolvedValue({ + ok: false, + error: "Error: runtime install failed\n at worker", + }); + + await expect( + runTinyModelsCommand({ action: "download", model: "lfm2-700m", flags: { json: true } }), + ).rejects.toThrow("One or more tiny title models failed to download"); + + expect(JSON.parse(output.join(""))).toEqual({ + results: [{ model: "lfm2-700m", ok: false, error: "Error: runtime install failed\n at worker" }], + }); + }); });