XLS-352: pin every zip entry's mtime via zipEntryDate (supersedes #2) - #3
Open
senoff wants to merge 2 commits into
Open
XLS-352: pin every zip entry's mtime via zipEntryDate (supersedes #2)#3senoff wants to merge 2 commits into
senoff wants to merge 2 commits into
Conversation
…shing The full loader (workbook.xlsx.load) threw `TypeError: Cannot read properties of undefined (reading 'match')` in colCache.decodeEx(undefined) when a workbook carried an `_xlnm.Print_Area` defined name with no range — the artifact Excel and some third-party writers leave behind when a print area is cleared. The streaming reader (WorkbookReader) already tolerated it, so a file the doctor/read path accepted was rejected by the write path. Guard the decode on `definedName.ranges[0]` being present; an absent ref means "no print area", matching Excel and the streaming reader. Valid print areas are unaffected. Regression: spec/unit/xlsx/xform/book/workbook-xform.spec.js — reconcile tolerates empty Print_Area (no throw) and still reconciles a valid one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDNnWYdcu8CXnPPMLqxHMH
Threaded to both the streaming WorkbookWriter (via a shared _append choke
point) and the buffered xlsx.write/writeBuffer path (via ZipWriter's
finalize-time _pinEntryDates, covering JSZip's auto-created folder
entries too, which never pass through append). Fixes XLS-350's
byte-identity coin-flip: date is a per-entry zip option defaulting to
new Date() at append time, and a module-level zip:{date} is a no-op.
Rebased onto the Print_Area reconcile fix so this pin is available
without dropping that fix's coverage for downstream consumers pinning
this repo by commit SHA.
Verified this session (fresh checkout): red on unpatched code (6/6 fail),
green on the fix (6/6 pass, both writer paths), 208 integration + 888
unit tests, 0 regressions.
--no-verify: pre-commit eslint fails on PRE-EXISTING lines (comma-dangle,
space-before-function-paren, an import extension) untouched by this diff
-- the known prettier<->eslint config drift already tracked as PR#69 in
this repo. Per AGENTS.md rule 1, disclosed here rather than reshaping
unrelated code or touching .eslintrc/.prettierrc.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #2 — same fix (per-entry `zipEntryDate`, threaded to both the streaming and buffered write paths; module-level `zip:{date}` is a proven no-op), rebased onto `4294597` (the Print_Area reconcile fix) instead of directly on `master`.
Why superseded, not just an update to #2: `xlsx-for-ai-server` pins this fork by commit SHA, and its current pin (`4294597`) already includes the Print_Area reconcile fix. #2's commit (`cb42cbb`) branches from `8b2d62e`, one commit before `4294597` — pinning to it would silently drop that fix. Caught this via a real CI regression (2 failing Print_Area fidelity tests) after initially pinning `xlsx-for-ai-server` to `cb42cbb`.
Verified this session
4.4.0-protobi.11;npm pack --dry-runshows the expected 187-file tarball. Actualnpm publishis deliberately NOT done — held per the requesting handoff for the org's own SPM to authorize.Note on CI
actions/cache, a known repo-wide infra issue predating this branch, not a defect in this diff) — see the identical all-red status on XLS-352: pin every zip entry's mtime via an optional zipEntryDate option #2.Note on
--no-verifyAGENTS.mdrule 1 ("If the pre-commit hook fights you... commit with--no-verify... do NOT reshape unrelated code"), committed with--no-verifyrather than touching unrelated lines or.eslintrc/.prettierrc.#2 can be closed once this lands.
🤖 Generated with Claude Code
https://claude.ai/code/session_013Vedui93dYwzJgZVtn1P7c