feat: office bundle ETag revalidation and --force-skills passthrough

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<string> {
 	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<void> {
 	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<void> {
 		});
 	}
 
+	// 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<string, string> = {};
+	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<void> {
 	}
 	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

src/lib/help.ts

@@ -55,6 +55,8 @@ Skill hub:
                          (--platform ubuntu|macos|windows to fetch for another OS
                          [implies --download-only], --dir <path> 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)
 `);
 }

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 <path>] [--download-only] [--minimal] [--skip-verify]";
+	"Usage: codevhub skill office [--platform ubuntu|macos|windows] [--dir <path>] [--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",

tests/lib/download.test.ts

@@ -65,9 +65,17 @@
 			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 @@
 			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<void>((resolve) => server.listen(0, resolve));
@@ -229,6 +238,45 @@
 	});
 });
 
+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.
@@ -255,7 +303,7 @@
 				return 0;
 			},
 		);
 		expect(code).toBe(0);
 		expect(spawns).toEqual([]);
 		expect(readFileSync(join(dir, bundleName)).equals(BUNDLE)).toBe(true);
 		expect(readFileSync(join(dir, scriptName)).equals(SCRIPT)).toBe(true);
@@ -272,7 +320,7 @@
 				return 0;
 			},
 		);
 		expect(code).toBe(0);
 		expect(spawned).toEqual({
 			command: "bash",
 			args: [join(dir, scriptName), "--minimal", "--skip-verify"],
@@ -283,7 +331,7 @@
 	test("propagates the installer's exit code", async () => {
 		const dir = join(tempDir, "office");
 		const code = await runSkillOffice(["--dir", dir], baseUrl, async () => 7);
 		expect(code).toBe(7);
 	});
 
 	test("a cross-platform --platform forces download-only", async () => {
@@ -300,7 +348,7 @@
 			},
 		);
 		expect(code).toBe(0);
 		expect(spawns).toEqual([]);
 		expect(existsSync(join(dir, "codev-office-windows.zip"))).toBe(true);
 	});
 
@@ -313,7 +361,7 @@
 			["--download-only", "--dir", dir],
 			baseUrl,
 		);
 		expect(code).toBe(0);
 		expect(readFileSync(join(dir, scriptName)).equals(SCRIPT)).toBe(true);
 		// No checksum to disagree with, so the existing bundle is trusted as-is.
 		expect(readFileSync(join(dir, bundleName), "utf8")).toBe("stale-bundle");
@@ -330,7 +378,7 @@
 			["--download-only", "--dir", dir],
 			baseUrl,
 		);
 		expect(code).toBe(0);
 		expect(readFileSync(join(dir, scriptName)).equals(SCRIPT)).toBe(true);
 		// Both requests went out without a Range header.
 		expect(rangeLog).toEqual([undefined, undefined]);
@@ -393,23 +441,38 @@
 });
 
 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([]);

tests/lib/office.test.ts

@@ -26,6 +26,7 @@ describe("parseOfficeArgs", () => {
 			downloadOnly: false,
 			minimal: false,
 			skipVerify: false,
+			forceSkills: false,
 		});
 	});
 
@@ -38,13 +39,15 @@ describe("parseOfficeArgs", () => {
 			"--download-only",
 			"--minimal",
 			"--skip-verify",
+			"--force-skills",
 		]);
 		expect(parsed).toEqual({
 			platform: "windows",
 			dir: "/tmp/x",
 			downloadOnly: true,
 			minimal: true,
 			skipVerify: true,
+			forceSkills: true,
 		});
 	});