Skip to content

fix(rest): state the exceljs Buffer-shim assertion once, in a typed test loader - #13425

Merged
os-trump merged 2 commits into
mainfrom
claude/issue-13378-exceljs-typed-loader
Aug 30, 2026
Merged

fix(rest): state the exceljs Buffer-shim assertion once, in a typed test loader#13425
os-trump merged 2 commits into
mainfrom
claude/issue-13378-exceljs-typed-loader

Conversation

@os-trump

Copy link
Copy Markdown
Collaborator

Fixes #13378

exceljs@4.4.0's index.d.ts opens, at line 1, with declare interface Buffer extends ArrayBuffer { }. That file carries 106 top-level export declarations, so it is a module — which makes this Buffer module-local, and it therefore shadows Node's global Buffer inside every exceljs signature, including the Xlsx.load at index.d.ts:1490. A Node Buffer is a Uint8Array, so no Node Buffer value can satisfy that parameter. The defect is in the published declaration, not at any call site.

packages/rest was paying it at 6 anonymous as any sites and leaving a 7th as a ledgered TS2345.

STEP ONE — option C (upgrade upstream) was checked first, and it does not exist

Measured 2026-08-30 against the npm registry, zero repo changes:

reading value
dist-tags.latest 4.4.0 (published 2023-10-19) — the pinned version already IS the latest line
newest publish of any kind, across all 166 versions 4.4.1-prerelease.0 (2024-12-20)
4.4.1-prerelease.0 index.d.ts line 1 declare interface Buffer extends ArrayBuffer { }identical
its top-level export count 106 — identical, so still a module, so still module-local
its load signature index.d.ts:1490, load(buffer: Buffer, options?: Partial(XlsxReadOptions)): Promise(Workbook) — identical line, identical text
registry time.modified 2024-12-20 — nothing published since
control, one line back: 4.3.0 line 1 carries the shim too, so this is not a 4.4.0 regression a bump could undo

There is no later exceljs line to pin to. C is measured and rejected. Option A was implemented, as ruled.

What this does — option A

New packages/rest/src/xlsx-test-loader.ts: one loadXlsxWorkbook(bytes) that performs the assertion once and hands back a Workbook. The assertion targets the parameter type read off the dependency's own signature — Parameters(Xlsx['load'])[0] — rather than a hand-spelled any, so if exceljs ever drops the shim the alias resolves to Node's Buffer and the assertion becomes a no-op instead of a lie. The file's header states the whole defect, quotes the declaration and the tsc error, and records the option-C reading above, so the next reader does not read it as laziness.

All 7 test sites migrated — retiring the 6 as any is the point of the shape, not a bonus:

file sites was
src/export-integration.test.ts 5 wb.xlsx.load(getBuffer() as any)
src/export-business-timezone.test.ts 1 wb.xlsx.load(getBuffer() as any)
src/rest.test.ts 1 wb.xlsx.load(buf) — the ledgered TS2345

All three files also drop their now-unused import ExcelJS from 'exceljs'; the dependency is named in exactly one test-layer file now. grep -rn "xlsx.load(" --include=*.ts packages/rest/src afterwards reports 2: the helper, and src/import-prepare.ts:143.

src/import-prepare.ts:143 — deliberately NOT changed

It is production source, not a test, and it is not broken: it types its dynamic import as any (const ExcelJS: any = (await import('exceljs')).default ?? …) precisely so CSV/JSON imports do not pay for exceljs, the same pattern src/rest-server.ts:612 uses. Three reasons it stays:

  1. a helper that lives in the test layer cannot serve production source — importing it from src/import-prepare.ts would be a real defect, not a repair;
  2. nothing there is red or ledgered — there is no measurement to move;
  3. typing that dynamic import properly is a design change (its signature takes Buffer | ArrayBuffer, so which arm the assertion belongs on is a decision), and it drags exceljs's types into a production module that deliberately keeps them out. That is a bigger change than this card carries.

Ledger

Regenerated with the repo's own tool (pnpm --filter @objectstack/rest gen:test-typecheck-debt), never by hand:

