Skip to content

test(platform): guard the convex runtime boundary - #3102

Closed
Israeltheminer wants to merge 1 commit into
mainfrom
feat/convex-runtime-guard
Closed

test(platform): guard the convex runtime boundary#3102
Israeltheminer wants to merge 1 commit into
mainfrom
feat/convex-runtime-guard

Conversation

@Israeltheminer

Copy link
Copy Markdown
Collaborator

A file under convex/ that reaches a Node built-in without declaring 'use node' now fails typecheck, 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 reaches node:fs the push fails:

✘ [ERROR] Could not resolve "node:path"
ERROR Convex deploy failed (exit code: 1)

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 a lib/ barrel, which re-exported a loader, which imported node:fs. Only the first file is under convex/ and only it can carry the directive.

What changed

scripts/check-convex-runtime.ts walks convex/, follows local imports transitively, and reports any file that reaches a node: built-in without the directive — printing the chain:

convex/knowledge/pii_gate.ts reaches node:fs
  via convex/knowledge/pii_gate.ts
    → lib/pii/index.ts
    → lib/pii/data/loader.ts
    → node:fs

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.

  • Type-only imports. TypeScript erases them before bundling. Several files on main do import type { IncomingMessage } from 'node:http' and deploy fine.
  • Test files. Never deployed, and free to use node:fs.
  • Bare package specifiers. A package that pulls Node in through its own dependencies is a real hazard this cannot see. The 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 a zod.ts next 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.

@Israeltheminer

Copy link
Copy Markdown
Collaborator Author

The failure this guards cannot happen on main any more.

The guard exists because Convex bundled every file under convex/ for its V8 runtime, so a helper reaching node:path broke the push. #3107 removed that runtime: there is no convex deploy or convex dev in any package script, and the convex/ files that survive are imported by the Node backend, where a Node built-in is ordinary.

The 124 remaining 'use node' pragmas are vestigial for the same reason.

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 convex/ — and that would be worth its own issue rather than a rebase of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant