Uh oh!
There was an error while loading. Please reload this page.
fix(metadata-core,service-cluster): stop emitting and publishing the CJS half of ./testing - #13114
Merged
Merged
Conversation
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 2 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
This was referenced Aug 29, 2026
os-elon
marked this pull request as ready for review
August 29, 2026 05:53
os-elon
enabled auto-merge
August 29, 2026 05:53
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes#13013
#13001made both./testingsubpaths ESM-only, dropping therequirecondition that pointed atdist/testing.cjs. The build kept emitting those files andfiles: ["dist"]kept packing them, so every release shipped bytes no exports condition could reach. This prunes them without moving anything reachable.The route, chosen on measurement
The card left three routes open and did not commit. All three were measured.
fileswhitelistscripts/check-published-files.mjs:473fails closed on any negatedfilespattern ("which this guard does not model"). A positive re-spelling would have to enumerate build output by glob, and every entry beyonddist/README.md/CHANGELOG.mdneeds a registered reason. It also leaves the build still emitting the dead bytes.Why per-entry splitting is a silent breaking change
indexandtestingshare a chunk, and the chunk carries the error classes:src/errors.tsexportsMetadataError,ConflictError,NotFoundError,SchemaValidationError,BranchError. One config per entry givestesting.jsits own copies of them, soConflictErrorreached through@objectstack/metadata-core/testingstops being the class thrown by@objectstack/metadata-core— and the contract suite this entry point exists to publish asserts exactly that identity (src/contract-suite.ts:344,.rejects.toBeInstanceOf(ConflictError)). Every downstream driver package running the suite would fail, on a change that reads as a build-config tidy-up.Splitting by format avoids it entirely: the ESM half keeps both entries, so chunk sharing is untouched; the CJS half takes
src/index.tsalone, and.is its only entry point, so there is exactly one copy either way.The
cleanhazard is a race, not an orderingThe card expected "the second config wipes the first's output". It is worse than that: tsup runs an array config through
Promise.all(tsup/dist/index.js, theArray.isArray(configData)map), so the halves build concurrently and acleanin either one races the other's writes in either direction.cleantherefore moves out of tsup and into thebuildscript (rm -rf dist && tsup). That is also a stronger clean than tsup's own, which unshifts!**/*.d.{ts,cts,mts}and so preserves stale declarations — it would have leftdist/testing.d.ctson disk through every rebuild of an existing worktree, which is precisely the file this change exists to stop emitting.Verification
Union re-run on the final commit,
2fde235c84.1. Nothing reachable changed — sha256 over both
dist/trees, before vs after. Byte-for-byte identical:index.js,testing.js, both maps,chunk-46MG4YHS.js+ map,index.d.ts,testing.d.ts,repository-*.d.ts(and the service-cluster equivalents).index.cjschanges only because the shared CJS chunk is now inlined into the sole remaining CJS entry;index.d.ctslikewise. This is the load-bearing evidence for the class-identity claim above — the ESM output is not merely equivalent, it is the same bytes.2. The published tarball, measured both ways with a positive control. The pre-fix side is a real measurement, not an inference: the fix was committed first, then the two configs were reverted to their pre-fix bytes with the mutation confirmed on disk by anchored
grep -ccounts (pre-fix marker 2, post-fix marker 0), rebuilt, packed, and restored viagit checkout HEAD -- ...withgit diff HEADproved empty. A trap held the restore on every exit path.@objectstack/metadata-coretesting.cjs,testing.cjs.map,testing.d.cts,chunk-H2D6OJ76.cjs,chunk-H2D6OJ76.cjs.map,repository-Dl3EudaY.d.cts@objectstack/service-clustertesting.cjs,testing.cjs.map,testing.d.ctsPositive control: the same
npm pack --dry-runinvocation still listsdist/testing.js(24.6 kB) anddist/testing.js.mapin the after run, so the absences are readings and not a broken instrument.3.
check:dual-build-cjs-loadsstill reads a non-empty, honest population. This was the card's stated risk — a gate that greens because it no longer reads anything is the vacuous-pass class of #13014.Entry points, packages and probes are unchanged; the PARSES population falls by exactly 3, which is exactly the three CJS files that stopped existing (
metadata-core/dist/testing.cjs,metadata-core/dist/chunk-H2D6OJ76.cjs,service-cluster/dist/testing.cjs). Two independent readings agree on the 613: this branch measured it atdb39dfc1c9, and #13050's ownMEASUREDsnapshot records it at8cb96ec41b.MEASURED.cjsFilesis deliberately not updated — it is a provenance record attributed to a named commit, and it is still true of that commit. The gating values are the floors, andMIN_CJS_FILES = 520clears with margin. The gate's own self-test (53 cases, including "every vacuity floor driven to zero with its green control") ran in the same command, so the instrument is proved able to return non-zero in the same run that returned this pass.4.
check:published-filescannot object, verified by reading rather than by a green run. Its five invariants ask that the whitelist covers declared entry points (SUFFICIENT); itswalk()never entersdist/at all (SKIP_DIRS), so an extra shipped file is outside every question it asks, in both directions. It is green, but that green is not the evidence.Gates. Derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(re-derived after the changeset existed and after each sync, never off the stale list): 28 families, all green. Two returned non-zero and are NOT MEASURED rather than red —check-test-completenessprinted its usage banner (it needs a turbo test log argument, so it never entered the gate body) andcheck-half-statesexited 3, PREREQUISITE NOT MET, because this container's token is the 14-byte proxy placeholder; it swept nothing.Tests. The two changed packages plus every direct consumer of the
./testingsubpaths:metadata-core(260),service-cluster(66),metadata-fs(70),metadata-protocol(2010 passed, 10 skipped),service-cluster-redis(28) — 2434 passed, 0 failed.metadata-coretypecheckgreen. These suites resolve the subpath through the packageexportsto the builtdist/testing.js(no vitest alias inpackages/metadata-fs/vitest.config.ts; a direct ESM import from inside the package resolves and binds real exports), so they exercise the built artefact rather than source. Independently,require.resolve('@objectstack/metadata-core/testing')answersERR_PACKAGE_PATH_NOT_EXPORTED, which re-confirms the card's premise from the consumer side.Out of scope, filed
#13112 — 28 published packages ship 36
.d.ctsdeclarations (5.2 MiB) that notypescondition points at. Same defect class, measured ~48x larger than this card. Deliberately not fixed here: unliketesting.cjs, those files have a plausible reader (arequire-side consumer resolvingindex.cjsis where an adjacentindex.d.ctswould be looked for), and the remedy could equally be declaring them rather than deleting them — a published-surface decision across 28 packages, not a mechanical prune.Routing note
Triage routed this to
domain:devxby subject (build/publish pipeline shape, read by twoscripts/gates) while the fix lands inpackages/metadata-core(lane: engine) andpackages/services/service-cluster(lane: services), both outside the devx tree — and wrote its reasoning down expressly so it can be overturned. If either owning seat objects to this shape, this PR should be re-routed or split by package ownership rather than pushed through.Also worth recording for the next reader: the card writes the second package as
packages/service-cluster; it actually lives atpackages/services/service-cluster. That is the path-precision false-zero shape the card itself warned about.Generated by Claude Code
Generated by Claude Code