fix(spreadsheet-preview): open openpyxl workbooks that have a comment, table or chart

The XLSX preview hands an admitted copy of the workbook to ExcelJS 4.4.0,
which links each worksheet to its comments, comment notes (VML), tables and
drawing by looking the relationship Target up verbatim in maps keyed by its
own spelling, then dereferences the result unchecked. openpyxl, the library
most agents write .xlsx files with, uses absolute Targets
(/xl/tables/table1.xml), its own comment part name (xl/comments/comment1.xml,
which ExcelJS never loads) and a drawing XML ExcelJS parses to nothing. One
comment, table or chart was enough for "Spreadsheet preview failed".

repairExcelJsModel() (spreadsheet-xlsx-core.js) runs on ExcelJS's parsed
model right before its reconcile: it re-points a link to the key ExcelJS
stored for the same part (so an openpyxl table is kept) and drops a link,
tablePart, drawing or hyperlink reference that would dereference nothing.
It adds nothing and resolves keys as own properties only. The worker wraps
workbook.xlsx.reconcile per instance to call it, and throws if a future
ExcelJS renames that method. SPREADSHEET_ASSET_VERSION is refreshed.

Tests: unit tests on model objects, and three real openpyxl 3.1.5
workbooks in test/fixtures/xlsx that fail the worker tests without the hook.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/web/public/spreadsheet-preview-worker.js

