feat: replace SheetJS with @jetstreamapp/simple-excel for spreadsheet import and export

[paustint]

GOALS:

• avoid "string too large" errors (errors reported regularly)
• stream workbooks where appropriate (reduce memory load on large worksheets)
• avoid non-npm dependencies

Excel files are now written and read by @jetstreamapp/simple-excel, a streaming, dependency-free engine built and verified against Excel, Google Sheets, Numbers and LibreOffice. Exports stream rows into a Blob, so memory no longer grows with row count and the large downloads that failed with RangeError now complete; cells over 32,767 characters are truncated and reported on every path, including background jobs and the Permission Manager export, which used to fail outright. Background Excel downloads show a live row count and can be cancelled while the file is being built.

Imports sniff the file first: password-protected workbooks, legacy .xls, .xlsb and .ods files get a message that says what to do instead of a half-working load, CSV bytes from Drive or the desktop open-with path go to the CSV parser, error cells read as blank, escaped control characters and CRLF survive, and a blank header cell in the middle of a multi-object template no longer shifts the following columns onto the wrong fields.

Worksheet names are sanitized and de-duplicated the way Excel requires, so two objects whose names collide within 31 characters no longer overwrite each other in the SObject Export workbook. Permission Manager header rows are bold. The xlsx package, its codepage table and the build-script special cases for it are removed.

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@​jetstreamapp/​simple-excel@​0.1.0

View full report

[Copilot]

Note

Copilot was unable to run its full agentic suite in this review.

Pull request overview

This PR migrates Excel read/write and file parsing away from SheetJS (xlsx) to @jetstreamapp/simple-excel, switching to streaming workbook generation and passing around raw bytes for Google Drive downloads, while improving user-facing error handling (e.g., password-protected workbooks).

Changes:

• Replace xlsx usage with @jetstreamapp/simple-excel across UI utilities, exports, and loaders (streaming writer + workbook reader).
• Update Google Drive selectors and relevant types to pass file bytes instead of parsed workbooks.
• Add tested, centralized file-parse error messaging and expand/adjust unit tests for new behavior.

Reviewed changes

Copilot reviewed 29 out of 31 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
scripts/replace-package-deps.mjs	Stops preserving xlsx dependency when swapping dependencies.
scripts/build-electron.mjs	Removes xlsx from dependency-removal list during Electron build.
package.json	Adds @jetstreamapp/simple-excel and removes xlsx tarball dependency.
libs/ui/src/lib/form/file-selector/GoogleFileSelectorExternalButton.tsx	Stops parsing XLSX client-side; forwards downloaded bytes to consumers.
libs/ui/src/lib/form/file-selector/GoogleFileSelector.tsx	Converts gapi binary strings to bytes; forwards unparsed bytes downstream.
libs/ui/src/lib/file-download-modal/tests/RecordDownloadModal.spec.tsx	Awaits async download path; adds Blob assertion for Excel writer output.
libs/ui/src/lib/file-download-modal/RecordDownloadModal.tsx	Awaits async Excel generation; updates Excel error handling behavior.
libs/ui/src/lib/file-download-modal/FileDownloadModal.tsx	Makes Excel generation async; expands supported data types to include Blob.
libs/types/src/lib/ui/types.ts	Changes InputReadGoogleSheet contract from workbook to { name, bytes }.
libs/shared/ui-utils/src/lib/shared-ui-utils.ts	Replaces XLSX utilities with simple-excel writer/reader + sniffing-based parse pipeline.
libs/shared/ui-utils/src/lib/load-multi-object-template.utils.ts	Adds streaming reader for multi-object template sheets via simple-excel.
libs/shared/ui-utils/src/lib/file-parse-error.utils.ts	Introduces centralized user-facing parse error messaging.
libs/shared/ui-utils/src/lib/tests/shared-ui-utils.spec.ts	Updates/adds tests for streaming Excel writer and workbook reader behavior.
libs/shared/ui-utils/src/lib/tests/prepare-load-multi-object-template.spec.ts	Updates round-trip test to use simple-excel reader.
libs/shared/ui-utils/src/lib/tests/file-parse-error.utils.spec.ts	Adds fixture-based tests for encrypted workbook and error-messaging rules.
libs/shared/ui-utils/src/index.ts	Exports the new file-parse error utility.
libs/shared/ui-core/src/jobs/Jobs.tsx	Surfaces truncated-cell warnings in job success messages/status.
libs/shared/ui-core/src/jobs/JobWorker.ts	Streams Excel generation with progress + cancellation; reports truncation counts.
libs/features/sobject-export/src/sobject-export-utils.ts	Uses sanitizeSheetName to prevent worksheet name collisions after truncation.
libs/features/sobject-export/src/tests/sobject-export-utils.spec.ts	Adds tests for worksheet name sanitization/de-duplication behavior.
libs/features/manage-permissions/src/utils/permission-manager-export-utils.ts	Rewrites permission export XLSX generation to streaming writer with merges/styles.
libs/features/manage-permissions/src/utils/tests/permission-manager-field-export.spec.ts	Reworks tests to validate generated XLSX (rows, styles, merges, truncation).
libs/features/manage-permissions/src/ManagePermissionsEditor.tsx	Awaits async workbook generation and wires truncation notifications.
libs/features/load-records/src/steps/SelectObjectAndFile.tsx	Uses parseFile and centralized parse error messaging for local/Drive files.
libs/features/load-records-multi-object/src/useProcessLoadFile.ts	Accepts workbook bytes, uses centralized parse errors + toast on failure.
libs/features/load-records-multi-object/src/load-records-multi-object-utils.ts	Parses template via streaming reader; improves dataset construction robustness.
libs/features/load-records-multi-object/src/tests/load-records-multi-object-utils.spec.ts	Writes real XLSX fixtures via writer; parses real template file; adds new edge-case tests.
libs/features/load-records-multi-object/src/LoadRecordsMultiObject.tsx	Removes XLSX parsing; feeds bytes into the new workbook parser.
apps/jetstream-desktop-client/src/app/components/core/useElectronActionLoader.tsx	Uses centralized file-parse error messaging for Electron open-with flows.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Unresolved issues remain in permission-export memory use, cancellation, warning propagation, and CSV byte detection.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