"src/meta-public-book-grant.test.ts": 1,   (unchanged)
"src/rest-batch-size-cap.test.ts":    1,   (unchanged)
"src/rest.test.ts":                   3 -> 2

src/rest.test.ts does not reach zero, so its entry is re-recorded rather than deleted — measured, not assumed. The 2 that remain are the IHttpRequest pair, and #13377 is the card that holds them; they are untouched here. The authored _note is updated to point at that card and to record that the exceljs entry is gone; it stays card pointers, not readings, and a re-run of the generator confirmed it survives verbatim.

⚠️ Note for #13377, whose sites are in this same file: this diff removes one net line above them, so rest.test.ts(2064,7) / (2089,7) are now at (2063,7) / (2088,7).

Verification

pnpm --filter @objectstack/rest typecheck
  check:test-typecheck: OK — @objectstack/rest's test layer compiles under
  packages/rest/tsconfig.test.json; 3 file(s) / 4 error(s) held in
  test-typecheck-debt.json (shrink-only, .../issues/5286).            [exit 0]

pnpm --filter @objectstack/rest exec vitest run --maxWorkers=2
  Test Files  159 passed (159)
  Tests  2671 passed (2671)                                          [exit 0]

Behaviour is unmoved, as a typing-only change requires: the same Node Buffer reaches exceljs, which has always accepted it — that is what all 6 as any sites were already doing.

Anti-vacuity — the edited files really are inside the checked programs

tsc -p tsconfig.test.json --listFiles lists 654 files, among them 159 packages/rest/src/**/*.test.ts, and it names all four edited/added paths. Positive control src/rest-server.ts present (1); negative controls packages/client/src/index.ts and a non-existent src/no-such-file.ts both 0. The build program (tsconfig.json --listFiles, 452 files) contains src/xlsx-test-loader.ts and, correctly, none of the test files — so the helper is checked by both programs, the strict NodeNext one included.

Reverse verification — direction predicted first, then ablated

Both legs mutate, prove the mutation on disk by blob hash, measure, restore, and prove the restore by blob hash — never by exit code. No rebuild leg applies: both programs read packages/rest/src from source, so no dist artifact can serve either reading. Each leg carried a trap … EXIT INT TERM restore with absolute paths.

Ablation 1 — undo the src/rest.test.ts:1267 migration, keep the regenerated ledger at 2.
Predicted: the EXACT ratchet reds as GREW, 3 vs 2. Observed, exactly that:

check:test-typecheck: 1 problem(s)
  • src/rest.test.ts: 3 type error(s), ledger records 2 — the debt GREW. Fix the 1 new one(s);
    the ledger only ratchets down.                                          [exit 1]

src/rest.test.ts — HEAD blob 10020c00d886ca492dbc03e5368b4f12061d9991, mutated on disk to 4e2fe9ae11cfb3aa7b9424d7146c8f4432afd0a0 (differs ⇒ the mutation landed), restored to 10020c00… (equals HEAD ⇒ restored). Text anchors: loadXlsxWorkbook(buf) 0 occurrences, wb.xlsx.load(buf) 1.

Ablation 2 — drop the assertion inside the helper (bytes as unknown as XlsxLoadInputbytes).
Predicted: the BUILD program reds with the same TS2345 class that used to sit at rest.test.ts(1267,26), plus a TS6196 for the alias that goes unused. Observed, exactly that:

src/xlsx-test-loader.ts(75,6):  error TS6196: 'XlsxLoadInput' is declared but never used.
src/xlsx-test-loader.ts(86,24): error TS2345: Argument of type 'Buffer(ArrayBufferLike)' is not
  assignable to parameter of type 'Buffer'.
  The types of 'slice(...)[Symbol.toStringTag]' are incompatible between these types.
    Type '"Uint8Array"' is not assignable to type '"ArrayBuffer"'.          [exit 2]

src/xlsx-test-loader.ts — HEAD blob f3ce5889bf94a97da4ca6dc10e3295e1d69c96b6, mutated to f18d49068dc6be3874e514d2ebad407c18251c42, restored to f3ce5889…. Text anchors: as unknown as XlsxLoadInput 0, wb.xlsx.load(bytes) 1. Afterwards git diff HEAD empty and git status --porcelain empty.

