Download lint worker #521

anderson4j · 2025-06-03T14:14:05Z

Switches so the vscode extension downloads the appropriate linter version when we connect to an older neo4j server. This way, when we use older syntax that is no longer valid on newer servers - ex. call subqueries without a variable scope clause - we won't get linting for it when connected to an older server.

Ex. connected to 5.23

but connected to Aura

This is also what we would get on the main branch connecting to 5.23

To avoid the PR being too massive, we've decided to add this part behind a feature flag before adding the e2e tests

… vscode, (temporarily as local file)

…sion check shows we need it.

…re checks happening by default. Converts dbSchema to fit old language support when using these (note:move functions). Fixes server version check/language version check to loop correctly on returned rows.

changeset-bot · 2025-06-03T14:14:09Z

⚠️ No Changeset found

Latest commit: 342b9d8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

anderson4j · 2025-06-04T13:01:15Z

packages/query-tools/src/queries/version.ts

@@ -17,11 +17,12 @@ export function getVersion(): ExecuteQueryArgs<{
      return { name, versions };
    },
    collect(rows, summary) {
-      rows.forEach((row) => {
+      for (const row of rows) {


The old approach (also mine) was apparently bugged. Turns out return in forEach returns you from that cycle of the foreach, not the outer function, unlike this approach with for(... of ...)

anderson4j · 2025-06-04T13:06:54Z

packages/vscode-extension/src/extension.ts

@@ -48,7 +48,7 @@ export async function activate(context: ExtensionContext) {
    debug: {
      module: debugServer,
      transport: TransportKind.ipc,
-      options: { env: { CYPHER_25: 'true' } },
+      options: { env: { CYPHER_25: 'false' } }, //TODO: Do we even need this feature flag at all?


When this was true by default we would always get supportsCypherAnnotation=true and try polling functions/procedures with CYPHER annotation -> break polling on old servers.

anderson4j · 2025-06-05T13:04:54Z

pnpm-workspace.yaml

 catalog:
-  'neo4j-driver': '5.12.0'
+  neo4j-driver: 5.12.0
+registry: http://localhost:4873/


TODO: Remove this when we're happy to merge under feature flag

anderson4j · 2025-06-05T13:28:17Z

packages/vscode-extension/src/extension.ts

@@ -48,7 +48,7 @@ export async function activate(context: ExtensionContext) {
    debug: {
      module: debugServer,
      transport: TransportKind.ipc,
-      options: { env: { CYPHER_25: 'true' } },
+      options: { env: { CYPHER_25: 'false' } },


When it was true by default the function/procedure polling always ran with CYPHER 5/CYPHER 25 prefixes, breaking on old server versions.

Is this correct? There are tests that rely on us polling cypher 25 structures:

Completions depends on the Cypher version

Signature help depends on the Cypher version

For the old versions to work in the playground this switch was needed, but yeah the cypher 25 support is not tested in the playground then. I think it makes more sense to keep it this way, and switch this value when wanting to test cypher 25 features. This way is after all the behaviour that users will see when running the extension.

Looks like tests pass anyway, since the runners set the environment variable as well. As a broader question, I think it makes sense to change tests to only use a feature-flag for the specific cases where we are "future-proofing" how CYPHER 25 would work. Otherwise we might miss bugs that are circumvented by the featureflag in checks like:

const supportsCypherAnnotation = _internalFeatureFlags.cypher25 || serverCypherVersions?.includes('25');

If for example the serverCypherVersion polling is bugged to always return undefined but tests run with the flag, this would not show up in any tests.

supportsCypherAnnotation = true -> behaviour in test is as expected

Without the flag - like when the extension is actually run - we rely on the serverCypherVersions check -> bug shows up

I think that change of how tests work (if I've convinced you that it should be done) should come in another PR though.

ncordon

Good initial stab. It needs changes before merging

ncordon · 2025-06-06T10:36:52Z

packages/lint-worker/src/helpers.ts

+  const availableLinters: Record<string, string> = {
+    '2.0.0-next.20': '2025.4.0', // 29/4 - 2025.04.0=30/4
+    //"2.0.0-next.19": "", // 22/4 - maybe SKIP
+    //"2.0.0-next.18": "", // 7/4  - skip because next release is 2025.04.0
+    '2.0.0-next.17': '2025.3.0', // 25/3 - 2025.03.0=27/3
+    '2.0.0-next.16': '2025.2.0', // 17/2 - 2025.02.0=27/2
+    //"2.0.0-next.15": "", // 10/2 - maybe SKIP
+    '2.0.0-next.14': '2025.1.0', // 4/2  - 2025.01.0=6/2
+    //"2.0.0-next.13": "", // 23/12 2024 - skip, next is 01.0
+    '2.0.0-next.12': '5.26.0', // 13/12 - 5.26(.x)=9/12 (very close to initial 5.26, if after)
+    //"2.0.0-next.11": "", // 13/11 - SKIP
+    //"2.0.0-next.10": "", // 13/11 - SKIP
+    '2.0.0-next.9': '5.25.0', // 28/10 - 5.25 = 31/10
+    '2.0.0-next.8': '5.24.0', // 14/9 - 5.24 = 27/9
+    '2.0.0-next.7': '5.23.0', // 29/7 - 5.23 = 22/8
+    '2.0.0-next.6': '5.20.0', // 3/5 - 5.20 = 23/5
+    '2.0.0-next.5': '5.19.0', // 2/4 - 5.19 = 12/4
+    '2.0.0-next.4': '5.18.0', // 6/3 - 5.18 = 13/3
+    '2.0.0-next.3': '5.17.0', // 7/2 - 5.17 = 23/2
+    '2.0.0-next.2': '5.14.0', // 24/11 2023 - 5.14 = 24/11
+    //"2.0.0-next.1": "",  // 22/11 - SKIP
+    '2.0.0-next.0': '5.13.0', // 25/10 - 5.13 = 23/10
+  };


This could be an array of tuples so you can keep both the key and the value

If this is documentation only for us it should leave in another place?

Also some more thoughts:

We should do a release for the missing versions. For example 5.21.0, 5.22.0

A better approach would be to not maintain a list, we should just contact the npm registry to see what available versions there are, take the approppriate one from that list instead https://registry.npmjs.com/@neo4j-cypher/lint-worker/

About releasing missing versions - are you thinking we should have a lintWorker release for every server release, even if the @neo4j-cypher/language support releases used are the same? If so, I think it wouldn't give any big benefits, and would be confusing for users when we have multiple releases without changes between.

For example for 5.21 - there was no next.x release of @neo4j-cypher/language support between the one released shortly before 5.20 (next.6) and the one released shortly before 5.23 (next.7). So if we only want to depend on next.x releases, 5.21 would have to use the same as either 5.20 or 5.23, making it the same lint-worker. We would then also download this identical copy with a different name, using memory unnecessarily.

So to me the 2 approaches we could go for is
A: Make different lint-worker releases with other versions of the artifacts. 2 ways I can think to do this too:

From canary releases of @neo4j-cypher/language-support. Pro - releases exist. Con - a bit painful to figure out artifact versions, and no guarantee that an update of artifacts was made exactly after a server release.

Backfilling with @neo4j-cypher/language-support releases for specific servers. Pro - guarantee we get artifacts from correct time, probably the most "correct" way. Con - it will take some time to make the releases.

B: Stick to the old method with holes. Server versions that dont have an exact match will default to the following linter release (ex. 5.21 servers would use 5.23 linter). Certain users will get artifacts that are not a perfect match for old servers, but for future releases there will be direct matches (except for patch releases). I was leaning towards this since we're a bit short on time, and using 5.23 linting on 5.21 is not a huge leap, and still should contain new grammar introduced in 5.21.

I like the idea of using versions from the registry, will switch to that right away.

The one thing Im a bit hesistant on is how it would work offline. If you've already downloaded 6 linters and are switching local servers these while offline, it would be nice to be able to switch between existing linters too. Doubt this is too common an occurence though, so maybe we can save a fix for this until later. (Fix-sketch: assembling a new list of "available linters" offline from the file system, and matching like before)

ncordon · 2025-06-06T15:01:44Z

packages/lint-worker/src/helpers.ts

+export async function convertDbSchema(
+  originalSchema: DbSchema,
+  neo4j: Neo4jSchemaPoller,
+): Promise<DbSchema | NoCyphVerSchema | FuncOnlySigSchema> {


Having this as DbSchemaV1, DbSchemaV2, DbSchemaV3 would be better.

The language-server seems a better place to put this in my opinion

ncordon · 2025-06-06T15:18:28Z

packages/query-tools/src/schemaPoller.ts

+  public driver?: Driver;
+  public serverVersion?: string;


This is a code smell, neither of these should be public. Probably the serverVersion should be polled with the rest of the queries that are triggered against the database only once

Moved serverVersion to be a property of the connection, and set it like you suggest

ncordon · 2025-06-06T15:24:14Z

packages/vscode-extension/src/connectionService.ts

+  //TODO: Set this to proper npm registry when package is published.
+  const downloadUrl = `http://localhost:4873/@neo4j-cypher/lint-worker/-/lint-worker-${serverVersion}.tgz`;


Leaving here the url we'll want for fast reference: https://registry.npmjs.org/@neo4j-cypher/lint-worker/-/lint-worker-2025.4.0.tgz

ncordon · 2025-06-06T15:25:27Z

packages/vscode-extension/src/connectionService.ts

+  const filePath = path.join(destDir, fileName);
+  //TODO: Set this to proper npm registry when package is published.
+  const downloadUrl = `http://localhost:4873/@neo4j-cypher/lint-worker/-/lint-worker-${serverVersion}.tgz`;
+  const response = await axios.get(downloadUrl, { responseType: 'stream' });


Good choice for an http client 👍. It says it's both usable in the browser and in nodejs

ncordon · 2025-06-06T16:02:00Z

packages/language-support/src/featureFlags.ts

@@ -1,6 +1,7 @@
 interface FeatureFlags {
  consoleCommands: boolean;
  cypher25: boolean;
+  versionedLinters: boolean;


This isn't needed here, it should be in the Neo4jSettings / features in the language server

ncordon · 2025-06-06T16:05:12Z

packages/language-server/src/server.ts

@@ -127,9 +126,22 @@ connection.onSignatureHelp(doSignatureHelp(documents, neo4jSchemaPoller));
 // Trigger the auto completion
 connection.onCompletion(doAutoCompletion(documents, neo4jSchemaPoller));

+connection.onNotification(
+  'updateLintWorker',
+  (connectionSettings: Neo4jConnectionSettings) => {


You shouldn't reuse this type of message, it should be kept for connections only, we should create a linter specific structure:

export type LintWorkerSettings = { lintWorkerPath: string }

ncordon · 2025-06-06T16:05:31Z

packages/language-server/src/server.ts

+  'updateLintWorker',
+  (connectionSettings: Neo4jConnectionSettings) => {
+    const lintWorkerPath = connectionSettings.lintWorkerPath;
+    _internalFeatureFlags.versionedLinters = true;


This is not needed

ncordon · 2025-06-06T16:05:37Z

packages/language-server/src/server.ts

 connection.onNotification(
  'connectionUpdated',
  (connectionSettings: Neo4jConnectionSettings) => {
+    _internalFeatureFlags.versionedLinters = false;


ncordon · 2025-06-06T16:07:56Z

packages/language-server/src/linting.ts

@@ -32,9 +50,13 @@ async function rawLintDocument(
    }

    const proxyWorker = (await pool.proxy()) as unknown as LintWorker;
+
+    const fixedDbSchema = _internalFeatureFlags.versionedLinters


The feature flags should be accessed through the corresponding data structure and propagated here, see this snippet from server.ts to see how the linting feature flag is accessed:

async function lintSingleDocument(document: TextDocument): Promise<void> { if (settings?.features?.linting) { return lintDocument(

anderson4j · 2025-06-11T09:09:07Z

packages/lint-worker/src/helpers.ts

+  const serverVersion = await getServerVersion(neo4j);
+  const linterVersion = serverVersionToLinter(serverVersion);
+
+  if (compareMajorMinorVersions(linterVersion, '5.18.0') < 0) {


though the schema looked differently here (in lang-supp next.3 and earlier), we still get by when passing our old object, since the only fields used are the same. The corresponding lintCypherQuery didnt use functions/procedures anyway, so we can remove this.

(now that lintWorker takes in DbSchema as any, typecheck doesnt complain either)

anderson4j added 4 commits June 3, 2025 16:04

adds version comparison logic, and allows switching of linter used in…

818d783

… vscode, (temporarily as local file)

adds downloading and unpacking lintworker - calls these if server ver…

f663725

…sion check shows we need it.

merge main

61d4949

sets env.CYPHER_25 to false to prevent version-based function/procedu…

b033984

…re checks happening by default. Converts dbSchema to fit old language support when using these (note:move functions). Fixes server version check/language version check to loop correctly on returned rows.

anderson4j added 4 commits June 3, 2025 16:15

removes needless formatting test workaround, updates lockfile

8635dd3

adds a feature flag to toggle using the new feature or not

4f4177e

creates lintworker helpers file

9b009a7

clean up some todos

6f06dfe

anderson4j commented Jun 4, 2025

View reviewed changes

remove 1 more todo after thinking on it

d3efbfc

anderson4j commented Jun 5, 2025

View reviewed changes

more self review

342b9d8

ncordon requested changes Jun 6, 2025

View reviewed changes

anderson4j commented Jun 11, 2025

View reviewed changes

		//TODO: Set this to proper npm registry when package is published.
		const downloadUrl = `http://localhost:4873/@neo4j-cypher/lint-worker/-/lint-worker-${serverVersion}.tgz`;

Download lint worker #521

Are you sure you want to change the base?

Download lint worker #521

Uh oh!

Conversation

anderson4j commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

anderson4j Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anderson4j Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anderson4j Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ncordon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anderson4j Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anderson4j commented Jun 3, 2025 •

edited

Loading

changeset-bot bot commented Jun 3, 2025 •

edited

Loading

anderson4j Jun 4, 2025 •

edited

Loading

anderson4j Jun 4, 2025 •

edited

Loading

anderson4j Jun 9, 2025 •

edited

Loading

anderson4j Jun 9, 2025 •

edited

Loading