Repository navigation
fix(artifacts): complete positional ZIP reads - #4414
Conversation
|
Synchronized with main 5fa4def in 0b5df5e. Resolved the test import conflict using the new artifactExpansionBudget owner; the deleted decompression-budget API/tests were not restored. The diff against current main remains only the original two ZIP files. Revalidated identical final ZIP tests: current main without the short-read fix fails 3 / passes 9; fixed passes all 12. The short-read injection still uses a real descriptor and production ZIP parsing/extraction. A fresh compiled control completed enumeration and byte-exact extraction with 128 short descriptor reads. The broader artifact/ZIP suite passed 289 tests across 31 files, with 3 public CLI extraction failures: “Extraction output parent is unavailable.” The same three failures reproduce on the no-ZIP-fix current-main control. Normal ZIP inspect-artifact CLI succeeds. A real SDK stdio MCP attempt fails at open_binary readiness, leaving inspect_artifact without a target; it is not counted as passing. Fallback compile:termux, TypeScript 6, lint/module boundaries, knip, metadata, architecture guards, discovery, two-file formatting and the scoped actual pre-commit hook passed. Standard npm run check remains blocked by native typecheck's missing /proc/self/exe. Cancellation coverage proves no entry is published, not immediate interruption of every underlying read; forced-short-read public CLI/MCP remains unverified. No performance claim. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
 [Medium impact] The PR appears safe to merge; no actionable defects were found. Summary
Reviews (1) · Last reviewed commit: "fix(artifacts): retain ZIP short reads a..." · Reviewed by Greptile |
A valid ZIP can fail enumeration or extraction when a positional
FileHandle.readreturns fewer bytes than requested. Complete each read within the admitted file extent before handing it to zip.js, and report premature EOF as an integrity failure. Existing allocation limits and beyond-EOF format classification remain enforced. This is an archive-reader correction within existing scope.On main
d72d856f, the identical regression suite gives 3 failures/8 passes before the fix, then 11 passes. All 29 related filesystem/lifecycle/ZIP boundary tests pass, including compiled CLI controls.compile:termux, locked TypeScript 6--noEmit, lint/module boundaries, knip, metadata, architecture, changed-file formatting, pre-commit and diff checks pass on pinned Node 24.18.0/npm 11.16.0.Short reads are injected on real descriptors; ZIP parsing and extraction use production code. Cancellation coverage establishes no entry publication after cancellation. Standard
npm run checkremains blocked by the native compiler's unavailable/proc/self/exe. Full-suite, other-host and forced-short-read public CLI/MCP coverage remain unverified.Refs #3847, ZIP short-read candidate.
Author readiness
Human owner: @Afloat16
I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.