test(platform): guard the convex runtime boundary - #3102
Closed
Israeltheminer wants to merge 1 commit into
Closed
Conversation
Collaborator
Author
|
The failure this guards cannot happen on The guard exists because Convex bundled every file under The 124 remaining Recommend closing unless the guard is meant to cover something else. If a boundary still needs enforcing on 0.5, it is a different one — which modules the backend may import from |
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.
A file under
convex/that reaches a Node built-in without declaring'use node'now failstypecheck, with the import chain that gets there.Why
Convex bundles every file under
convex/for its V8 runtime unless the file itself says otherwise — regardless of who imports it. A helper only node-side actions use is still bundled for V8, and if it reachesnode:fsthe push fails:This shipped in #3083 and stopped the app starting. Nothing local caught it: oxfmt,
oxlint --type-aware, repo-wide typecheck, knip, SAST and 75,800 tests were all green, because every one of them runs under Node where the import resolves. It surfaced only after a container build, as all sixteen browser-test shards failing at once — which reads like a flaky suite, not a broken build.The reach is usually indirect. In that case a file under
convex/imported alib/barrel, which re-exported a loader, which importednode:fs. Only the first file is underconvex/and only it can carry the directive.What changed
scripts/check-convex-runtime.tswalksconvex/, follows local imports transitively, and reports any file that reaches anode:built-in without the directive — printing the chain:It runs from
typecheck, so it is already on every PR with no workflow change, and takes 0.7s over 1,586 files.The rule is now in the repo contract beside the other boundaries no linter covers.
Risk
Three things are deliberately not flagged, each because flagging them would make the guard wrong on day one, and a guard that cries wolf gets switched off.
import type { IncomingMessage } from 'node:http'and deploy fine.node:fs.node:prefix is the unambiguous signal; guessing at packages would produce false alarms.The first draft flagged all of main because it missed the first two. That is in the tests now.
Tests
Thirteen cases over real directory trees rather than a mocked filesystem, because what the guard must get right is the walk itself: the directive after a block comment, value versus type imports, a chain that leaves the scanned tree, a test file, a bare specifier, a cycle, and several offenders at once.
Six deliberate breakages, all caught. Two survived the first round: the bare-specifier case used
'zod', which resolves to nothing either way, so the mutation changed nothing — the fixture now puts azod.tsnext to the entry file, so treating the specifier as relative would resolve and report a break that does not exist.Scope
Catches the
node:prefix only. A file reaching Node through a package's own dependencies still fails at deploy, and this will not warn about it.Gate: repo-wide typecheck,
oxlint --type-aware, oxfmt, knip, SAST 0 findings, platform suite 75,825 passing.