libs/shared/ui-core/src/jobs/JobWorker.ts:384

• Cancellation is only checked from onProgress. If a cancel message arrives after the last progress callback—or if this writer emits no progress for a small workbook—prepareExcelFile can resolve and this success response is posted, so the UI reports/downloads a job the user canceled. Re-check canceledJobIds after the awaited write and before constructing the success response.
  libs/shared/ui-core/src/jobs/Jobs.tsx:438
• This warning is lost on the Google Drive fallback path: handleGoogleUploadFailure overwrites newJob.statusMessage with its generic message (and persists that object when there is no token or the upload rejects). Thus a gdrive export containing truncated cells no longer tells the user that values were shortened, despite setting finished-warning here. Preserve/append truncationWarning when threading this job through the Google-upload failure handler.

libs/features/manage-permissions/src/utils/permission-manager-export-utils.ts:282

• This path still builds the full dataRows array and then getColumnWidths([...headerRows, ...dataRows], ...) creates a second converted copy of every cell before the writer starts. Large Permission Manager exports therefore retain roughly two worksheet-sized representations, undermining the memory-reduction goal and still risking browser exhaustion. Compute widths from a bounded sample (or while generating rows) and stream the full rows without retaining the complete arrays.

  return { headerRows, dataRows, merges, columns: getColumnWidths([...headerRows, ...dataRows], new Set([0])) };

libs/shared/ui-utils/src/lib/shared-ui-utils.ts:1379

• The byte path only sends data to PapaParse when sniff(head) returns text. Valid CSV/TSV inputs that are UTF-16 (an Excel/Drive export with an FF FE BOM) return unknown, and an empty CSV returns empty; both are then sent to parseWorkbook and reported as an unreadable XLSX. Use the filename/extension as a fallback for CSV/TSV and decode the BOM-selected encoding before parsing.

  const head = source instanceof Blob ? new Uint8Array(await source.slice(0, SNIFF_BYTES).arrayBuffer()) : source.subarray(0, SNIFF_BYTES);
  if (sniff(head) === 'text') {
    return parseCsvContent(await bytesToText(source), options);
  }
  return parseWorkbook(source, options);

scripts/build-electron.mjs:59

