diff --git a/src/lib/download.ts b/src/lib/download.ts index e2c4f5a..1678875 100644 --- a/src/lib/download.ts +++ b/src/lib/download.ts @@ -4,9 +4,11 @@ import { createWriteStream, existsSync, mkdirSync, + readFileSync, renameSync, rmSync, statSync, + writeFileSync, } from "node:fs"; import { dirname } from "node:path"; import { Readable, Transform } from "node:stream"; @@ -18,6 +20,13 @@ import { loggedFetch } from "@/lib/log.js"; // memory, which is fine at 5-100MB and hopeless at 1.1GB — this helper pipes // the body straight to disk, hashes it as it flows, and resumes interrupted // transfers from a `.partial` file via HTTP Range. +// +// Staleness without a manifest: the server's ETag is persisted next to the +// file (`${dest}.etag`). A finished file is revalidated with If-None-Match +// (304 = still the published object, anything else = republished, re-download) +// and a resume is guarded with If-Range so stale partial bytes are never +// spliced onto a republished object. The truth lives on the object itself, so +// nothing on the publish side can drift. export interface DownloadProgress { received: number; @@ -50,15 +59,53 @@ async function fileSha256(path: string): Promise { return hash.digest("hex"); } +function readEtag(etagFile: string): string | null { + try { + const etag = readFileSync(etagFile, "utf8").trim(); + return etag.length > 0 ? etag : null; + } catch { + return null; + } +} + export async function downloadFile(opts: DownloadOptions): Promise { const { url, dest, endpoint, onProgress, signal } = opts; const partial = `${dest}.partial`; + const etagFile = `${dest}.etag`; - // A finished file that already verifies makes the whole call a no-op, so - // re-running after a crash between files never re-downloads. + // A finished file short-circuits the download. With an expected sha256 it + // must verify. Without one, a stored ETag lets us ask the server whether + // the published object changed; no ETag on record (a manually copied file, + // or a pre-ETag download) means the local file is trusted as-is. if (existsSync(dest)) { - if (!opts.sha256 || (await fileSha256(dest)) === opts.sha256) return; - rmSync(dest); + if (opts.sha256) { + if ((await fileSha256(dest)) === opts.sha256) return; + rmSync(dest); + } else { + const etag = readEtag(etagFile); + if (!etag) return; + let probe: Response; + try { + probe = await loggedFetch(endpoint, url, { + headers: { "If-None-Match": etag }, + signal, + }); + } catch { + return; // server unreachable — keep what we have + } + if (probe.status === 304 || !probe.ok) { + // Still the published object (304), or a server hiccup — either + // way the local file is the best copy available. + await probe.body?.cancel(); + return; + } + // Republished: drop the response (the fresh download below streams + // its own) and every local trace of the old object. + await probe.body?.cancel(); + rmSync(dest, { force: true }); + rmSync(partial, { force: true }); + rmSync(etagFile, { force: true }); + } } mkdirSync(dirname(dest), { recursive: true }); @@ -75,8 +122,17 @@ export async function downloadFile(opts: DownloadOptions): Promise { }); } + // If-Range makes a resume safe across republishes: when the entity no + // longer matches the partial's ETag, the server ignores Range and answers + // 200, and the start-over branch below discards the stale partial bytes. + const headers: Record = {}; + if (offset > 0) { + headers.Range = `bytes=${offset}-`; + const etag = readEtag(etagFile); + if (etag) headers["If-Range"] = etag; + } const res = await loggedFetch(endpoint, url, { - headers: offset > 0 ? { Range: `bytes=${offset}-` } : undefined, + headers: offset > 0 ? headers : undefined, signal, }); @@ -97,6 +153,13 @@ export async function downloadFile(opts: DownloadOptions): Promise { } if (res.body === null) throw new Error(`download had no body: ${url}`); + // Persist the entity tag of what we are about to write — before streaming, + // so an interrupted transfer leaves a partial+ETag pair the next resume can + // validate with If-Range. A server that sends no ETag clears the record. + const resEtag = res.headers.get("etag"); + if (resEtag) writeFileSync(etagFile, resEtag); + else rmSync(etagFile, { force: true }); + // A missing header must fall through to opts.size. `Number(null)` is 0, which // is finite — reading it straight would make the fallback unreachable and // report a total of `offset` (0 on a fresh download) for every chunked diff --git a/src/lib/help.ts b/src/lib/help.ts index 16e3f11..a94d059 100644 --- a/src/lib/help.ts +++ b/src/lib/help.ts @@ -55,6 +55,8 @@ Skill hub: (--platform ubuntu|macos|windows to fetch for another OS [implies --download-only], --dir for the download folder, --download-only to skip running the installer, - --minimal / --skip-verify passed through to the installer) + --minimal / --skip-verify passed through to the + installer, --force-skills to replace already-installed + skills with the bundled versions) `); } diff --git a/src/lib/office.ts b/src/lib/office.ts index ef016e6..8dc19d7 100644 --- a/src/lib/office.ts +++ b/src/lib/office.ts @@ -15,7 +15,7 @@ import { officeDownloadsDir } from "@/lib/paths.js"; // installer that prompts for sudo/UAC, which an Ink render would fight over. export const OFFICE_USAGE = - "Usage: codevhub skill office [--platform ubuntu|macos|windows] [--dir ] [--download-only] [--minimal] [--skip-verify]"; + "Usage: codevhub skill office [--platform ubuntu|macos|windows] [--dir ] [--download-only] [--minimal] [--skip-verify] [--force-skills]"; export type OfficePlatform = "ubuntu" | "macos" | "windows"; @@ -36,6 +36,7 @@ export interface OfficeArgs { downloadOnly: boolean; minimal: boolean; skipVerify: boolean; + forceSkills: boolean; error?: string; } @@ -44,6 +45,7 @@ export function parseOfficeArgs(argv: string[]): OfficeArgs { downloadOnly: false, minimal: false, skipVerify: false, + forceSkills: false, }; for (let i = 0; i < argv.length; i++) { const arg = argv[i]; @@ -84,6 +86,9 @@ export function parseOfficeArgs(argv: string[]): OfficeArgs { case "--skip-verify": parsed.skipVerify = true; break; + case "--force-skills": + parsed.forceSkills = true; + break; default: parsed.error = `unknown option: ${arg}`; return parsed; @@ -162,16 +167,18 @@ export function installerArgs( platform: OfficePlatform, ): string[] { // The bash scripts take GNU-style flags; the PowerShell script takes - // -Minimal / -SkipVerify switches. + // -Minimal / -SkipVerify / -ForceSkills switches. if (platform === "windows") { return [ ...(parsed.minimal ? ["-Minimal"] : []), ...(parsed.skipVerify ? ["-SkipVerify"] : []), + ...(parsed.forceSkills ? ["-ForceSkills"] : []), ]; } return [ ...(parsed.minimal ? ["--minimal"] : []), ...(parsed.skipVerify ? ["--skip-verify"] : []), + ...(parsed.forceSkills ? ["--force-skills"] : []), ]; } @@ -231,7 +238,8 @@ export async function runSkillOffice( console.error( `Heads-up: the bundle is ~${APPROX_BUNDLE_MB[platform]} MB — downloading might ` + "take a while. An interrupted run picks up where it left off; an " + - "already-downloaded file is reused (delete it to force a fresh download).", + "already-downloaded bundle is reused after checking with the server " + + "that it is still the published version.", ); logInfo("office bundle download starting", { action: "office.install", diff --git a/tests/lib/download.test.ts b/tests/lib/download.test.ts index 29adce1..80ae975 100644 --- a/tests/lib/download.test.ts +++ b/tests/lib/download.test.ts @@ -65,9 +65,17 @@ beforeEach(async () => { res.writeHead(404).end(); return; } + // MinIO-style conditional semantics: a per-body ETag, 304 on a matching + // If-None-Match, and Range honored only when If-Range (if sent) matches. + const etag = `"${sha256(body).slice(0, 16)}"`; const range = req.headers.range; rangeLog.push(range); - if (range && !ignoreRange) { + if (req.headers["if-none-match"] === etag) { + res.writeHead(304, { ETag: etag }).end(); + return; + } + const ifRange = req.headers["if-range"]; + if (range && !ignoreRange && (ifRange === undefined || ifRange === etag)) { const start = Number(/^bytes=(\d+)-$/.exec(range)?.[1]); if (!Number.isFinite(start) || start >= body.length) { res.writeHead(416).end(); @@ -77,16 +85,17 @@ beforeEach(async () => { res.writeHead(206, { "Content-Length": String(rest.length), "Content-Range": `bytes ${start}-${body.length - 1}/${body.length}`, + ETag: etag, }); res.end(rest); return; } if (omitContentLength) { - res.writeHead(200); + res.writeHead(200, { ETag: etag }); res.end(body); return; } - res.writeHead(200, { "Content-Length": String(body.length) }); + res.writeHead(200, { "Content-Length": String(body.length), ETag: etag }); res.end(body); }); await new Promise((resolve) => server.listen(0, resolve)); @@ -229,6 +238,45 @@ describe("downloadFile", () => { }); }); +describe("downloadFile ETag revalidation", () => { + test("a finished file is kept when the server answers 304", async () => { + const dest = join(tempDir, "payload.bin"); + await downloadFile({ url: `${baseUrl}/payload.bin`, dest, endpoint: "t" }); + const before = rangeLog.length; + await downloadFile({ url: `${baseUrl}/payload.bin`, dest, endpoint: "t" }); + expect(readFileSync(dest).equals(PAYLOAD)).toBe(true); + // Exactly one request: the conditional probe, no re-download. + expect(rangeLog.length).toBe(before + 1); + }); + + test("a republished object is re-downloaded (ETag mismatch)", async () => { + const dest = join(tempDir, "payload.bin"); + await downloadFile({ url: `${baseUrl}/payload.bin`, dest, endpoint: "t" }); + const NEW = Buffer.from("republished-bytes"); + objects.set("/payload.bin", NEW); + await downloadFile({ url: `${baseUrl}/payload.bin`, dest, endpoint: "t" }); + expect(readFileSync(dest).equals(NEW)).toBe(true); + }); + + test("a file with no ETag on record is trusted without any request", async () => { + const dest = join(tempDir, "payload.bin"); + writeFileSync(dest, "manually-placed"); + await downloadFile({ url: `${baseUrl}/payload.bin`, dest, endpoint: "t" }); + expect(readFileSync(dest, "utf8")).toBe("manually-placed"); + expect(rangeLog).toEqual([]); + }); + + test("a resume across a republish restarts instead of splicing (If-Range)", async () => { + const dest = join(tempDir, "payload.bin"); + // A partial and ETag from an older publish of the object. + writeFileSync(`${dest}.partial`, Buffer.from("stale-old-bytes")); + writeFileSync(`${dest}.etag`, '"stale-etag"'); + await downloadFile({ url: `${baseUrl}/payload.bin`, dest, endpoint: "t" }); + // If-Range mismatched → server sent 200 → clean restart, correct bytes. + expect(readFileSync(dest).equals(PAYLOAD)).toBe(true); + }); +}); + // End-to-end through runSkillOffice against the local server. The test host is // linux/macos in CI, so the detected platform maps to one of the bash bundles. // File names are the deterministic per-platform contract — no manifest. @@ -393,23 +441,38 @@ describe("runSkillOffice", () => { }); describe("installerArgs", () => { - const base = { downloadOnly: false, minimal: true, skipVerify: true }; + const base = { + downloadOnly: false, + minimal: true, + skipVerify: true, + forceSkills: true, + }; test("bash platforms get GNU-style flags", () => { expect(installerArgs(base, "ubuntu")).toEqual([ "--minimal", "--skip-verify", + "--force-skills", ]); }); test("windows gets PowerShell switches", () => { - expect(installerArgs(base, "windows")).toEqual(["-Minimal", "-SkipVerify"]); + expect(installerArgs(base, "windows")).toEqual([ + "-Minimal", + "-SkipVerify", + "-ForceSkills", + ]); }); test("no flags when none requested", () => { expect( installerArgs( - { downloadOnly: false, minimal: false, skipVerify: false }, + { + downloadOnly: false, + minimal: false, + skipVerify: false, + forceSkills: false, + }, "windows", ), ).toEqual([]); diff --git a/tests/lib/office.test.ts b/tests/lib/office.test.ts index f6096a9..f5d5f85 100644 --- a/tests/lib/office.test.ts +++ b/tests/lib/office.test.ts @@ -26,6 +26,7 @@ describe("parseOfficeArgs", () => { downloadOnly: false, minimal: false, skipVerify: false, + forceSkills: false, }); }); @@ -38,6 +39,7 @@ describe("parseOfficeArgs", () => { "--download-only", "--minimal", "--skip-verify", + "--force-skills", ]); expect(parsed).toEqual({ platform: "windows", @@ -45,6 +47,7 @@ describe("parseOfficeArgs", () => { downloadOnly: true, minimal: true, skipVerify: true, + forceSkills: true, }); });