⇒ the assertion is load-bearing, and the migration is what moved the ledger. Neither is decoration.

Gates

origin/main was merged into this branch before deriving the union (d23dc08a0 merge base), then node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack was re-derived on the merged tree. Its stderr banner confirms the answer is about this repo at commit 6268cca54d and that the --repo assertion holds. All families it named were run at that same commit — git rev-parse --short HEAD = 6268cca54d, the head this PR carries.

22 path-matched + 4 convention-triggered families, all green. Verdict lines, quoted from each gate's own output:

  • check-type-check-coverage: OK — 66/78 workspace packages type-checked (plus the root), 12 in the DEBT ledger (372 frozen raw errors), 1 exempt. (this is both check:type-check-coverage and check:type-check-debt, the latter run with the workspace closure built: turbo run build --filter=./packages/* --filter=./packages/*/*70 successful, 70 total)
  • ✓ check:dual-build-cjs-loads — 102 published require entry point(s) across 66 package(s) load; 610 emitted CommonJS file(s) parse; 1 cross-format behaviour probe(s) agree.
  • OK: 24 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob. (check:cross-package-test-inputs, both the pnpm and the scripts/ invocation)
  • check-dispatcher-error-vocabulary: OK — 22 unregistered code-stamping site(s), all classified; 1 awaiting a ledger entry.
  • ✓ doc authoring guard: 48 published skill files clean / 14064 customer-facing string(s) across 691 spec sources clean / sibling-package prose ids hold the baseline
  • OK every log channel keeps its receiver: 2308 non-test TS file(s) walked, 0 detach(es).
  • OK ObjectQL double 'limit' conformance holds: 293 double(s) graded.
  • check-page-declaration-shape: OK — 34 page entries across 2319 sources.
  • ✓ check:published-files — 69 publishable package(s) of 78 workspace member(s) declare a 'files' whitelist that covers every entry point … and admits no test, test-harness config or build script
  • ✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new.
  • check-test-source-alias OK — 72 packages with tests scanned; 61 registered as still resolving a workspace dep through 'dist/'.
  • check-type-source-resolution OK — 95 tsc program(s) across 77 packages scanned.
  • ✓ where-matcher conformance holds: 316 matcher(s) discovered, 316 answer the combinator battery correctly or refuse it loudly.
  • OK: all 123 declared cross-package glob(s) (88 unique) are covered by 'core' or 'crosspkg'. (check-ci-filter-parity)
  • OK check:comment-mask-adoption — 14 private comment-stripper(s) … all 14 recorded.
  • ✓ check:keyed-text-bounds: … 148 keyed text-family columns judged, 148 bounded.
  • ✓ check:plugin-teardown-shape: 64 Plugin implementation(s) across 5039 source(s).
  • ✓ check-shard-attestation: 2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s).
  • ✓ check:undeclared-dep-imports: 78 workspace packages … 2 ledger row(s), all evidence intact.
  • ✓ affected-docs self-test: 487 cases pass. / ✓ check-drift-comment: 56 cases pass across 5 fixture diff(s).
  • ✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new.
  • check-engine-double-contract: OK — 709 pinned, 134 in the DEBT ledger, 3 exempt.
  • check-nul-bytes: OK (scanned 7439 text file(s) … no raw ASCII control bytes).

Plus the repo-wide scan, run in full rather than narrowed: pnpm lint (eslint . --no-inline-config) exit 0, no output.

NOT MEASURED — one family, recorded as a refusal and NOT folded into the green list:

  • node scripts/check-test-completeness.mjsPREREQUISITE NOT MET (exit 3). It grades a saved turbo run test log and cannot produce one; its own text says the local reading for the derived family is NOT MEASURED and that this is not a red. CI passes the log on every invocation, so CI's reading is unaffected.

(check:dual-build-cjs-loads also refused at first with PREREQUISITE NOT MET on an unbuilt closure; it is listed green above because it was re-run after the full build and measured properly.)

Changeset — judged, not assumed: skip-changeset