• @jetstreamapp/simple-excel is now imported by the renderer through @jetstream/shared/ui-utils, so Nx will include it in the desktop target dependencies. This cleanup list removes dependencies bundled into the renderer, but the replacement for the removed xlsx entry is missing; the package will remain installed and unnecessarily ship in the desktop artifact. Add @jetstreamapp/simple-excel to the removal list.

  return ['react', 'tslib', 'stripe'].filter((dep) => {

• Files reviewed: 29/31 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved findings cover OAuth redirects, development-port selection, email persistence, Google Drive export, cancellation cleanup, truncation messaging, and duplicate-download guards.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (3)

apps/api/src/app/controllers/oauth.controller.ts:95

• This always sends the OAuth result back to JETSTREAM_CLIENT_URL. If Vite selects a fallback development port because 4200 is occupied, the browser started OAuth on (for example) 4210 but the callback redirects to 4200, breaking the login flow for that worktree; preserve the originating client URL rather than hard-coding the configured origin.

    const clientUrl = new URL(ENV.JETSTREAM_CLIENT_URL).origin;
    const returnParams: OauthLinkParams = {
      type: 'salesforce',
      clientUrl,
    };

libs/api-config/src/lib/email.config.ts:71

• This fire-and-forget insert lets sendEmail resolve before the email_activity row is persisted. Cron callers can finish or call process.exit() before the row is written, so the Cloudflare cooldown query can miss a sent alert and send duplicates; await the insert while retaining the existing error handling.

    .catch((err) => logger.error({ message: err?.message }, '[EMAIL][ERROR] Error logging email activity'));

libs/shared/ui-core/src/jobs/JobWorker.ts:399

• A canceled workbook build is reported here, but its id is never removed from canceledJobIds. Every canceled background download therefore remains in the worker's Set for its lifetime, causing unbounded growth and stale cancellation state; delete the id after computing/reporting the cancellation error (or in a finally block).

          logger.error('Error in BulkDownload job:', ex);

• Files reviewed: 41/43 changed files
• Comments generated: 7
• Review effort level: Lite

[paustint]

Went through Copilot's suppressed (low-confidence) comments across both reviews — several were valid:

• libs/shared/ui-core/src/jobs/JobWorker.ts:384 — fixed, the cancel set is re-checked after the awaited write so a late cancel is not posted as a success.
• libs/shared/ui-core/src/jobs/JobWorker.ts:399 — fixed, the cancel flag is cleared in a finally once the download job has reported.
• libs/shared/ui-core/src/jobs/Jobs.tsx:438 — fixed, the truncation sentence is threaded through the Drive upload, its failure fallback and the no-token local save.
• libs/features/manage-permissions/src/utils/permission-manager-export-utils.ts:282 — fixed, widths come from the label header row plus a 500-row sample instead of a second copy of every cell.
• libs/shared/ui-utils/src/lib/shared-ui-utils.ts:1379 — fixed, byte inputs decode by BOM (UTF-16) and fall back to the csv/tsv extension before going to the workbook reader.
• scripts/build-electron.mjs:59 — fixed, @jetstreamapp/simple-excel is stripped from the packaged desktop app.

oauth.controller.ts:95 and email.config.ts:71 were artifacts of the bad squash (reverting merged PRs) and are gone after rebuilding the commit on main.

[Copilot]

🟡 Changes recommended

Four unresolved moderate findings remain around Drive parse warnings and async export cancellation and concurrency.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (2)

libs/features/load-records/src/steps/SelectObjectAndFile.tsx:164

• This Drive path drops the errors returned by parseFile, unlike the local path immediately above. A malformed CSV downloaded from Drive can therefore be accepted without the warning that the same file gets locally; preserve these errors and apply the same warning/suppression logic here (or share that helper).

      const { data: rawData, headers } = await parseFile(bytes, { onParsedMultipleWorkbooks, extension: getExtensionFromName(name) });
      const { data, removedCount } = removeEmptyRows(rawData);

libs/ui/src/lib/file-download-modal/RecordDownloadModal.tsx:599

• The async workbook build can now outlive this modal: Cancel/onClose remains available while handleDownload is awaiting prepareExcelFile, but no cancellation or closed guard is set. A user can cancel and still get a later saveFile/Google upload (and the handler can continue after unmount); coordinate this button and modal close path with an abort or completion guard.

              <button
                className="slds-button slds-button_brand"
                onClick={handleDownload}
                disabled={invalidConfig || isLoadingChildRelationships || isPreparingFile}

• Files reviewed: 29/31 changed files
• Comments generated: 2
• Review effort level: Lite

[paustint]

Went through Copilot's suppressed (low-confidence) comments from the latest review — both were valid:

• libs/features/load-records/src/steps/SelectObjectAndFile.tsx:164 — fixed, the Drive path now surfaces parseFile errors through the same warning helper as a local file.
• libs/ui/src/lib/file-download-modal/RecordDownloadModal.tsx:599 — fixed, Cancel/close abort the in-flight workbook build via an AbortSignal and an aborted build never saves (covered by a new spec).

[Copilot]

🟡 Changes recommended

Eight moderate review findings remain unresolved across parsing, async state handling, progress reporting, and export memory usage.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (7)

libs/features/load-records-multi-object/src/load-records-multi-object-utils.ts:370

• This fallback only adds a data error and leaves sobject undefined, but validateObjectData uses the absence of an sobject error to call describeSObject(org, dataset.sobject). For an unreadable sheet that reaches validation, describeSObject receives undefined (and its startsWith access throws), producing an extra misleading The object "undefined" could not be loaded... error instead of preserving the single worksheet-read failure. Make validation skip metadata lookup when the fallback has no object value, or mark this dataset as unreadable before validation.

/** Stand-in for a worksheet that could not be read at all, so the rest of the workbook is still usable */
function buildUnreadableDataset(sheetName: string): LoadMultiObjectData {
  const dataset: Partial<LoadMultiObjectData> = {
    worksheet: sheetName,
    data: [],
    dataById: {},
    headers: [],
    referenceHeaders: new Set(),
    errors: [
      {
        property: 'data',
        worksheet: sheetName,
        location: WORKSHEET_LOCATIONS.dataStartCell,
        locationType: 'CELL',
        message: `Jetstream could not read the data on this worksheet. Check that headers are on row 5, data starts on row 6, and the sheet matches the template layout.`,
      },
    ],

libs/shared/ui-core/src/jobs/JobWorker.ts:309

• progress.current is the number of rows emitted by the writer, including header rows and (for subqueries) rows from additional sheets, but Job.tsx renders current of total whenever total > 0. Using downloadedRecords.length therefore shows impossible values such as 1001 of 1000 and is misleading for multi-sheet exports; keep the total indeterminate until the writer reports a matching workbook-wide total.

                    total: downloadedRecords.length,

libs/shared/ui-utils/src/lib/shared-ui-utils.ts:1375

• The .json extension check happens before the byte sniffer, so any byte-backed file named .json is forced through parseJson even when its bytes are an XLSX, encrypted workbook, or legacy format. That bypasses the new format detection and produces a generic JSON error instead of the intended workbook-specific guidance (the code already handles a CSV/XLSX mismatch by sniffing). Keep the string-JSON fast path, but sniff byte inputs before deciding whether to call parseJson.

  options = options || {};
  if (options.extension === INPUT_ACCEPT_FILETYPES.JSON) {
    // FileSelector reads .json as text, but the signature permits bytes, so decode rather than assume
    return parseJson(isString(content) ? content : await bytesToText(content));
  }

libs/shared/ui-utils/src/lib/shared-ui-utils.ts:1395

• This branch now throws XlsxError for encrypted/legacy/non-XLSX byte inputs, but CreateFieldsImportExport.handleImport still invokes parseFile without a try/catch (and passes it as an async onReadFile callback with no onError). Selecting one of those files in the Create Fields import therefore produces an unhandled rejection instead of the promised actionable toast; update that caller (and any similar parse entrypoint) to use getFileParseErrorMessage.

  return parseWorkbook(source, options);

libs/ui/src/lib/file-download-modal/FileFauxDownloadModal.tsx:174

• This new catch silently discards any rejection from the async onDownload callback. For example, handleManifestDownload awaits getPackageXml; if that request fails, the modal has already closed and this catch provides no toast or callback to report the failed download. Propagate the rejection or add an explicit error callback instead of leaving async failures invisible.

    } catch {
      // TODO: show error message somewhere

libs/ui/src/lib/form/file-selector/GoogleFileSelector.tsx:109

• Parsing is now delegated to the consumer, and LoadRecordsSelectObjectAndFile.handleGoogleFile is async, but this callback is still invoked without awaiting its Promise. The selector therefore clears loading immediately after the bytes are handed off, allowing a refresh or second picker selection while the previous workbook is still being parsed; the two async parses can race and overwrite the selected data. Allow onReadFile to return a promise and await it before clearing the loading state.

        // The gapi client hands back the response body as a binary string (one byte per char code) rather than
        // bytes, so it is converted once here and every consumer downstream works with the file's actual bytes.
        onReadFile({
          name: selectedItem.name || '',
          bytes: binaryStringToBytes(resultBody).buffer as ArrayBuffer,
          selectedFile: selectedItem,
        });

libs/ui/src/lib/form/file-selector/GoogleFileSelectorExternalButton.tsx:132

• This path has the same race after parsing moved out of the selector: onReadFile can start an async consumer parse, but it is not awaited before setDownloading(false) in finally. That re-enables the external picker while the previous parse is still running, so selecting another file can leave results applied out of order. Make the callback promise-aware and await it before ending the download/loading state.

        // The file is handed on unparsed - whoever receives it decides how to read it and reports its own errors
        setSelectedFile(syntheticDoc);
        setManagedFilename(fileName);
        callbackRefs.current.onReadFile && callbackRefs.current.onReadFile({ name: fileName, bytes, selectedFile: syntheticDoc });

• Files reviewed: 30/32 changed files
• Comments generated: 1
• Review effort level: Lite

[jetstream-bot-agent]

Went through Copilot's suppressed (low-confidence) comments — several were valid:

• libs/shared/ui-core/src/jobs/JobWorker.ts:307 — fixed, the streaming progress tick reported per-sheet rows against the record total (showing e.g. "1,001 of 1,000"); it now reports the running row count alone.
• libs/features/load-records-multi-object/src/load-records-multi-object-utils.ts:428 — fixed, an unreadable worksheet's fallback dataset was still validated, which called describeSObject(undefined) and added two misleading extra errors; unreadable sheets now skip validation (spec added).
• libs/features/create-object-and-fields/src/CreateFieldsImportExport.tsx:58 — fixed, the one remaining parseFile caller without a try/catch now shows the same actionable parse-error toast as the others.
• libs/ui/src/lib/file-download-modal/FileFauxDownloadModal.tsx:174 — fixed, the empty catch now logs and toasts, since the awaited onDownload can reject after the modal has closed.
• libs/ui/src/lib/form/file-selector/GoogleFileSelector.tsx:105 and GoogleFileSelectorExternalButton.tsx:132 — fixed, onReadFile is awaited so the selector stays busy until the consumer has finished parsing the file.

The rest were already implemented (post-write cancel re-check, canceledJobIds cleanup, truncation warning on the Drive fallback, UTF-16/empty CSV sniffing, build-electron removal list, Drive-path parse errors, RecordDownloadModal abort) or didn't apply (oauth.controller.ts / email.config.ts were bad-squash artifacts and are not in this PR; bytes never arrive tagged .json, and a file the user named .json should parse as JSON regardless).

[Copilot]

🟡 Changes recommended

Unresolved issues affect unreadable-sheet previews, failed Drive selection state, permission export memory usage, and truncation warning propagation.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (3)

libs/features/load-records-multi-object/src/load-records-multi-object-utils.ts:370

• This fallback is still rendered by the worksheet preview/config UI, which reads dataset.sobject and dataset.operation as populated values. Leaving both fields absent produces an undefined • undefined summary and an unusable operation selector for every unreadable sheet; either model the unreadable state explicitly and hide that editor, or provide a dedicated non-editable placeholder for it.

function buildUnreadableDataset(sheetName: string): LoadMultiObjectData {
  const dataset: Partial<LoadMultiObjectData> = {
    worksheet: sheetName,
    data: [],
    dataById: {},
    headers: [],
    referenceHeaders: new Set(),
    errors: [

libs/features/manage-permissions/src/utils/permission-manager-export-utils.ts:44

• Although the workbook writer streams rows after this point, each worksheet generator still appends every exported row to dataRows before writeWorksheet starts. Large permission tables therefore retain the full cell matrix in memory and can still exhaust the browser heap; make dataRows an iterable/generator and materialize only the row currently being written.

 * A sheet assembled in memory before it is streamed into the workbook.
 *
 * The writer has no worksheet object to decorate after the fact - rows, merges and column widths all have to be
 * known by the time the sheet is opened - so each `generate*Worksheet` returns this and `writeWorksheet` does the
 * writing for all four sheets.

libs/shared/ui-core/src/jobs/Jobs.tsx:353

• This warning is preserved for the non-bulk Google path below, but the bulk-API Google path calls uploadToGoogleDrive without passing warning: truncationWarning. If that upload fails, handleGoogleUploadFailure overwrites the status with the generic local-fallback message, so the user is not told that the fallback file contains truncated cells. Pass the warning through that bulk upload call as well.

            const truncationWarning = truncatedCells
              ? ` ${formatNumber(truncatedCells)} ${pluralizeFromNumber('value', truncatedCells)} exceeded Excel's cell limit and ${
                  truncatedCells === 1 ? 'was' : 'were'
                } truncated.`
              : '';

• Files reviewed: 34/36 changed files
• Comments generated: 2
• Review effort level: Lite

[paustint]

Went through Copilot's suppressed (low-confidence) comments from every review on this PR. One was valid:

• libs/features/load-records-multi-object/src/review/LoadRecordsMultiObjectSheetConfig.tsx:56 — fixed. A worksheet with no object/operation (the new unreadable-sheet fallback, or a blank B1/B2, which already behaved this way on main) rendered as undefined • undefined; it now shows No object • No operation next to the error banner that explains why.

One is worth recording as wrong, because following it would break the desktop build:

• scripts/build-electron.mjs:59 — Copilot asked for @jetstreamapp/simple-excel to be added to the dependency-removal list "like the removed xlsx entry". It must not be. libs/shared/utils imports sanitizeSheetName from it, and the desktop main process imports libs/shared/utils, so the esbuild output emits a real require("@jetstreamapp/simple-excel") (verified in dist/apps/jetstream-desktop/main.js). xlsx was renderer-only, which is why it was safe to strip.

The rest were already handled:

• JobWorker.ts (cancel re-check after the awaited write, canceledJobIds cleanup, progress total) — all three are in the current code (post-write cancel check, finally { canceledJobIds.delete(...) }, total: 0 while streaming).
• Jobs.tsx:438 — uploadToGoogleDrive/handleGoogleUploadFailure already thread warning through.
• Jobs.tsx:353 — doesn't apply. The bulk-API path returns from the worker before any workbook is written, so truncatedCells is never present and the uploaded file is Salesforce's CSV.
• permission-manager-export-utils.ts:44/:282 — the double copy is gone; widths come from getExcelColumnWidths(headerRow, dataRows), which samples a bounded number of rows. The per-sheet rows are still materialized, but they are a conversion of table data the grid already holds in memory.
• shared-ui-utils.ts:1379 — UTF-16 and empty CSVs are handled: textEncodingFromBom picks the encoding and a .csv/.tsv/.txt extension routes non-zip bytes to PapaParse.
• shared-ui-utils.ts:1395 — CreateFieldsImportExport.handleImport already wraps parseFile and toasts getFileParseErrorMessage.
• shared-ui-utils.ts:1375 — leaving the .json fast path ahead of the sniffer is deliberate; the extension is the user's declared intent and only a deliberately misnamed file is affected.
• FileFauxDownloadModal.tsx:174 — the catch logs and toasts now, with isPreparingFile guarding re-entry.
• RecordDownloadModal.tsx:599 — guarded by isPreparingFile plus an AbortController aborted on modal close.
• SelectObjectAndFile.tsx:164 — both paths share notifyParseErrors, so Drive files get the same warnings as local ones.
• load-records-multi-object-utils.ts:370 (describeSObject) — unreadable sheets are filtered out of validateObjectData, so describeSObject is never called with undefined.
• GoogleFileSelector.tsx:109 / GoogleFileSelectorExternalButton.tsx:132 (not awaiting onReadFile) — already awaited; the ordering half of this became the two review threads and is fixed there.
• oauth.controller.ts:95 and email.config.ts:71 came from a review that saw a 43-file diff including commits already merged to main; neither file is part of this PR.

[Copilot]

🟡 Changes recommended

Three moderate issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (2)

libs/features/manage-permissions/src/utils/permission-manager-export-utils.ts:138

• The field worksheet contains Date values, and the shared prepareExcelFile explicitly uses dates: 'local' for Excel serial conversion. This custom writer omits that option, so non-UTC clients can get audit timestamps shifted to the engine's default timezone even though the headers and CSV export promise local time. Pass dates: 'local' here as well.

    cellOverflow: 'truncate',
    // Every permission cell is TRUE or FALSE and every object name repeats across sheets, so the bounded shared
    // string table is worth its memory here - this is what `bookSST: true` used to give us
    strings: 'auto',
  });

libs/shared/ui-utils/src/lib/shared-ui-utils.ts:1447

• openWorkbookForImport selects errors: 'null', but these rows are returned without normalizing null/undefined cells. Unlike the previous sheet_to_json(..., defval: '') path, a formula-error or blank cell can therefore reach the standard load-records field mapping as null instead of a blank string. Map row values with value ?? '' before returning, as the multi-object reader already does.

    const { rows, headers } = await workbook.sheet(selectedSheet).toObjects({ dropEmptyHeaders: true });
    return { data: rows, headers, errors: [] };

• Files reviewed: 35/37 changed files
• Comments generated: 1
• Review effort level: Lite

[paustint]

Checked the two new suppressed comments from the latest review — both are wrong, and I verified each against the engine rather than the docs:

• permission-manager-export-utils.ts:138 — dates: 'local' is already the writer default (dates: e.dates ?? "local" in the shipped bundle, and the WriteOptions doc says so too), so passing it explicitly is a no-op. prepareExcelFile spells it out because it also sets truncationSuffix; the permission writer needs neither, since EXCEL_TRUNCATION_SUFFIX is '...(truncated)' — the library default as well.
• shared-ui-utils.ts:1447 — toObjects already normalizes. Its defval defaults to '' and the object-row assembler does row[column] ?? defval, so a blank cell or an errors: 'null' formula-error cell comes back as '', never null. Round-tripped a sheet with null/undefined cells through the writer and reader to confirm: [{"a":"x","b":"","c":"z"},{"a":"q","b":"","c":""}]. The multi-object reader needs its explicit ?? '' because it reads in array mode via sheet.rows(...), where defval does not apply — the two readers use different APIs, so that comparison runs the other way.

Everything else in the suppressed blocks was covered in my earlier comment.

[Copilot]

🟡 Changes recommended

Two unresolved moderate issues affect invalid Google workbook state handling and successful Google upload completion.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (1)

libs/ui/src/lib/file-download-modal/FileDownloadModal.tsx:307

• After the upload job is emitted, handleUploadToGoogle closes the modal; the new unmount cleanup aborts this controller, so this check is true for every successful Google upload. handleDownload therefore returns before saveFileFormatToStorage and trackEvent run. Move the close until after those completion steps, or otherwise distinguish an upload that has already been queued from a cancelled build.

    if (excelOptions.signal.aborted) {
      return;

• Files reviewed: 36/38 changed files
• Comments generated: 1
• Review effort level: Lite

[paustint]

Checked the new suppressed comment — invalid, and I added a test so it stays that way.

• FileDownloadModal.tsx:307 — the claim is that the unmount cleanup aborts the controller before saveFileFormatToStorage/trackEvent run, so handleDownload bails on every successful Google upload. It does not. onModalClose() schedules a React state update that unmounts on a later macrotask, while handleDownload's continuation after await handleUploadToGoogle(...) runs as a microtask — it gets there first. Rendered the modal under a host that really unmounts it on close and confirmed: the job is emitted once, trackEvent fires with fileFormat: gdrive, and RECENT_FILE_FORMAT_FileDownloadModal is set to gdrive.

That abort check is still doing real work, just not for this case: a user who cancels mid-build makes handleUploadToGoogle return early without emitting or closing, and the check is what stops the completion steps from running for a download that never happened. Since the ordering is subtle enough to be worth pinning, this is now covered by FileDownloadModal.spec.tsx.

[Copilot]

Note

Copilot was unable to run its full agentic suite in this review.

Pull request overview

Copilot reviewed 37 out of 39 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

[Copilot]

🔵 Needs a closer look

Unresolved issues remain with warning accessibility, workbook format fallback, and failed-export side effects.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (3)

libs/shared/ui-core/src/jobs/Jobs.tsx:360

• finished-warning is now used for exports with truncated cells, but Job.tsx renders that status with the accessible description job success (lines 120–127). Screen-reader users therefore hear a lossy export announced as successful without any warning; add a warning-specific accessible description/icon treatment for this status.

              status: truncationWarning ? 'finished-warning' : 'success',
              statusMessage: `Records downloaded successfully.${truncationWarning}`,

libs/shared/ui-utils/src/lib/shared-ui-utils.ts:1378

• The extension fallback is applied to every non-zip classification. A renamed/encrypted or legacy CFB workbook sniffed as cfb-encrypted/cfb-legacy with a .csv extension is therefore decoded as CSV and can return a misleading CSV result instead of the format-specific XlsxError message promised by the workbook opener. Restrict this fallback to empty/unknown kinds so known binary formats take precedence.

    kind === 'text' || encoding !== 'utf-8' || (kind !== 'zip' && TEXT_EXTENSIONS.has(normalizeExtension(options.extension)));

libs/ui/src/lib/file-download-modal/FileFauxDownloadModal.tsx:172

• Awaiting this callback treats a caught export failure as success. ManagePermissionsEditor.handleExport catches workbook-generation errors, shows a toast, and resolves normally, so this modal still tracks file_download and persists the selected format even though no file was saved. Make the callback rethrow after reporting, or return an explicit success result and gate these side effects on it.

      await onDownload({ fileName, fileFormat: _fileFormat, mimeType, uploadToGoogle, googleFolder });

• Files reviewed: 37/39 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Two moderate findings remain unresolved, along with a Permission Manager warning-message mismatch.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (3)

libs/features/manage-permissions/src/ManagePermissionsEditor.tsx:524

• Passing the generic truncation notifier here makes the warning tell Permission Manager users to download as CSV or JSON, but this modal only offers CSV, XLSX, and Google Drive (and its CSV export is a ZIP); JSON is not an available alternative. Use a Permission Manager-specific message or make the notifier accept the caller's available formats.

            // Any value past Excel's per-cell character limit is truncated by the writer, which the user needs to know about
            { onCellsTruncated: notifyExcelCellsTruncated },

libs/shared/ui-utils/src/lib/load-multi-object-template.utils.ts:398

• rows() omits completely blank rows unless blankRows: true (as the writer test has to request explicitly), but buildDataset later treats each compact dataRows index as the physical Excel row via getExcelRow. If a user leaves an empty row between records, duplicate/reference/graph errors for every later record point to the wrong worksheet row. Preserve the source row number while streaming (or request blank rows and skip them without losing their index).

      for await (const row of sheet.rows({ startRow: headerRow })) {

libs/ui/src/lib/file-download-modal/FileDownloadModal.tsx:216

• handleUploadToGoogle calls onModalClose() after queuing the job, which unmounts this component and runs the abort cleanup; therefore this signal is already aborted when control returns here, so the function returns before saveFileFormatToStorage and trackEvent. The new Google-upload test expects those completion side effects, so return an explicit “upload queued” result from handleUploadToGoogle (with false for cancellation) instead of using the controller’s signal after the modal closes.

        await handleUploadToGoogle(excelOptions);
        if (abortController.signal.aborted) {
          return;
        }

• Files reviewed: 38/40 changed files
• Comments generated: 0 new
• Review effort level: Lite

[paustint]

Went through Copilot's suppressed (low-confidence) comments from the latest review — one was valid:

• libs/ui/src/lib/file-download-modal/FileDownloadModal.tsx:216 — fixed in the next push. Queueing a Google upload closes the modal, which unmounts it and aborts the signal, so the abort check right after skipped saveFileFormatToStorage/trackEvent. It only bit the csv-backed upload (prepared synchronously, so the unmount lands first) — the existing test covered the xlsx one, which is why it passed. handleUploadToGoogle now returns whether the upload was queued, and the test runs both sources.

The rest didn't apply — notably scripts/build-electron.mjs:59: @jetstreamapp/simple-excel must stay out of the removal list, since the desktop main bundle requires it at runtime through @jetstream/shared/utils.

[Copilot]

🟡 Changes recommended

The template reader mishandles error cells, permission exports omit the large-file row-count hint, and one new test does not verify its format-specific cases.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

• Files reviewed: 38/40 changed files
• Comments generated: 3
• Review effort level: Lite