chore(release): 0.9.1 - make the health module edge-safe by construction - #20
Merged
Conversation
0.9.0 introduced the one Node API this module has ever had, and it took down admin.agentage.io: every gated route 500'd, in an app that only ever wanted `links` from the shared barrel. The barrel re-exports this module, middleware.ts imports the barrel, and middleware runs in the Edge Runtime. Next detects Node APIs in edge bundles STATICALLY, so the `typeof` guard I wrote around the call was worthless - the check never runs. The module threw at evaluation and the gate failed before any page rendered. performance.timeOrigin gives the same instant to the millisecond (verified: 0ms delta) and is a Web API present in Node, the Edge Runtime and browsers. The module now has no Node APIs beyond process.env, which Next supports in edge bundles and which the whole provenance story rests on. test/edge-safety.test.ts keeps it that way. It is a source-text test on purpose: a behavioural test cannot catch this, because the code works perfectly under Node - the only thing that fails is a bundler reading the source. It scans raw text INCLUDING comments, since tsc emits comments into dist and not every analyzer parses an AST rather than grepping. It also scans dist when one exists, and asserts this module keeps its no-imports property. The guard fired on its first run, on the comment explaining the fix. Reworded rather than exempted: a name that must not appear in the shipped artifact must not appear in prose either. Downstream repos can drop their barrel workarounds after bumping, though dropping the re-export is still better hygiene than relying on this.
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
1 participant
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 the production outage 0.9.0 caused, in the library rather than in each consumer.
What happened
0.9.0 added
process.uptime()- the only Node API this module has ever contained. Every gated route on admin.agentage.io then 500’d, in an app that only ever wantedlinksfrom the shared barrel.The chain: the
@agentage/sharedbarrel re-exports@agentage/observability/health→middleware.tsimports the barrel → middleware runs in the Edge Runtime. Next detects Node APIs in edge bundles statically, so thetypeof process?.uptime === "function"guard I wrote around the call was worthless - the check never runs. The module threw at evaluation and the gate failed before any page rendered.Dropping the re-export fixes the symptom per repo (agentage/admin#46). This fixes the cause, so no future consumer can be poisoned by importing a barrel.
The fix
performance.timeOriginis the same instant to the millisecond - verified 0ms delta against the uptime-based value - and is a Web API present in Node, the Edge Runtime and browsers alike.dist/health.jsnow references exactly oneprocessproperty:process.env, which Next supports in edge bundles and which the entirecommit/buildTimeprovenance story rests on.The guard
test/edge-safety.test.tsis a source-text test, deliberately. A behavioural test cannot catch this class of bug - the code works perfectly under Node. The only thing that fails is a bundler reading the source, so that is what the test reproduces.It scans raw text including comments, because tsc emits comments into
distand not every analyzer that might read this package parses an AST rather than grepping. It also scansdistwhen a build exists, and pins the module’s no-imports property.It fired on its first run - on the comment explaining the fix. I reworded the comment rather than exempting comments: a name that must not appear in the shipped artifact must not appear in prose either.
Verification
npm run verifygreen: 115 tests (15 in the new edge-safety suite), build, dist smoke. Post-build scan ofdist/health.jsshowsprocess.envand nothing else.Consumers should still prefer not re-exporting the kit from an edge-imported barrel - this makes that a hygiene preference rather than an outage.