Fix core release coupling, declaration build, and CI gates - #847
Merged
iamtoruk merged 6 commits intoJul 27, 2026
Merged
Conversation
The gate compared only the three package.json manifests, so it passed while package-lock.json still recorded a stale workspace version. npm ci does not reject that either (checked on npm 10.9.2, 11.12.1, 11.16.0, 11.17.0), so nothing caught the exact drift the script exists to catch. Reproduced with a fixture whose manifests all read 0.9.20 while the lockfile recorded packages/core at 0.9.19 and a "*" CLI dependency: the gate exited 0. It now exits 1.
The build is `tsup && tsc`. tsup runs with clean:true, so it wipes dist and writes JavaScript; if tsc then fails, dist holds .js with no declarations. The build exits non-zero, but packages/core declared no prepublishOnly, so nothing rebuilt at publish time and a later npm publish would ship it. Reproduced: remove the declarations from a copy of dist and npm pack --dry-run still succeeds, with all 41 exports subpaths pointing at files absent from the tarball. npm pack was never the guard. Adds prepublishOnly (build then verify) and scripts/verify-dist.mjs, which asserts every exports target exists. CI runs verify-dist as well, so the guard is exercised on every push rather than only on the rare publish.
package.json#files ships dist and schemas but not src, so every emitted .d.ts.map pointed at a source file the consumer never receives. That was 151 files and roughly 135 kB of maps that resolve to nothing. Dropping declarationMap takes the tarball from 389 files to 238 and 376 kB to 354 kB. The declarations themselves are unchanged: 151 .d.ts, all 41 export targets still verified present.
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
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.
Makes
@codeburn/corebuildable and publishable from a clean checkout. Build, release, and CI plumbing only — nosrc/changes, no schema changes, no behavior changes.Relates to #796 and #809.
Baseline on
feat/core-extraction@88f298cnpm cipackages/core@0.9.19against a manifest saying0.9.20npm run check:workspace-versions0.9.20npm run typecheck -w @codeburn/corenpm test -w @codeburn/corenpm run build -w @codeburn/coreERR_WORKER_OUT_OF_MEMORYnpm pack -w @codeburn/core --dry-runThe build failure:
tests/import-smoke.test.tsshells out tonpm run buildinbeforeAll, so the DTS crash took the test suite down with it. That is the whole delta between 507 and 509 tests.Changes
1. Couple core and CLI releases (
c52c567)Root, CLI, and core were at
0.9.19/0.9.19/0.9.20, and the CLI depended on"@codeburn/core": "*". A published CLI could resolve any core version, including ones it was never tested against.All three now sit at
0.9.20, the CLI pins the exact core version, andscripts/check-workspace-versions.mjsfails the build if they drift. The gate was tested against fixtures covering*,^0.9.20, CLI drift, core drift, and the aligned case — it rejects the first four and accepts the last.2. Emit declarations with
tsc(0054c9c)tsup's DTS worker bundles types for all 41 entries in one pass and exhausts the heap. Reproduced on Node 22.13, 24, 25, and 26 — this is not a local-environment artifact.
dts: falseintsup.config.ts; declarations now come fromtsc -p tsconfig.build.json(emitDeclarationOnly). tsup still emits the ESM JavaScript. All 398 relative imports insrc/already carry.jsextensions, so unbundled.d.tsresolve correctly undernode16/nodenext.Side effect: the core build went from crashing to ~40 ms, and the test suite from 34 s to 4 s.
3. Gate core in CI (
7e29455)CI ran Semgrep only — never a clean install, core build, typecheck, test, or pack. Adds a
corejob on Node 22.13.x / 24.x / 26.x runningnpm ci,check:workspace-versions,typecheck,test,build, andnpm pack --dry-run. The Semgrep job is unchanged. Also adds"sideEffects": falseto core so bundlers can tree-shake the 41 subpaths.Verification
From a fresh clone, not the working tree:
The same six gates were run in Docker on
node:22.13(npm 10.9.2),node:24(npm 11.16.0), andnode:26(npm 11.17.0). All pass on all three.Consumer smoke — the real tarball installed into an empty project:
node:fs/child_process/net/httpimportmoduleResolution: node16withskipLibCheck: false— no diagnosticsThe existing no-I/O import guard and content-minimization tests are untouched and still pass.
Notes for review
Three things worth a maintainer's eye, none blocking:
The
npm cifailure did not reproduce. The audit behind this work recorded a cleannpm cifailing on lock/manifest disagreement. It passes on npm 10.9.2, 11.12.1, 11.16.0, and 11.17.0. The underlying drift was real and is fixed; the reported symptom was not reproducible.declarationMap: trueships 151.d.ts.mapfiles (~135 kB unpacked) that point atsrc/, whichfilesdoes not publish. They resolve to nothing for consumers. DroppingdeclarationMap, or addingsrctofiles, would both be defensible — happy to do either.The Node 22.13 leg skips
tests/providers/zed-decode.test.ts(6 tests). The suite gates it onzlib.zstdCompressSync, which landed in Node 22.15, whileengines.nodeis>=22.13.0. Testing the declared floor is correct, but zed decoding has no coverage on that leg. Raising the floor to>=22.15.0would close the gap.No publish is included here. npm publication stays a maintainer action.
Update: two defects found in review, both fixed
A multi-model review pass over this branch surfaced two release hazards. Both are now closed by the last two commits, and both were reproduced before being fixed.
4. Check
package-lock.jsonin the version gate (6f1f8a4)scripts/check-workspace-versions.mjscompared only the three manifests, so it passed while the lockfile still recorded a stale workspace version — andnpm cidoes not reject that either (checked on npm 10.9.2, 11.12.1, 11.16.0, 11.17.0). The gate was blind to the exact drift it exists to catch. Reproduced with a fixture whose manifests all read0.9.20while the lockfile recordedpackages/coreat0.9.19plus a"*"CLI dep: the gate exited 0. It now exits 1.5. Guard publish against a half-built dist (
0ec9ea9)The build is
tsup && tsc, and tsup runs withclean: true. If tsup succeeds and tsc fails,distholds.jswith no declarations. The build exits non-zero — butpackages/coredeclared noprepublishOnly, so nothing rebuilt at publish time.Reproduced: strip the declarations from a copy of
distandnpm pack --dry-runstill succeeds, with all 41 exports subpaths pointing at files absent from the tarball.npm packwas never the guard.Adds
prepublishOnly(build, then verify) andpackages/core/scripts/verify-dist.mjs, which asserts every exports target exists. CI runsverify-disttoo, so the guard is exercised on every push rather than only on the rare publish. Negative-tested: it accepts an intactdistand rejects the half-built one.This hazard is pre-existing, not introduced here — the old single-
tsupbuild had the same window. It is cheap to close, so it is closed.Two review claims that did not survive checking
TS<4.7consumers usingmoduleResolution: "node"would lose types now thatdist/index.d.tsis a thin re-export rather than a tsup bundle. Refuted — a real node10 consumer (module: commonjs,moduleResolution: node,skipLibCheck: false) compiles with no diagnostics. TypeScript maps./schema.jsto./schema.d.tsunder node10 as well. Also verified clean underbundlerandnodenextacross all 41 subpaths.engines.node: ">=22.13.0"is wrong because zed decoding needs zstd from 22.15. Refuted by source —packages/core/src/providers/zed/decode.ts:6-7documents that the host only calls the decoder after confirmingzstdDecompressSyncexists. Deliberate, documented degradation. It remains a CI visibility nit: those 6 tests skip silently on the 22.13 leg while CI reports green.declarationMapis now dropped (ed4f84d). It was emitting 151.d.ts.mapfiles pointing atsrc/, whichfilesdoes not ship, so they resolved to nothing for consumers. The tarball goes from 389 files / 376.2 kB to 238 files / 354.4 kB. Declarations are unchanged: 151.d.ts, all 41 export targets still verified present.