Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "codev-ai",
"version": "0.5.9",
"version": "0.5.10",
"description": "CoDev — AI Coding Agent Hub. Install, configure, and manage multiple AI coding agents.",
"keywords": [
"ai",
Expand Down
32 changes: 27 additions & 5 deletions src/lib/download.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,20 @@ export async function downloadFile(opts: DownloadOptions): Promise<void> {
let hash = createHash("sha256");
let offset = 0;
if (existsSync(partial)) {
offset = statSync(partial).size;
const h = hash;
await pipeline(createReadStream(partial), async (chunks) => {
for await (const chunk of chunks) h.update(chunk as Buffer);
});
// Without an expected sha256 there is nothing downstream to catch a bad
// splice, so a resume is only trustworthy when the partial's ETag is on
// record to send as If-Range. A bare Range answered with 206 would
// append the republished object's bytes onto stale ones — scrap such a
// partial and download from scratch instead.
if (!opts.sha256 && !readEtag(etagFile)) {
rmSync(partial, { force: true });
} else {
offset = statSync(partial).size;
const h = hash;
await pipeline(createReadStream(partial), async (chunks) => {
for await (const chunk of chunks) h.update(chunk as Buffer);
});
}
}

// If-Range makes a resume safe across republishes: when the entity no
Expand All @@ -138,6 +147,19 @@ export async function downloadFile(opts: DownloadOptions): Promise<void> {

let append = false;
if (res.status === 206 && offset > 0) {
// Belt and braces on top of If-Range: a middlebox that mishandles it
// can still answer 206 for a republished entity (this corrupted real
// bundle downloads — "bad zipfile offset" from spliced halves). When
// the 206 carries an ETag that differs from the partial's, the range
// is against a different object: scrap the partial and refetch clean.
const storedEtag = readEtag(etagFile);
const gotEtag = res.headers.get("etag");
if (storedEtag && gotEtag && gotEtag !== storedEtag) {
await res.body?.cancel();
rmSync(partial, { force: true });
rmSync(etagFile, { force: true });
return downloadFile(opts);
}
append = true;
} else if (res.ok) {
// 200 despite Range (server ignored it) or a fresh download: start over.
Expand Down
39 changes: 38 additions & 1 deletion tests/lib/download.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
// When true the server sends no Content-Length (chunked), as a proxy or a
// streaming origin may.
let omitContentLength = false;
// Simulate a middlebox that honors Range but ignores If-Range semantics —
// answering 206 (with the current entity's ETag) even when If-Range mismatches.
let brokenIfRange = false;
// Extra objects (path -> body) the server should serve.
let objects: Map<string, Buffer>;

Expand All @@ -62,6 +65,7 @@
rangeLog = [];
ignoreRange = false;
omitContentLength = false;
brokenIfRange = false;
objects = new Map([["/payload.bin", PAYLOAD]]);
server = createServer((req, res) => {
const body = objects.get(req.url ?? "");
Expand All @@ -79,7 +83,11 @@
return;
}
const ifRange = req.headers["if-range"];
if (range && !ignoreRange && (ifRange === undefined || ifRange === etag)) {
if (
range &&
!ignoreRange &&
(brokenIfRange || ifRange === undefined || ifRange === etag)
) {
const start = Number(/^bytes=(\d+)-$/.exec(range)?.[1]);
if (!Number.isFinite(start) || start >= body.length) {
res.writeHead(416).end();
Expand Down Expand Up @@ -279,6 +287,35 @@
// If-Range mismatched → server sent 200 → clean restart, correct bytes.
expect(readFileSync(dest).equals(PAYLOAD)).toBe(true);
});

test("refuses a bare-Range resume of an ETag-less partial without a sha256", async () => {
const dest = join(tempDir, "payload.bin");
// A partial with no `.etag` on record (pre-ETag download, or a cleared
// record). Without an expected sha256 nothing downstream would catch a
// splice, so the partial must be scrapped, not resumed.
writeFileSync(`${dest}.partial`, Buffer.from("stale-old-bytes"));
await downloadFile({ url: `${baseUrl}/payload.bin`, dest, endpoint: "t" });
// No Range header ever reached the server — a clean full download.
expect(rangeLog).toEqual([undefined]);
expect(readFileSync(dest).equals(PAYLOAD)).toBe(true);
expect(existsSync(`${dest}.partial`)).toBe(false);
});

test("scraps the partial when a broken hop answers 206 across a republish", async () => {
// A middlebox that honors Range while ignoring If-Range: it answers 206
// for a republished entity, which used to splice the halves together
// ("bad zipfile offset" on real bundle downloads).
brokenIfRange = true;
const dest = join(tempDir, "payload.bin");
writeFileSync(`${dest}.partial`, Buffer.from("stale-old-bytes"));
writeFileSync(`${dest}.etag`, '"stale-etag"');
await downloadFile({ url: `${baseUrl}/payload.bin`, dest, endpoint: "t" });
// The lying 206 carried the new object's ETag → partial scrapped, then
// a clean full refetch (no Range on the second request).
expect(rangeLog).toEqual(["bytes=15-", undefined]);
expect(readFileSync(dest).equals(PAYLOAD)).toBe(true);
expect(existsSync(`${dest}.partial`)).toBe(false);
});
});

// End-to-end through runSkillOffice against the local server. The test host is
Expand Down Expand Up @@ -307,7 +344,7 @@
return 0;
},
);
expect(code).toBe(0);

Check failure on line 347 in tests/lib/download.test.ts

View workflow job for this annotation

GitHub Actions / matrix (windows-latest)

tests/lib/download.test.ts > runSkillOffice > --download-only stages both files and never spawns

AssertionError: expected 1 to be +0 // Object.is equality - Expected + Received - 0 + 1 ❯ tests/lib/download.test.ts:347:16
expect(spawns).toEqual([]);
expect(readFileSync(join(dir, bundleName)).equals(BUNDLE)).toBe(true);
expect(readFileSync(join(dir, scriptName)).equals(SCRIPT)).toBe(true);
Expand All @@ -324,7 +361,7 @@
return 0;
},
);
expect(code).toBe(0);