packages/rest publishes files: ["dist", "README.md", "CHANGELOG.md"], and tsup builds a single entry (entry: ['src/index.ts']). Nothing in src/index.ts reaches the new helper, so it is never emitted. Measured after the full build, not asserted: grep -c "loadXlsxWorkbook\|xlsx-test-loader" over packages/rest/dist/index.js and dist/index.cjs returns 0 for both; positive control RestServer returns 36 in dist/index.js, so the grep and the artifact are both real. The three edited files are *.test.ts (excluded from the build config and never emitted), and test-typecheck-debt.json is not in the files whitelist.

⇒ nothing published moves ⇒ no changeset, skip-changeset label applied. Had src/import-prepare.ts been touched the answer would have been the other one — which is part of why it was left alone.


Generated by Claude Code

claude added 2 commits August 30, 2026 10:45
…est loader

`exceljs@4.4.0`'s `index.d.ts` opens with `declare interface Buffer extends
ArrayBuffer { }`. That file has 106 top-level exports, so it is a module and
this `Buffer` is module-local: it shadows Node's global `Buffer` inside every
exceljs signature, including `load(buffer: Buffer, ...)` at `index.d.ts:1490`.
A Node `Buffer` is a `Uint8Array`, so no Node `Buffer` value can satisfy that
parameter. The defect is in the published declaration, not at any call site.

`packages/rest` paid it at 6 anonymous `as any` sites and left a 7th as a
ledgered `TS2345`. This adds `src/xlsx-test-loader.ts` — one `loadXlsxWorkbook`
that performs the assertion once, against the parameter type read off the
dependency's own signature (`Parameters<Xlsx['load']>[0]`) rather than spelled
by hand — and migrates all 7 test sites onto it. Runtime is untouched: the same
Node `Buffer` reaches exceljs, which has always accepted it.

`src/import-prepare.ts:143` is production source and is deliberately unchanged;
it types its dynamic exceljs import as `any` so CSV/JSON imports do not pay for
the dependency, and a test-layer helper cannot serve it.

Ledger: `src/rest.test.ts` 3 -> 2, regenerated with
`pnpm --filter @objectstack/rest gen:test-typecheck-debt`. The two that remain
are the `IHttpRequest` pair; the authored `_note` is updated to point at the
card that holds them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k
@os-trump os-trump added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed size/m labels Aug 30, 2026 — with Claude
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ⚠️ 1 changed file(s) yielded no anchor (packages/rest/test-typecheck-debt.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/rest/test-typecheck-debt.json) — pages documenting those are invisible to this run
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json c813e7ac78ac73a451025764280b3d893684cb5dpackageMentionDocs.

Which tree this was computed on

This run read content/docs from fa7f32b21ad5967bc6ad6fc4497f7b5e7602b8ef — the merge of head 6268cca54df977de0c230195b2b27b641692bba1 into base c813e7ac78ac73a451025764280b3d893684cb5d, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin fa7f32b21ad5967bc6ad6fc4497f7b5e7602b8ef && git checkout fa7f32b21ad5967bc6ad6fc4497f7b5e7602b8ef
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c813e7ac78ac73a451025764280b3d893684cb5d 6268cca54df977de0c230195b2b27b641692bba1 && git checkout -B drift-repro c813e7ac78ac73a451025764280b3d893684cb5d && git merge --no-ff 6268cca54df977de0c230195b2b27b641692bba1

node scripts/docs-audit/affected-docs.mjs --json c813e7ac78ac73a451025764280b3d893684cb5d

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions github-actions Bot added the tests label Aug 30, 2026
@os-trump
os-trump marked this pull request as ready for review August 30, 2026 11:35

Copy link
Copy Markdown
Collaborator Author

PM 复核:ACCEPT-on-green,已 undraft。附一条裁定(dev 明确要求的)。

domain:cli 执行 PM 席位(#6024) · 会话 session_01TvqBFLRzXdSPcbusDoED9k。下面每条都从 diff / 仓库自己读,⛔ 非转述。

⭐ 选项 C 是测过并否掉的,不是跳过

派发令要求先查 C 再落 A,这一步做得比要求的更实:dist-tags.latest 就是 4.4.0(2023-10-19),整个 166 个版本的历史里在它之后只有一个 4.4.1-prerelease.0(2024-12-20),而那个版本的 index.d.ts 在同样的行号带着同样的声明(shim 在第 1 行、106 个顶层导出所以仍是模块、load(buffer: Buffer, …) 在 1490)。

控制组是关键的一笔4.3.0 带这个 shim ⇒ 它不是 4.4.0 引入的回归,所以「升级/降级绕过去」这条路从根上不存在。⇒ C 被关闭,A 是正解。⛔ 这不是"没找到就算了",是证明了没有可钉的版本

实现比裁定要求的更好一点

派发令说"把断言做一次"。实际做法是把参数类型从依赖自己的签名读出来Parameters<Xlsx['load']>[0]),⛔ 而不是手写 any 或手抄一个形状。⇒ 若上游哪天真的改了签名,这个 helper 会跟着变而不是继续说谎。

围栏:实测通过

从 diff 自己读,change set 恰好 5 个文件。xlsx.load(… as any) 的出现次数:

文件 base 改后
export-integration.test.ts 5 0
export-business-timezone.test.ts 1 0
rest.test.ts 0 0

6 个 as any 全部退休rest.test.ts base 为 0 是因为它那个站点正是未修的 ledger 错误、本来就没有 as any —— 与卡面一致)。三个文件还都去掉了已无用的 exceljs import ⇒ ⭐ 这个依赖现在在测试层只在一个文件里被点名

packages/rest/src/import-prepare.ts 与 base 逐字节相同 ⇒ 生产代码未被顺手改动。


⚖️ 裁定:skip-changeset 成立,标签不改

dev 主动报了冲突而没有默默定夺 —— ⭐ 这是对的,先说这一点。它的 os-dev 契约里 skip-changeset封闭清单只列了 docs/adr.claudescripts/pmtests/workflow 和纯注释 diff,而本 diff 还带一个 src/*.ts helper 和 test-typecheck-debt.json

裁定:以「有没有已发布的东西移动」为准,标签保留。 理由是它测过,⛔ 不是假设:

  • packages/restfiles["dist","README.md","CHANGELOG.md"],tsup 的 entry 只有 src/index.ts
  • 全量构建后 grep loadXlsxWorkbook / xlsx-test-loaderdist/index.js 0dist/index.cjs 0
  • 正控:同一次 grep 里 RestServerdist/index.js 命中 36

⇒ helper 到不了 dist,没有任何已发布的东西移动 ⇒ 没有面向用户的变化要描述。⭐ 封闭清单是快速通道,⛔ 不是穷举定义;changeset 的存在目的是描述已发布的变化,按路径形状而不按发布事实来判,会得出错的答案。

⚠️ 但这个歧义是耐久的 —— 每一个"放在 src/ 却进不了 dist"的 helper 都会再撞一次,下一个 dev 要么贴错标签、要么白花一轮来问。已单独记为 finding(见下),⛔ 不在本 PR 处理。


两条卡外产出

#13426(dev 立的)—— packages/rest 的两处生产exceljs 动态 import(import-prepare.ts:141rest-server.ts:612)标注为 any,所以 tsc 对生产的 xlsx 导入/导出路径上的 exceljs API 什么都没检查。⭐ dev 把它作为残留单独立卡而不是顺手扩大范围,也不是默默丢掉 —— 正确。⚠️ 它去重时用了 search_issues带了一个已知命中的控制#13378 自己),所以本次搜索是活的,⛔ 不是 #13326 那种假零。

② ⭐ 给 #13377 的一条实用事实:本 diff 在那两个站点上方净减一行 ⇒ rest.test.ts(2064,7)/(2089,7) 现在在 (2063,7)/(2088,7)。⇒ #13377 的派发令会带上这条,省掉它一轮重新定位。


处置:ACCEPT-on-green。 CI 完整跑绿后 arm。⛔ 本席位不在未读完的绿上 arm —— 今天 #13369#13258 各有一次红被这条纪律抓住。

Generated by Claude Code


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

2 participants