@@ -171,6 +171,17 @@ async function loadWorkbook(bytes) {
   // defineProperty throws if a future ExcelJS renames `_definedNames`, rather
   // than silently expanding again.
   Object.defineProperty(nextWorkbook._definedNames, 'model', { configurable: true, get: () => [], set: () => {} });
+  // ExcelJS follows a worksheet's comment, VML, table and drawing links
+  // unchecked, and openpyxl spells them so the lookup finds nothing: one comment,
+  // table or chart failed the whole preview. `load()` calls `this.reconcile()`
+  // once on the parsed model; repair the model first (core.repairExcelJsModel).
+  // Throws if a future ExcelJS renames reconcile, rather than failing again.
+  const reconcile = nextWorkbook.xlsx.reconcile;
+  if (typeof reconcile !== 'function') throw new Error('ExcelJS reconcile is unavailable');
+  nextWorkbook.xlsx.reconcile = function reconcileRepaired(model, options) {
+    core.repairExcelJsModel(model);
+    return reconcile.call(this, model, options);
+  };
   // ExcelJS expands every address of a `<dataValidation sqref>` into its own
   // object (a whole-column dropdown is a million), and the preview never shows
   // validations, so they are not parsed at all. `maxRows` is a per-sheet

src/web/public/spreadsheet-preview.js

@@ -20,7 +20,7 @@
 (function initSpreadsheetPreview(global) {
   'use strict';
 
-  const SPREADSHEET_ASSET_VERSION = '911680fac09d';
+  const SPREADSHEET_ASSET_VERSION = '0833a414e785';
   const MAX_PREVIEW_BYTES = 10 * 1024 * 1024;
   const DEFAULT_TIMEOUT_MS = 20000;
   const MAX_SCROLL_PX = 8000000;

src/web/public/spreadsheet-xlsx-core.js

@@ -472,6 +472,102 @@
     return zipApi.zipSync(admission.entries, { level: 0 });
   }
 
+  // ExcelJS (4.4.0, lib/xlsx/xlsx.js + xform/sheet/worksheet-xform.js `reconcile`)
+  // links a worksheet to its comments, comment notes (VML), tables and drawing by
+  // looking the relationship Target up VERBATIM in maps keyed by its OWN spelling
+  // (`../comments1.xml`, `../drawings/vmlDrawing1.vml`, `../tables/table1.xml`,
+  // `drawing1`), then dereferences the result unchecked. openpyxl, which most
+  // agents write spreadsheets with, uses absolute Targets, its own part names
+  // (`/xl/comments/comment1.xml`) and a drawing XML ExcelJS parses to nothing, so
+  // a workbook with ONE comment, table or chart threw ("reading 'comments'",
+  // "'name'", "'anchors'") before a single cell was shown.
+  //
+  // This runs on ExcelJS's parsed model right before that reconcile. It only
+  // re-points a link to the key ExcelJS itself stored for the same part, or drops
+  // a link (or a drawing) that would dereference nothing; nothing is added. The
+  // preview shows none of these parts. Lookups are own-property only, so a Target
+  // or rId such as `__proto__` cannot resolve through Object.prototype.
+  const REL_TYPE = 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/';
+  const EXCELJS_DRAWING_TARGET = /\/drawings\/([a-zA-Z0-9]+)[.][a-zA-Z]{3,4}$/;
+
+  function ownValue(map, key) {
+    return map && typeof key === 'string' && Object.prototype.hasOwnProperty.call(map, key) ? map[key] : undefined;
+  }
+
+  // The key ExcelJS gives a part it loaded from `xl/<path>` is `../<path>`
+  // (relative to xl/worksheets/). A Target that names the same part another way
+  // (`/xl/tables/table1.xml`) is rewritten to that key.
+  function worksheetRelativeTarget(target) {
+    const path = excelJsEntryName(target[0] === '/' ? target : `xl/worksheets/${target}`);
+    return path.startsWith('xl/') ? `../${path.slice(3)}` : null;
+  }
+
+  function resolveTarget(rel, map, usable) {
+    if (typeof rel.Target !== 'string') return false;
+    if (usable(ownValue(map, rel.Target))) return true;
+    const key = worksheetRelativeTarget(rel.Target);
+    if (key && usable(ownValue(map, key))) {
+      rel.Target = key;
+      return true;
+    }
+    return false;
+  }
+
+  function repairExcelJsModel(model) {
+    if (!model || typeof model !== 'object') return model;
+    const isObject = (value) => Boolean(value) && typeof value === 'object';
+    const drawings = isObject(model.drawings) ? model.drawings : {};
+    // ExcelJS reconciles every drawing that has rels and reads `.anchors` off it.
+    for (const name of Object.keys(drawings)) {
+      if (!isObject(drawings[name]) || !Array.isArray(drawings[name].anchors)) {
+        delete drawings[name];
+        if (isObject(model.drawingRels)) delete model.drawingRels[name];
+      }
+    }
+    // ExcelJS also reconciles every table it parsed, so one it parsed to nothing goes.
+    if (isObject(model.tables)) {
+      for (const key of Object.keys(model.tables)) if (!isObject(model.tables[key])) delete model.tables[key];
+    }
+    const worksheetRels = isObject(model.worksheetRels) ? model.worksheetRels : {};
+    for (const worksheet of Array.isArray(model.worksheets) ? model.worksheets : []) {
+      if (!isObject(worksheet)) continue;
+      const list = ownValue(worksheetRels, String(worksheet.sheetNo));
+      const rels = Array.isArray(list) ? list : [];
+      const kept = rels.filter((rel) => {
+        if (!isObject(rel)) return false;
+        if (rel.Type === `${REL_TYPE}comments`) return resolveTarget(rel, model.comments, isObject);
+        if (rel.Type === `${REL_TYPE}vmlDrawing`) {
+          return resolveTarget(rel, model.vmlDrawings, (vml) => isObject(vml) && Array.isArray(vml.comments));
+        }
+        if (rel.Type === `${REL_TYPE}table`) return resolveTarget(rel, model.tables, isObject);
+        return true;
+      });
+      if (Array.isArray(list)) worksheetRels[String(worksheet.sheetNo)] = kept;
+      const byId = new Map(kept.map((rel) => [rel.Id, rel]));
+      // `<tablePart r:id>`, `<drawing r:id>` and `<hyperlink r:id>` are followed
+      // through the rels unchecked, so each must name a rel that is still there.
+      if (Array.isArray(worksheet.tables)) {
+        worksheet.tables = worksheet.tables.filter((part) => {
+          const rel = isObject(part) ? byId.get(part.rId) : undefined;
+          return Boolean(rel) && rel.Type === `${REL_TYPE}table`;
+        });
+      }
+      if (isObject(worksheet.drawing)) {
+        const rel = byId.get(worksheet.drawing.rId);
+        const match = rel && typeof rel.Target === 'string' ? rel.Target.match(EXCELJS_DRAWING_TARGET) : null;
+        if (!rel || typeof rel.Target !== 'string' || (match && !ownValue(drawings, match[1]))) {
+          worksheet.drawing = undefined;
+        }
+      }
+      if (Array.isArray(worksheet.hyperlinks)) {
+        worksheet.hyperlinks = worksheet.hyperlinks.filter(
+          (link) => isObject(link) && (!link.rId || byId.has(link.rId))
+        );
+      }
+    }
+    return model;
+  }
+
   function parseCellRef(ref) {
     const match = /^\$?([A-Z]{1,3})\$?([1-9]\d*)$/i.exec(String(ref || ''));
     if (!match) return null;
@@ -983,6 +1079,7 @@
     createXmlCounter,
     admitXlsx,
     buildAdmittedArchive,
+    repairExcelJsModel,
     excelJsEntryName,
     parseCellRef,
     parseRange,