fix(install,tests): fmt nit + downloadText flowing-mode bug
CI on PR #684 caught two real issues that local checks missed: **Lint failure (cargo fmt).** A regression test landed with a multi-line `let ContentBlock::Text { text, .. } = real_user.content...` pattern that local rustfmt accepted but CI's pinned toolchain collapsed onto a single line. Reformatted to match. **npm wrapper smoke failure ("Checksum manifest is missing deepseek-<platform>").** Subtle Node.js streams interaction in `install.js` introduced by the network-resilience cluster: * `httpRequest` attaches a `data` event listener on the response to re-arm the stall timer. * Attaching a `data` listener on a `Readable` puts the stream into flowing mode immediately. * `downloadText` then ran `for await (const chunk of response)` to collect the body — the async iterator expects paused-mode and silently misses chunks that flow before / between iteration ticks. * For small bodies (the ~100-byte SHA256 manifest), the entire response could flow through the stall listener before the async iterator's `read()` calls landed, leaving the joined body empty. * Result: `parseChecksumManifest("")` returned an empty Map → `verifyChecksum` saw no entries → "manifest is missing X" after the actual binary download succeeded. Binary downloads were unaffected because `download()` uses `response.pipe(sink)` plus a `data` listener for progress — both consume chunks via `data` events, no async iterator involved. Fix: collect the response body in `downloadText` via direct `data`/ `end` event subscription. `data` listeners stack — both the stall re-arm and the body collector fire on every chunk, no flowing-vs- paused conflict. Stall detection still works. Verified locally: `node scripts/release/npm-wrapper-smoke.js` "npm wrapper smoke passed with local assets from <url>". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -635,10 +635,7 @@ fn turn_metadata_skips_tool_result_messages() {
|
||||
// The earlier real user message receives the turn_meta prefix.
|
||||
let real_user = messages.first().expect("first user message");
|
||||
assert_eq!(real_user.role, "user");
|
||||
let ContentBlock::Text { text, .. } = real_user
|
||||
.content
|
||||
.first()
|
||||
.expect("user text content")
|
||||
let ContentBlock::Text { text, .. } = real_user.content.first().expect("user text content")
|
||||
else {
|
||||
panic!("expected Text block on real user message");
|
||||
};
|
||||
|
||||
@@ -778,12 +778,26 @@ async function downloadText(url) {
|
||||
stallMs: downloadStallMs(),
|
||||
});
|
||||
const response = result.response;
|
||||
const chunks = [];
|
||||
response.setEncoding("utf8");
|
||||
for await (const chunk of response) {
|
||||
chunks.push(chunk);
|
||||
}
|
||||
return chunks.join("");
|
||||
// NOTE: do NOT use `for await (const chunk of response)` here.
|
||||
// `httpRequest` attaches a `data` listener on the response to re-arm
|
||||
// the stall timer, which puts the stream in flowing mode. The async
|
||||
// iterator expects paused mode and will silently miss every chunk —
|
||||
// this manifested as an empty checksum manifest in the npm wrapper
|
||||
// smoke test ("Checksum manifest is missing <asset>"). Subscribing
|
||||
// to `data` events directly stacks alongside the stall listener and
|
||||
// both fire per chunk, so we collect the body correctly without
|
||||
// disturbing the stall detection.
|
||||
return new Promise((resolve, reject) => {
|
||||
const chunks = [];
|
||||
response.on("data", (chunk) => {
|
||||
chunks.push(chunk);
|
||||
});
|
||||
response.on("end", () => {
|
||||
resolve(chunks.join(""));
|
||||
});
|
||||
response.on("error", reject);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user