ci: add built-artifact release gate (#765, #749) - #766
Draft
tyler-reitz wants to merge 9 commits into
Draft
Conversation
Two dist-level regressions shipped as patch releases with nothing in the release flow comparing the built artifact: the 4.2.4 ObservableStatus type break (#749) and the 4.2.4/4.2.5 App Router / Vite crash caused by the CJS use-sync-external-store shim being inlined into the ESM output (#759, fixed in #760). In both cases the source was fine and only the emitted artifact regressed. Adds scripts/release-gate.mjs, run in CI as a new "Verify built artifact" job between build and publish. It verifies the packed reactfire.tgz that the publish job uploads verbatim, so it checks the exact bytes that ship, and it is dependency-free so the job needs no npm ci. Checks (#765 items 1, 2, 4, 5 plus #749): - no-cjs-in-esm: no CJS interop or dynamic require in the ESM entry - externals: externals stay external, nothing unexpected is inlined - exports-map: every path in exports/main/module/typings is in the tarball - types: emitted .d.ts match a checked-in accepted baseline - size: packed tarball and entry-point gzip within 10% of baseline Verified against the published tarballs: 4.2.5 fails no-cjs-in-esm and externals, 4.2.4 fails those plus types (surfacing the ObservableStatus union as a readable diff), and the current build passes. #749 asks for enforcement rather than a warning, but a textual diff cannot classify additive versus non-additive on its own. So the gate fails on any delta and the acknowledgement is `npm run gate:accept` plus a committed baseline, which puts the semver decision in front of a reviewer as a diff. Also repairs the size-limit config, which pointed at TSDX-era filenames (dist/reactfire.esm.js) that the vite build has not emitted for some time, so `npm run size` was silently checking nothing. Items 3 and 7 of #765 (entry-point load test, runtime smoke render) are not included; see release-gate/README.md.
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Follow-up to review feedback on the release gate. Three fixes, all cases
where the gate quietly under-delivered.
1. Add test/release-gate.test.mjs (21 tests, no emulators needed).
The gate had no tests, which is the exact failure mode it exists to prevent:
edit a marker regex, break detection, and every run still goes green. A
silently disarmed gate is worse than no gate because it manufactures
confidence. Fixtures are byte-faithful to what shipped, including the 4.2.5
require shim, and cover the checks end to end against synthetic packages.
Required a small refactor for testability: checks now take a report object
instead of mutating module-level arrays, helpers are exported, and main()
only runs when the script is invoked directly.
2. Fix collectImports missing every minified import form.
The patterns required whitespace after the keyword, so `import{a}from"react"`,
`export{a as b}from"react"` and `export*from"rxjs"` all returned nothing. It
worked only because rolldown happens to emit spaced import statements while
mangling identifiers, which is an incidental output detail rather than a
contract. That is the same class of build-output change that caused #759, so
the gate's own parser was vulnerable to what it exists to detect. Partial
failure was the dangerous case: full failure trips MUST_BE_EXTERNAL loudly,
but a partial format change silently weakens the unexpected-externals half.
3. Make listTypeFiles recursive.
tsconfig emits with rootDir ./src, so a subdirectory of src/ emits nested
declarations. The flat readdir meant dist/nextjs/*.d.ts would sit entirely
outside the #749 check the moment #739 lands src/nextjs.
Also adds an eslint override so plain .mjs is linted as Node with vitest
globals (typescript-eslint disables no-undef for .ts, but not .mjs), and
extends the lint and format globs to cover scripts/.
An independent review found the gate claimed broader coverage than it had. All five are fixed, and every fix is pinned by a test that fails when the fix is reverted (verified by mutating each one back). 1. exports-map never checked `main` or `typings`. It filtered to paths starting with ".", but `main` is "dist/index.umd.cjs" and `typings` is "dist/index.d.ts", neither of which has the prefix. So the two fields its own failure message named were the two it skipped. Probed directly: with both missing and no exports map, it reported zero failures. It passed until now only because exports['.'].require duplicates `main`; `typings` has no equivalent and was genuinely unchecked. The test that appeared to cover this deleted the one path that was covered, so it passed for the wrong reason. 2. The size tolerance was too loose to catch what it advertised. Measured from the published tarballs, the #759 shim inlining moved the ESM entry +3.1% gzip and the tarball +0.3%, both well inside the old ±10% band. The claim that a size guard "might well have caught the shim getting inlined" was wrong, and is corrected in the README: size is a coarse guard against gross packaging changes, not an inlining detector. Tolerance is now ±2%, above CI's version-stamp churn (+0.5% tarball) and below a #759-sized change. The check also had zero test coverage; deleting it outright left every test green. It is now covered, including at the #759 magnitude. 3. Added and removed .d.ts detection was unpinned. The logic was correct but nothing held it there. This is the case the recursion fix in the previous commit exists to feed: a new entry point emitting dist/nextjs/*.d.ts is exactly #739, and a disappearing declaration file is a hard breaking change. 4. Both bundle checks only ever opened the `module` entry. Nothing guarantees a single ESM chunk. A second entry point or rollup deciding to split would put code where an inlined require is invisible, and chunking is the kind of incidental build-output change this gate exists to be robust against. Both checks now walk every ESM file in the package, and expected externals are matched across their union so an import from a chunk still counts. 5. `react` was matched as an exact string, and --accept took a tarball. Switching to the automatic JSX runtime would import react/jsx-runtime and double false positive: unexpected external, plus "react was inlined" when it was not. Now matched as a pattern, like firebase already was. Separately, `--accept <tarball>` would have recorded a baseline from an arbitrary old build, blessing whatever regression it contained; it is now rejected, as are unknown flags.
The zizmor security scan reports 15 `unpinned-uses` findings against this file: actions referenced by floating tag (`actions/checkout@v4`) rather than pinned to a commit hash, which the blanket policy requires. A floating tag can be repointed at new code by whoever controls it, so an unpinned action is an unreviewed dependency with access to the job. Only 3 of the 15 are lines this branch added; the other 12 predate it. The scan runs only when a workflow file changes, and this is the first PR to touch .github/workflows/, so the condition was pre-existing and simply never surfaced before. Fixing only the 3 new ones would leave the check red, since it fails on any Medium/High finding. Each SHA is the current tip of that action's v4 tag, with the resolved release in a trailing comment so the version stays legible and Dependabot can still bump them. docs.yaml has the same two unpinned refs and is not touched here: the scan did not flag it, and this PR is already carrying a release gate. Worth a separate pass.
Clears the two remaining zizmor medium findings on test.yaml. excessive-permissions (6): the workflow declared no `permissions:` block, so every job ran with the default token scope. Added a top-level `permissions: contents: read`. No job writes to the repo through GITHUB_TOKEN; the publish job authenticates to npm with NODE_AUTH_TOKEN, so this does not touch the publish path's credentials. artipacked (4): actions/checkout persists the token into .git/config by default, leaving it readable by every later step in the job. Nothing here re-uses git credentials after checkout, so `persist-credentials: false` is safe. The publish job has no checkout step and is unchanged. Both classes predate this branch. Verified with zizmor 1.25.2 locally: test.yaml goes from 10 medium to 0. The 6 remaining cache-poisoning findings are pre-existing, are already suppressed by the CI config (the failing run exited 13/Medium, not 14/High), and fixing them would mean restructuring how the workflow caches around the publish path, which does not belong in this PR. docs.yaml has findings of its own and is untouched here; the scan only runs on changed workflow files. Worth a separate pass.
Contributor
Author
|
@jhuleatt this is ready for review. All checks green. Summary of what needs you, roughly in order of how much it matters: 1. One design decision, and it is the reason I have not merged this. #749 asks for the candidate's types to be diffed against the last published version on npm. This PR instead diffs against a baseline committed in the repo, updated deliberately via My lean is the npm-published version, but I did not want to change it unilaterally, because merging as-is auto-closes #749. If we go the npm route I would rather downgrade that to a plain reference and keep the issue open. Happy either way, I just want the call to be yours since it is your issue. 2. Two things needing admin, which I do not have.
3. Heads-up on something this PR uncovered, no action needed from you here. This is the first PR to touch 4. Also correcting myself. An earlier revision of this description claimed a working size guard would likely have caught the #759 shim inlining. That was wrong. Measured against the published tarballs, that regression moved the ESM entry only +3.1% gzip and the tarball +0.3%, so the ±10% band I originally used would have sailed straight past it. Tolerance is now ±2% and the description says plainly that Scope note: this implements #765 items 1, 2, 4, 5 plus #749. Items 3 and 7 (entry-point load test, runtime smoke render) are not included and are documented in Separately, #740 is still waiting on your semver call whenever you get a chance. |
#749 specifies packing the last published version and diffing against that. The gate diffed against a copy checked into `release-gate/baseline/` instead, which surfaces at PR time but can drift from npm. That drift is live rather than theoretical here: reactfire is published by hand, so a release can land on npm without the baseline being refreshed. The `types` and `size` checks now compare against `npm pack reactfire@latest`. If the registry is unreachable, or the package has never been published, both skip with a note rather than failing, so an outage cannot wedge a pull request that has nothing to do with the published surface. The bundle checks need no network and always run. Removing the baseline removes what `--accept` used to record, so the acknowledgment becomes a fingerprint: `release-gate/accepted.json` holds a digest of the type surface, the measured sizes, and the published version they were taken against. PR-time enforcement is preserved (a type change still fails until someone runs `npm run gate:accept` and commits the result, so the semver call is still made in review), but there is no longer a second copy of every `.d.ts` to maintain. The digest covers the exact surface accepted, so a later edit invalidates it rather than riding along, and it is scoped to one published version, so it expires on the next release instead of carrying forward silently. Also implements the half of #749 that was missing: an acknowledgment alone did not stop a type change shipping as a patch, which is exactly what 4.2.4 did. Once `package.json` moves off the published version the gate treats it as a release candidate and requires a minor or major bump for a changed type surface. During normal development the two versions match (the bump is its own commit, e.g. 7f93210 "4.2.6"), so the rule stays quiet and cannot false-positive on every types-touching PR. Classifying a diff as additive versus breaking still needs a real API differ and is deliberately left out. Tests go 38 to 56, covering the acknowledgment lifecycle (accepted, invalidated by a later edit, expired by a new release), the three version-rule cases, the no-published-release skip, the digest, and version parsing. Verified end to end against the published tarballs: 4.2.4 and 4.2.5 both fail, 4.2.6 passes.
The npm comparison had one fail-open path: any failure to download the published release, including a registry outage, skipped the `types` and `size` checks with a note and left the job green. A skip is indistinguishable from a pass in the check's status, so a blip would have produced a silently ungated release, which is the failure mode this gate exists to prevent. The two causes are now separated. "Never published" stays a skip, since at bootstrap there is genuinely nothing to compare against and it will not resolve by retrying. A failed fetch is retried three times with backoff and then reported as a failure, naming the underlying error and saying the job is re-runnable. Wedging a pull request for a few minutes is a better trade than a gate that has quietly stopped gating. The failure is reported once rather than once per affected check, so the registry error does not bury itself under two check names. `fetchPublished` now takes an injectable runner, so both classification and the retry behaviour are covered without touching the network: a 404 is not retried, a timeout is retried three times, and a retry that succeeds returns the package. Tests go 56 to 64.
CI stamps an experimental version into package.json before packing, so the gate sees `4.2.6-exp.<sha>` while 4.2.6 is published. Two comparisons assumed the candidate and the published release would be identical outside a real release, and both were wrong in CI. The version rule compared version strings. Since the stamped version is never equal to the published one, every pull-request build read as a release candidate, and since the numeric core matches, as a patch bump. The first pull request to legitimately change the type surface would have failed with a version error unrelated to the change. It compares the numeric core now, so a stamped build of the published version is not a release while a genuine bump, including its prereleases, still is. The size acknowledgment compared recorded byte counts exactly. Those numbers come from `gate:accept` run locally and are checked against a CI build, which the stamp alone makes differ (measured ~+0.5% on the tarball), so no acknowledgment would ever have applied in CI and an intended size change could not be landed at all. Recorded sizes are now matched within the existing tolerance, with the entry set still required to agree so an acknowledgment cannot silently cover a new entry point. Neither surfaced on this branch because the type surface is currently identical to 4.2.6, so the checks returned before reaching either comparison. Found by reading the CI log rather than from a failure. Tests go 64 to 69, covering the stamped-version case, a prerelease of a genuine bump in both directions, and acknowledgment matching at CI-sized drift, beyond tolerance, and across a changed entry set.
tyler-reitz
force-pushed
the
chore/built-artifact-gate
branch
from
July 28, 2026 19:51
6135f29 to
21492b4
Compare
tyler-reitz
marked this pull request as draft
July 28, 2026 20:46
Installs the packed tarball into a throwaway project with real react and firebase, then actually loads both entry points and checks that known exports are present. Runs against React 18 and 19, mirroring the type-check matrix, as the new "Verify package loads" job. `publish` now also gates on it. This catches the #759 class by observing the failure rather than by pattern-matching the artifact. Verified against the published tarballs: 4.2.4 import() throws the require-shim error, require() loads 4.2.5 import() throws the require-shim error, require() loads 4.2.6 both load Only the ESM entry breaks, so a check that loaded one entry point would have missed it. `exports-map` cannot see this at all, since every file it looks for is present in 4.2.5, the package just does not run. Deliberately a separate script and a separate CI job rather than a sixth check inside release-gate.mjs. It needs a real dependency tree, and keeping it out of the gate is what lets the gate stay dependency-free and unbreakable by a dependency change. A load that resolves but exports nothing is still broken, so the probes assert known exports from several submodules rather than only that the import settled. A probe that dies without printing is reported as a failure rather than read as a pass. 25 tests. The runner is injectable, so the orchestration is covered without installing anything. Mutation testing found a real gap: removing the CJS branch from collectFailures left every test green, because the suite only covered an ESM-only failure. Added the mirror cases. The remaining half of #765, item 7 (runtime smoke render), now has a narrower justification: it was argued for as the only check that observes a runtime failure, and this one does that for the #759 class. Its unique value is now limited to browser-only breakage. Noted in the README rather than acted on.
armando-navarro
approved these changes
Jul 29, 2026
armando-navarro
left a comment
armando-navarro
left a comment
There was a problem hiding this comment.
Thanks Tyler. Reviewing the PR as it stands now, with the npm comparison and the new load test in. I verified the reworked machinery end to end and it holds up. Approving. The asks below are non-blocking on today's state, though the first two deserve attention before or right after merge.
What I verified
- The 94 tests pass (69 gate, 25 load), and a live gate run against npm 4.2.6 passes with 0.0% entry deltas. Since both sides are now measured on the same machine, the compression-environment noise a committed baseline would carry is designed away.
- The acknowledgment lifecycle behaves: an unacknowledged type drift fails,
gate:acceptlets exactly that surface through, and any later edit invalidates the digest. The suite also pins that a new npm release expires an old acknowledgment. - With the registry unreachable, the gate fails loudly after three attempts instead of skipping. A skip would be indistinguishable from a pass, so this is the right call, and the failure text says so.
- The load test catches the real thing: against the published 4.2.5 tarball it fails with the exact rolldown
requirecrash on the ESM entry, and the current build loads both entries cleanly on React 18. - The version rule blocks a type change shipped as a patch (the 4.2.4 case), stays quiet for CI's
-expversion rewrite today while the majors match, and still applies to prereleases of a genuine bump. - Pipeline wiring:
publishneeds the test job plus both verify jobs on both publishing triggers, the new job reuses the same pinned action commits, needs no secrets, and runs on fork PRs.
The description no longer matches the PR
The body still describes the committed-baseline design. Five spots are now wrong:
- The "Decision needed before merge" section: the code has decided it.
- "Implements #765 items 1, 2, 4, 5": item 3 is in.
- The follow-ups bullet saying items 3 and 7 are not implemented.
- "38 tests": it is 69 plus 25 now.
- The new load-test job is not mentioned at all.
One more thought: since that decision had been explicitly flagged for Jeff, a short comment noting the switch and why would let his review start from the right frame rather than from the stale table.
On the footer specifically: your own description says that under the npm route "Closes #749" should become a plain reference with the issue left open, so that line wants the same update, with the final close-or-keep-open call staying with Jeff as you originally framed it. To be fair on the merits, the implementation does deliver #749's literal ask, so if Jeff prefers to let the merge close it, that is defensible too.
The latest comparison will block every v4 release once v5 ships
This one is invisible today and painful later, and since the v5 branch already exists I want it on the record now. I ran the version helpers against the future state where 5.0.0 holds npm latest:
- A v4 maintenance release
4.2.7reads as a release candidate withisMinorOrMajorBumpfalse, so the gate fails it as "a patch bump over 5.0.0". - A v4 minor
4.3.0is misjudged the same way, because a lower major can never register as a bump. - Even a stamped canary build (
4.2.6-exp.<sha>) reads as a release candidate against5.0.0, and every v4 PR fails the types diff against the v5 surface before the version rule even runs. So the blast radius is the whole v4 line's CI, not just release cuts. - Acknowledging clears the types diff but not the version rule, so a contributor who hits this cannot get green without a code change. The change itself is small (the comparand is one constant), which is exactly why it is worth planning rather than discovering.
Fine to land as-is today (latest is 4.2.6, so nothing fires). My ask is just to file it with a clear "before 5.0.0 takes latest" milestone, or fix it here if you prefer. Two shapes that seem workable: make the compare target branch-aware (a per-branch dist-tag, or derive it from the candidate's own major), or make the version rule major-aware. Relatedly, once the gate lands on v5, every PR there will diff against the v4 surface and demand acknowledgment churn against a baseline that means nothing for that branch, so the same fix helps both.
One gate:accept covers types and size together
accepted.json records the type digest and the size snapshot in one step, and the size check honors whatever the snapshot says:
- I built the ordinary case: a candidate with one benign added declaration file and a tarball 40% over published, acknowledged the normal way, passes with zero failures. The +40% shows up only as a log note.
- The committed file records the size as a raw byte count with no delta or percentage, so nothing in the acknowledgment diff itself flags that a size regression rode along with the type change.
- Bounded, to be fair: the primary inlining detectors do not consult the acknowledgment at all, so this only softens the size backstop.
Some cheap improvements:
- Have
gate:acceptprint the size deltas it is about to bless, so whoever runs it sees what they are signing before committing (recording a readable delta in the file would go one better and put it in the PR diff too). - Have it run the three absolute checks first and refuse to record on failure, which also keeps a broken build from becoming anyone's blessed snapshot.
- A related docs nit: the README's accepting-a-change section skips the build-first step its running-it section includes, so on a fresh clone the accept command errors until you build.
Two carried notes on the checker itself
- The checks never look at
package.json's shape, only at whether referenced paths exist. A tarball with therequireexport condition removed,typingsdeleted, andpeerDependenciesdeleted still passes everything (I re-ran this against the current head). An in-gate assertion on the extracted tarball's critical fields, excludingversionsince CI rewrites it, would close it. - Nothing covers the wiring from a non-empty failures list to a non-zero exit. It works today (I checked all three exit codes), but a refactor could leave the gate exiting 0 forever with every test green. One child-process test against a crafted bad tarball would pin it.
Small notes
- The load job has no retry or outage handling the way the gate's fetch does, and neither new job caches npm downloads, so every PR pays two uncached installs per React version. Fine to leave, worth knowing.
- The load fixture pins firebase to
^11.10.0, which matches the repo's own devDependency today. When the repo moves to firebase 12, the fixture will quietly keep testing against 11 unless the constant moves with it. A one-line comment tying the two together would prevent the drift. type-checkis not inpublish's needs list (pre-existing, not this PR): a type error in source would not block a canary.
If any of this reads wrong, say so and I will dig back in. The version-helper runs and the coupling fixture are one-liners to re-run.
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
3 participants
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.
Summary
Adds a built-artifact release gate: a new Verify built artifact CI job between
buildandpublishthat checks the packedreactfire.tgzbefore it can be published.Two dist-level regressions have shipped as patch releases with nothing in the release flow comparing the built artifact:
ObservableStatus<T>from a flat interface into a discriminated union. Type-only, but it broke strict-TS consumers on a patch bump (Add a published .d.ts diff to the release process to catch breaking type changes #749).use-sync-external-store/shiminto the ESM output, producing a dynamicrequire()that threw in any browser bundle (App Router crash: reactfire 4.2.4/4.2.5 throw "dynamic usage of require is not supported" on the client #759, fixed by fix: externalize useSyncExternalStore shim to fix App Router crash #760).In both cases the source was fine and only the emitted artifact regressed.
The job verifies the same tarball the
publishjob uploads verbatim, so it checks the exact bytes that ship. It is dependency-free by design (node builtins plustar), so it needs nonpm ciand cannot itself be broken by a dependency change.Implements #765 items 1, 2, 4, 5 plus #749.
Decision needed before merge
#749 specifies packing the last published version and diffing against that. This PR instead diffs against a baseline committed in the repo (
release-gate/baseline/), updated deliberately vianpm run gate:accept.My lean is diffing against npm, since it cannot drift and is closer to the issue's intent. I have not changed it unilaterally because merging as-is auto-closes #749 — if we go the npm route, that should become a plain reference and the issue should stay open.
Checks
no-cjs-in-esmexternalsexports-mapexports/main/module/typingsmissing from the tarball.types.d.tsversus the accepted baseline, including added and removed declaration files.sizeOn
sizespecifically: it is a coarse guard against gross packaging changes, not an inlining detector, and should not be read as one. Measured from the published tarballs, the #759 inlining moved the ESM entry only +3.1% gzip (16815 to 17330 B) and the tarball +0.3%. The ±2% band is set above CI's version-stamp churn (+0.5% tarball, +0.2% entries) and below a #759-sized change.no-cjs-in-esmandexternalsare what actually catch inlining.Verification
Run against the published tarballs, not just the current build:
no-cjs-in-esmexternalstypes* 4.2.3 predates the shim import entirely, so that is expected rather than a false positive.
The 4.2.4
typesfailure renders theObservableStatuschange as a readable inline diff, which is the #749 case exactly.test/release-gate.test.mjs, 38 tests, no emulators needed (npm run test:gate). A gate that silently stops gating is worse than no gate, so the detection logic is pinned by tests rather than by having been verified by hand once. Fixtures are byte-faithful to the shapes that shipped, including the 4.2.5requireshim. Each check was mutation-tested (revert the logic, confirm a test fails), which caught one case where the size tolerance value was unpinned even though the check itself was covered.Workflow hardening (zizmor)
Two of the five commits are unrelated to the gate. This is the first PR to touch
.github/workflows/, and the zizmor scan only runs when a workflow file changes, so it fired here for the first time against a pre-existing, repo-wide condition:main'stest.yamlscores 34 findings (17 high, 8 medium). The check fails on any Medium or higher, so partial fixes would not clear it.unpinned-uses, the only mandatory check. Only 3 of the 15 were lines this branch added.permissions: contents: readandpersist-credentials: falseon the four checkouts. Clearedexcessive-permissionsandartipacked. No job writes to the repo viaGITHUB_TOKEN, and the publish job authenticates to npm withNODE_AUTH_TOKEN; that job has no checkout step and is otherwise unchanged.The 6 remaining
cache-poisoningfindings are pre-existing and already suppressed by the CI config. Fixing them would mean restructuring how the workflow caches around the publish path, which does not belong here.Notes for review
The obvious grep for #765 item 1 does not work, which is why the check looks the way it does. The issue suggests grepping the ESM dist for
require(. The shipped 4.2.5 bundle never writesrequire(: rolldown emittedtypeof require < "u"guards andrequire.apply(this, arguments), and minification renamed__commonJSto a single letter. A literal implementation of item 1 would have passed 4.2.5 clean. The gate matches the barerequireidentifier instead, across every ESM chunk rather than just the entry, since nothing guarantees the build stays single-chunk.On #749 wanting enforcement, not a warning. A textual diff cannot classify additive versus non-additive on its own. So the gate fails on any
.d.tsdelta, and the acknowledgement isnpm run gate:acceptplus a committed baseline, which puts the semver decision in front of a reviewer as a diff. Semantic classification via api-extractor would remove the judgement call but is a much larger project, deliberately not attempted here.Incidental repo changes. An eslint
**/*.mjsoverride, since typescript-eslint switchesno-undefoff for.tsand.tsxbut not for plain.mjs;scripts/added to thelintandformatglobs; a newtest:gatescript; and a repair to thesize-limitconfig, which pointed at TSDX-era filenames the vite build has not emitted for some time, sonpm run sizewas silently checking nothing.Follow-ups, not in this PR
reactandfirebaseinstalled, which would cost the gate its dependency-free property. Item 7 (runtime smoke render against Next App Router and Vite) is the strongest check and the only one that would have caught App Router crash: reactfire 4.2.4/4.2.5 throw "dynamic usage of require is not supported" on the client #759 by observing the crash rather than pattern-matching the artifact, but it is roughly a day and adds real CI minutes and flake surface. Neither is needed to close the two holes that actually shipped.no-cjs-in-esmhas nogate:acceptescape hatch, and matching the barerequireidentifier means it can also match the word inside a string literal or comment. Risk is latent rather than imminent:rxjsESM has zero occurrences, andrxfire's 8 are all in.cjs.jsfiles the ESM bundle never resolves. Fix is stripping strings and comments before matching.needs: [test, verify-package]edge. Making it required is about blocking merges.docs.yamlis still unhardened (2 medium, 2 high, plus 2 unpinned refs) and will surprise whoever edits it next.filesincludessrc, and npm does not exclude a nestednode_modules. A localnpm packwithsrc/nextjs/node_modulespresent produces a 186 MB tarball. Harmless in CI, but a live hazard for feat(nextjs): add firebase-cookie-middleware with security hardening #739: ifsrc/nextjslands as-is, everything under it publishes to npm. Thefilesallowlist probably wants narrowing as part of that packaging decision. The gate's local pack works around it by staging fromgit ls-filesplusdist/.Closes #749
Addresses #765 (items 1, 2, 4, 5)
🤖 Generated with Claude Code