Check failure on line 364 in tests/lib/download.test.ts

View workflow job for this annotation

GitHub Actions / matrix (windows-latest)

tests/lib/download.test.ts > runSkillOffice > runs the installer via bash with translated flags

AssertionError: expected 1 to be +0 // Object.is equality - Expected + Received - 0 + 1 ❯ tests/lib/download.test.ts:364:16
expect(spawned).toEqual({
command: "bash",
args: [join(dir, scriptName), "--skip-verify"],
Expand All @@ -335,7 +372,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);

Check failure on line 375 in tests/lib/download.test.ts

View workflow job for this annotation

GitHub Actions / matrix (windows-latest)

tests/lib/download.test.ts > runSkillOffice > propagates the installer's exit code

AssertionError: expected 1 to be 7 // Object.is equality - Expected + Received - 7 + 1 ❯ tests/lib/download.test.ts:375:16
});

test("a cross-platform --platform forces download-only", async () => {
Expand All @@ -352,7 +389,7 @@
},
);
expect(code).toBe(0);
expect(spawns).toEqual([]);

Check failure on line 392 in tests/lib/download.test.ts

View workflow job for this annotation

GitHub Actions / matrix (windows-latest)

tests/lib/download.test.ts > runSkillOffice > a cross-platform --platform forces download-only

AssertionError: expected [ 'powershell.exe' ] to deeply equal [] - Expected + Received - [] + [ + "powershell.exe", + ] ❯ tests/lib/download.test.ts:392:18
expect(existsSync(join(dir, "codev-office-windows.zip"))).toBe(true);
});

Expand All @@ -365,7 +402,7 @@
["--download-only", "--dir", dir],
baseUrl,
);
expect(code).toBe(0);

Check failure on line 405 in tests/lib/download.test.ts

View workflow job for this annotation

GitHub Actions / matrix (windows-latest)

tests/lib/download.test.ts > runSkillOffice > always refetches the setup script, but reuses a finished bundle

AssertionError: expected 1 to be +0 // Object.is equality - Expected + Received - 0 + 1 ❯ tests/lib/download.test.ts:405:16
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");
Expand All @@ -382,7 +419,7 @@
["--download-only", "--dir", dir],
baseUrl,
);
expect(code).toBe(0);

Check failure on line 422 in tests/lib/download.test.ts

View workflow job for this annotation

GitHub Actions / matrix (windows-latest)

tests/lib/download.test.ts > runSkillOffice > drops a stale .partial for the script instead of resuming onto it

AssertionError: expected 1 to be +0 // Object.is equality - Expected + Received - 0 + 1 ❯ tests/lib/download.test.ts:422:16
expect(readFileSync(join(dir, scriptName)).equals(SCRIPT)).toBe(true);
// Both requests went out without a Range header.
expect(rangeLog).toEqual([undefined, undefined]);
Expand Down Expand Up @@ -455,7 +492,7 @@
return 0;
},
);
expect(code).toBe(0);

Check failure on line 495 in tests/lib/download.test.ts

View workflow job for this annotation

GitHub Actions / matrix (windows-latest)

tests/lib/download.test.ts > runSkillOffice > --uninstall fetches only the uninstall script and runs it with passthroughs

AssertionError: expected 1 to be +0 // Object.is equality - Expected + Received - 0 + 1 ❯ tests/lib/download.test.ts:495:16
expect(spawned).toEqual({
command: "bash",
args: [join(dir, uninstallName), "--yes", "--skills-only"],
Expand Down
Loading