Skip to content

fix(server): unpin the yanked lint version breaking the vendor CDN test #1218

Description

@vivek7405

Problem

main is red and every PR is blocked. The required Unit + integration (node --test) check fails on one test:

✖ jspmGenerate #446: conflicting graph resolves to ONE consistent set (real CDN)
  AssertionError [ERR_ASSERTION]: lint resolves
  at packages/server/test/vendor/vendor.test.js:615

The test hard-pins @codemirror/lint@6.9.6 and asks the LIVE api.jspm.io to resolve it. That version has stopped resolving upstream. Asking the API directly:

$ curl -X POST https://api.jspm.io/generate -H 'content-type: application/json' \
  -d '{"install":["@codemirror/view@6.39.0","@codemirror/lint@6.9.6"],"flattenScope":true,"env":["browser","module","production"]}'

{"error":"Error: Module not found https://ga.jspm.io/npm:@codemirror/lint@6.9.6/dist/index.js imported from about:blank/"}

Neighbouring versions are fine, so this is specific to 6.9.6 rather than a CDN outage:

install result
@codemirror/lint@6.9.6 error: Module not found .../dist/index.js
@codemirror/lint@6.9.0 resolves
@codemirror/lint@6.8.5 resolves
@codemirror/lint@6.8.4 resolves

This is not a flake. It fails identically on every run, and a gh run rerun --failed after confirming api.jspm.io and ga.jspm.io both answered 200 failed the same way. It reproduces locally.

It first appeared on the main runs for 235d0fb0 and 4822e0d1 (2026-08-02), and both are still red. Confirmed unrelated to whatever PR happens to be in flight: PR #1217 (a website/**-only diff) hit it too.

Design / approach

The narrow fix is to bump the pinned version, but that just resets the clock: the test asserts against a live third-party registry, so any pinned version can be yanked or broken upstream at any time and take main down with it. The last such bump would have bought a few months at best.

Two changes, and the second is the one that matters:

1. Repin to a version that resolves (@codemirror/lint@6.9.0 is the nearest). Keeps the fixture realistic: the pair still reproduces the #446 shape, where @codemirror/view is pinned at 6.39.0 and lint transitively wants a newer view.

2. Make the test distinguish "our logic broke" from "upstream changed", and SKIP rather than FAIL on the latter. Right now those two are the same red. The test's ground truth is JSPM's own unified generate response (L620-625), so when that response comes back as an error, the test has no ground truth to compare against and cannot say anything about our merge logic. That case should skip with a clear message, not fail.

Deliberately NOT converting this to a pure mock. Its value is precisely that it compares jspmGenerate() against JSPM's own answer for the same install set, which is an integration guarantee a mock cannot give. The file already has the mocked half of this pair right above it (jspmGenerate #446: multi-install set resolves in ONE generate call (unified, not per-package), L~573, explicitly "deterministic mock, no live CDN"), so the unit-level assertion of our merge logic is already covered and does not need duplicating.

Worth considering while in here: the same fragility applies to every { skip: !NETWORK_OK } test in this file (there are ~18). A shared helper that turns "CDN could not resolve the fixture" into a skip would cover all of them, rather than just the one that happens to be broken today.

Implementation notes (for the implementing agent)

Where to edit

  • packages/server/test/vendor/vendor.test.js, the test at L602-640, jspmGenerate #446: conflicting graph resolves to ONE consistent set (real CDN). The failing assertion is assert.ok(map['@codemirror/lint'], 'lint resolves') at L615. The install pair is at L611. The ground-truth fetch('https://api.jspm.io/generate', ...) starts at L621.
  • The network gate is const NETWORK_OK = !process.env.WEBJS_SKIP_NETWORK_TESTS; at L404, with the explanatory comment at L401. ~18 tests in the file use { skip: !NETWORK_OK }; a shared resolve-or-skip helper would live next to it.
  • The code under test is jspmGenerate() in packages/server/src/vendor.js. It should NOT need changing: this is a broken test fixture, not a framework bug. If you find yourself editing src/, stop and re-read, because that would mean the diagnosis above is wrong.

Landmines / gotchas

  • Do not just delete or permanently skip the test. It is the regression test for dogfood: importmap per-package resolution yields inconsistent dep graph #446 (dogfood: importmap per-package resolution yields inconsistent dep graph, CLOSED), where per-package resolution produced TWO different @codemirror/view URLs merged last-write-wins, so one served entry imported a symbol another lacked. That is a real shipped bug and it needs a live guard.
  • The fixture has to keep the CONFLICT shape to be meaningful: view pinned at an older version, lint transitively requiring a newer one. Picking two packages with no version conflict silently turns this into a test that passes for the wrong reason. Verify the chosen pair actually conflicts before settling on it.
  • WEBJS_SKIP_NETWORK_TESTS=1 skips the whole network group, so it is NOT a way to verify your fix. Run without it.
  • clearVendorCache() is called at L613 before jspmGenerate; keep it, since jspmGenerate has an in-process cache (there is a test for that at L420) and a stale entry would mask the behaviour.
  • CI runs this in the Unit + integration (node --test) job, which is a REQUIRED check on main branch protection (Conventions, Unit + integration, Browser, E2E, Build).

Invariants to respect

  • packages/ is plain .js with JSDoc. Do NOT add a .ts file here (AGENTS.md, "Working in the WebJs framework repo itself").
  • AGENTS.md invariant 11 (no em-dashes, no hyphen/semicolon as pause punctuation, WebJs brand casing) applies to any prose or comment you add.
  • A skip must be LOUD. A silently-skipping test is how a real regression hides; print why it skipped.

Tests + docs surfaces

  • This IS the test layer. The counterfactual is the interesting part: prove the test still FAILS when the merge logic regresses (temporarily make jspmGenerate resolve per-package instead of unified, confirm red, restore), so the repin has not turned it into a test that cannot fail.
  • Also prove the new skip path: point the fixture at a knowingly-unresolvable version (@codemirror/lint@6.9.6 is a ready-made one) and confirm it SKIPS with a clear message rather than failing.
  • No doc surface: this is a test-only change with no public API movement, so the require-docs-with-src gate does not apply (it triggers on packages/*/src).
  • Bun parity: N/A, no runtime-sensitive source touched.

Acceptance criteria

  • node --test packages/server/test/vendor/vendor.test.js passes locally with network available
  • The Unit + integration (node --test) check is green on main again
  • The test SKIPS with a clear message, rather than failing, when the CDN cannot resolve the fixture at all
  • The fixture still expresses the dogfood: importmap per-package resolution yields inconsistent dep graph #446 conflict shape (an older pinned view, a lint that transitively wants a newer one), verified rather than assumed
  • A counterfactual proves the test still fails if jspmGenerate regresses to per-package resolution
  • A counterfactual proves the skip path triggers on an unresolvable fixture
  • packages/server/src/vendor.js is unchanged (or the diagnosis is corrected in the PR body if it genuinely needed a fix)

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions