Skip to content

feat: separate Ballerina LS download from build process - #1999

Closed
kanushka wants to merge 18 commits into
wso2:mainfrom
kanushka:separate-ls-release-setup
Closed

feat: separate Ballerina LS download from build process#1999
kanushka wants to merge 18 commits into
wso2:mainfrom
kanushka:separate-ls-release-setup

Conversation

@kanushka

@kanushka kanushka commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Decouples the Ballerina language server download from the build process, allowing pipelines to pin specific LS versions per branch via a config file.

Changes

New files

  • ls-version.properties — Pipeline config to pin LS version per branch (ls.tag=latest)
  • scripts/verify-ls.js — Fail-fast check: build aborts if LS JAR is missing from ls/
  • DEVELOPMENT_SETUP.md — Developer guide for local setup

Modified files

  • scripts/download-ls.js — Added --tag flag for unified version selection (latest/prerelease/specific-tag), extracted getAuthHeader(), version-aware checkExistingJar
  • package.json — Removed download-ls from postbuild, added verify-ls to build/rebuild/package scripts
  • .github/actions/build/action.yml — Added ballerinaLSTag input, reads ls-version.properties and downloads LS before build
  • .github/workflows/daily-build-ballerina.yml — Added LS download steps reading from ls-version.properties
  • .github/workflows/release-vsix.yml — Added ballerinaLSTag input
  • .github/workflows/build.yml — Added ballerinaLSTag input, passes to build action

How it works

  1. Each branch has ls-version.properties pinning the LS version (ls.tag=latest on main, ls.tag=v1.8.0.m1 on release branches)
  2. CI pipelines read this file and download the correct LS before building
  3. Local builds require developers to run download-ls manually — build fails with clear message if LS is missing
  4. The --tag flag unifies version selection: --tag latest, --tag prerelease, or --tag v1.8.0.m1

Relates to #1886

Summary by CodeRabbit

  • New Features

    • Option to override the Ballerina Language Server tag in build/release workflows; download runs only on cache misses and supports specifying exact tags or prereleases.
  • Documentation

    • Added a developer setup guide for the Ballerina extension describing LS handling and test commands.
  • Chores

    • Added a pre-package verification that a bundled Language Server JAR exists and a CI-configurable LS version file.

kanushka and others added 8 commits April 20, 2026 13:18
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…erties

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@kanushka
kanushka requested review from gigara and hevayo as code owners April 20, 2026 07:54
@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8a39a5aa-a70f-40c8-aa33-9091ddd55c59

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds configurable Ballerina Language Server (LS) selection: new workflow/action inputs to override the LS tag, steps to resolve tag (input → properties file → default), cache-keying by resolved tag, conditional download using the resolved tag, and scripts to verify and download specific LS releases.

Changes

Cohort / File(s) Summary
Build Action & Workflows
\.github/actions/build/action.yml, \.github/workflows/build.yml, \.github/workflows/release-vsix.yml, \.github/workflows/daily-build-ballerina.yml
Added ballerinaLSTag inputs; new step to resolve ls tag (prefer input, else read workspaces/ballerina/ballerina-extension/ls-version.properties, default latest); cache key changed to include resolved tag; added conditional download step that runs on cache miss and invokes download with --tag and --replace.
Language Server scripts
workspaces/ballerina/ballerina-extension/scripts/download-ls.js, workspaces/ballerina/ballerina-extension/scripts/verify-ls.js
download-ls.js: added --tag CLI handling, validation vs --prerelease, tag-specific release fetch, checkExistingJar(expectedVersion) signature change, centralized auth header for API/binary downloads, improved fallback and logs. verify-ls.js: new CLI that fails when no bundled LS JAR is found and prints download guidance.
Extension configuration & docs
workspaces/ballerina/ballerina-extension/ls-version.properties, workspaces/ballerina/ballerina-extension/package.json, workspaces/ballerina/ballerina-extension/DEVELOPMENT_SETUP.md
Added ls-version.properties with ls.tag=latest; added verify-ls to package/build/rebuild scripts and removed download-ls from postbuild; added DEVELOPMENT_SETUP.md documenting LS download/verification and CI pinning via ls-version.properties.

Sequence Diagram(s)

sequenceDiagram
  participant Workflow as GitHub Workflow
  participant Action as Build Composite Action
  participant Script as download-ls / verify-ls
  participant GitHubAPI as GitHub Releases API
  participant Cache as Actions Cache

  Workflow->>Action: call with inputs (ballerinaLSTag, token)
  Action->>Action: resolve tag (input → ls-version.properties → default "latest")
  Action->>Cache: restore cache key "ballerina-ls-{tag}"
  Cache-->>Action: hit/miss
  alt cache miss
    Action->>Script: run download-ls --tag "{tag}" --replace (GITHUB_TOKEN/CHOREO_BOT_TOKEN)
    Script->>GitHubAPI: request release/assets for tag
    GitHubAPI-->>Script: return release assets
    Script->>Action: place LS jar into ls/ directory
    Action->>Cache: save cache "ballerina-ls-{tag}"
  end
  Action->>Script: run verify-ls (ensure jar exists)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hop to read the tag and keep it neat,
I check the jars and make the downloads fleet,
If cache is empty, I fetch the chosen tune,
A little rabbit helper, dancing under the moon. 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description deviates significantly from the template. It provides a summary of changes but is missing critical sections like Purpose (linked issues), Goals, Approach, User Stories, Release Notes, Documentation, Training, Certification, Marketing, Testing, Security Checks, and Migrations. Add missing template sections to align with repository standards, particularly Purpose (with #1886 link), Goals, Approach, Release Notes, Documentation impact, and Testing details.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: separating Ballerina LS download from the build process, which is the core objective of this PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
.github/actions/build/action.yml (1)

186-222: ⚠️ Potential issue | 🟠 Major

Ballerina LS cache restore is effectively neutralized by --replace.

The Restore Ballerina LS from cache step at lines 186-193 populates workspaces/ballerina/ballerina-extension/ls, but the new Download Ballerina LS step always runs (when ballerina/bi is true) and passes --replace, which wipes that directory and re-downloads unconditionally. The cache save/restore becomes dead I/O and any hit is discarded before use.

Consider one of:

  • Gate the download step on steps.ballerina-ls-cache.outputs.cache-hit != 'true' and drop --replace (let checkExistingJar skip when the version already matches).
  • Or remove the now-redundant LS cache step if the intent is to always fetch a pinned version.
Option A: skip download on cache hit
-    - name: Download Ballerina LS
-      if: ${{ inputs.ballerina == 'true' || inputs.bi == 'true' }}
+    - name: Download Ballerina LS
+      if: ${{ (inputs.ballerina == 'true' || inputs.bi == 'true') && steps.ballerina-ls-cache.outputs.cache-hit != 'true' }}
       shell: bash
       run: |
-        pnpm --dir workspaces/ballerina/ballerina-extension run download-ls -- --tag "$LS_TAG" --replace
+        pnpm --dir workspaces/ballerina/ballerina-extension run download-ls -- --tag "$LS_TAG" --replace
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/actions/build/action.yml around lines 186 - 222, The cache restore
step (id: ballerina-ls-cache) is negated by the downloader always using
--replace (pnpm --dir ... run download-ls), so either gate the download on a
cache miss or remove the cache step; update the Download Ballerina LS step to
only run when ballerina/bi are true AND
steps.ballerina-ls-cache.outputs.cache-hit != 'true' and remove the --replace
flag from the pnpm download invocation (so checkExistingJar can skip when
versions match), or alternatively delete the Restore Ballerina LS step entirely
if you intend to always fetch.
workspaces/ballerina/ballerina-extension/scripts/download-ls.js (1)

185-224: ⚠️ Potential issue | 🟠 Major

--tag latest silently becomes prerelease when isPreRelease=true is in the env.

At line 195, effectivePrerelease = (requestedTag === 'prerelease') || usePrerelease, and usePrerelease includes process.env.isPreRelease === 'true' (line 12). So invoking download-ls -- --tag latest in an environment that exports isPreRelease=true (common in release workflows) will pick the latest prerelease instead of the latest stable release — the opposite of what the caller asked for.

An explicit --tag should take precedence over the env-var heuristic:

Proposed fix
-    const effectivePrerelease = (requestedTag === 'prerelease') || usePrerelease;
+    // Explicit --tag wins over the isPreRelease env heuristic.
+    const effectivePrerelease = requestedTag
+        ? requestedTag === 'prerelease'
+        : usePrerelease;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workspaces/ballerina/ballerina-extension/scripts/download-ls.js` around lines
185 - 224, In getLatestRelease, effectivePrerelease currently allows the
env-driven usePrerelease to override an explicit --tag latest; change the
calculation so an explicit requestedTag wins: set effectivePrerelease =
(requestedTag === 'prerelease') || (requestedTag == null && usePrerelease) (or
equivalent) so usePrerelease is only considered when no explicit tag was
provided; adjust the logic around requestedTag and effectivePrerelease
accordingly to ensure --tag latest always yields the stable release.
🧹 Nitpick comments (3)
workspaces/ballerina/ballerina-extension/scripts/download-ls.js (1)

228-241: Order the --tag validations before using requestedTag.

The "missing value for --tag" guard at line 232 runs after the conflict check at line 228, which already reads requestedTag. It works today (undefined short-circuits the &&), but reversing the order makes the intent obvious and is robust to future edits:

Proposed tweak
-        if (requestedTag && requestedTag !== 'latest' && requestedTag !== 'prerelease' && usePrerelease) {
-            throw new Error('Use either --prerelease or --tag <specific-tag>, not both');
-        }
-
-        if (tagIndex >= 0 && !requestedTag) {
-            throw new Error('Missing value for --tag');
-        }
+        if (tagIndex >= 0 && !requestedTag) {
+            throw new Error('Missing value for --tag');
+        }
+
+        if (args.includes('--prerelease') && requestedTag && requestedTag !== 'latest' && requestedTag !== 'prerelease') {
+            throw new Error('Use either --prerelease or --tag <specific-tag>, not both');
+        }

Also note: the current conflict check uses usePrerelease, which is true when isPreRelease=true is merely present in the env. That means --tag v1.8.0.m1 run under such an env throws, even though the user didn't pass --prerelease. Gating the conflict on args.includes('--prerelease') (as above) avoids that false positive.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workspaces/ballerina/ballerina-extension/scripts/download-ls.js` around lines
228 - 241, Reorder the --tag validation so the "Missing value for --tag" guard
(checking tagIndex and requestedTag) runs before any checks that read
requestedTag; specifically move the if (tagIndex >= 0 && !requestedTag) check
above the conflict check that references requestedTag. Also change the conflict
detection to use the actual CLI args flag (e.g. args.includes('--prerelease'))
instead of usePrerelease so it only errors when the user supplied --prerelease,
and keep getExpectedVersion, requestedTag, tagIndex and usePrerelease as the
referenced symbols to locate where to adjust the logic.
.github/workflows/daily-build-ballerina.yml (1)

48-64: Minor: handle empty/missing ls.tag value.

If ls-version.properties exists but the ls.tag= key is missing/commented/empty, TAG becomes an empty string and downstream receives --tag "", which download-ls.js#getTagValue silently treats as no tag. Consider defaulting to latest when the grep result is empty:

Proposed tweak
-          if [ -f "$LS_PROPS" ]; then
-            TAG=$(grep '^ls\.tag=' "$LS_PROPS" | cut -d'=' -f2 | tr -d '[:space:]')
-            echo "tag=$TAG" >> $GITHUB_OUTPUT
-            echo "Using LS tag from ls-version.properties: $TAG"
-          else
-            echo "tag=latest" >> $GITHUB_OUTPUT
-            echo "ls-version.properties not found, defaulting to latest"
-          fi
+          TAG=""
+          if [ -f "$LS_PROPS" ]; then
+            TAG=$(grep '^ls\.tag=' "$LS_PROPS" | cut -d'=' -f2 | tr -d '[:space:]')
+          fi
+          if [ -z "$TAG" ]; then
+            TAG="latest"
+            echo "No ls.tag resolved; defaulting to latest"
+          fi
+          echo "tag=$TAG" >> $GITHUB_OUTPUT
+          echo "Using LS tag: $TAG"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/daily-build-ballerina.yml around lines 48 - 64, The step
that reads ls-version.properties may produce an empty TAG when ls.tag is missing
or blank; update the block that sets TAG (using LS_PROPS/grep) to check if TAG
is empty (e.g., test -z "$TAG") and if so set TAG="latest" and log that default
is used, then write that final TAG to $GITHUB_OUTPUT so downstream uses latest;
reference the LS_PROPS/TAG variables and the steps.ls-version.outputs.tag and
ensure this prevents passing an empty --tag into download-ls
(download-ls.js#getTagValue).
.github/actions/build/action.yml (1)

195-213: Minor: same empty-value edge case as in the daily workflow.

If ls-version.properties exists without a valid ls.tag= line, TAG resolves to "" and --tag "" is forwarded. Worth defaulting to latest when the grep result is empty to preserve the documented fallback.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/actions/build/action.yml around lines 195 - 213, The bal-ls-version
step can produce an empty TAG when ls-version.properties exists but contains no
valid ls.tag entry; update the logic after extracting TAG (variable TAG) to
treat empty values as "latest" (e.g., if [ -z "$TAG" ]; then TAG="latest"; fi or
use parameter expansion TAG=${TAG:-latest}) and ensure the echo "tag=$TAG" and
the user-facing message reflect this fallback so --tag never receives an empty
string; reference the step id bal-ls-version and variables LS_PROPS,
OVERRIDE_TAG, and TAG when locating where to add the check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In @.github/actions/build/action.yml:
- Around line 186-222: The cache restore step (id: ballerina-ls-cache) is
negated by the downloader always using --replace (pnpm --dir ... run
download-ls), so either gate the download on a cache miss or remove the cache
step; update the Download Ballerina LS step to only run when ballerina/bi are
true AND steps.ballerina-ls-cache.outputs.cache-hit != 'true' and remove the
--replace flag from the pnpm download invocation (so checkExistingJar can skip
when versions match), or alternatively delete the Restore Ballerina LS step
entirely if you intend to always fetch.

In `@workspaces/ballerina/ballerina-extension/scripts/download-ls.js`:
- Around line 185-224: In getLatestRelease, effectivePrerelease currently allows
the env-driven usePrerelease to override an explicit --tag latest; change the
calculation so an explicit requestedTag wins: set effectivePrerelease =
(requestedTag === 'prerelease') || (requestedTag == null && usePrerelease) (or
equivalent) so usePrerelease is only considered when no explicit tag was
provided; adjust the logic around requestedTag and effectivePrerelease
accordingly to ensure --tag latest always yields the stable release.

---

Nitpick comments:
In @.github/actions/build/action.yml:
- Around line 195-213: The bal-ls-version step can produce an empty TAG when
ls-version.properties exists but contains no valid ls.tag entry; update the
logic after extracting TAG (variable TAG) to treat empty values as "latest"
(e.g., if [ -z "$TAG" ]; then TAG="latest"; fi or use parameter expansion
TAG=${TAG:-latest}) and ensure the echo "tag=$TAG" and the user-facing message
reflect this fallback so --tag never receives an empty string; reference the
step id bal-ls-version and variables LS_PROPS, OVERRIDE_TAG, and TAG when
locating where to add the check.

In @.github/workflows/daily-build-ballerina.yml:
- Around line 48-64: The step that reads ls-version.properties may produce an
empty TAG when ls.tag is missing or blank; update the block that sets TAG (using
LS_PROPS/grep) to check if TAG is empty (e.g., test -z "$TAG") and if so set
TAG="latest" and log that default is used, then write that final TAG to
$GITHUB_OUTPUT so downstream uses latest; reference the LS_PROPS/TAG variables
and the steps.ls-version.outputs.tag and ensure this prevents passing an empty
--tag into download-ls (download-ls.js#getTagValue).

In `@workspaces/ballerina/ballerina-extension/scripts/download-ls.js`:
- Around line 228-241: Reorder the --tag validation so the "Missing value for
--tag" guard (checking tagIndex and requestedTag) runs before any checks that
read requestedTag; specifically move the if (tagIndex >= 0 && !requestedTag)
check above the conflict check that references requestedTag. Also change the
conflict detection to use the actual CLI args flag (e.g.
args.includes('--prerelease')) instead of usePrerelease so it only errors when
the user supplied --prerelease, and keep getExpectedVersion, requestedTag,
tagIndex and usePrerelease as the referenced symbols to locate where to adjust
the logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9f9598ac-a302-4a35-b1f1-51cdbb6ee456

📥 Commits

Reviewing files that changed from the base of the PR and between 65fa90b and 733a060.

📒 Files selected for processing (9)
  • .github/actions/build/action.yml
  • .github/workflows/build.yml
  • .github/workflows/daily-build-ballerina.yml
  • .github/workflows/release-vsix.yml
  • workspaces/ballerina/ballerina-extension/DEVELOPMENT_SETUP.md
  • workspaces/ballerina/ballerina-extension/ls-version.properties
  • workspaces/ballerina/ballerina-extension/package.json
  • workspaces/ballerina/ballerina-extension/scripts/download-ls.js
  • workspaces/ballerina/ballerina-extension/scripts/verify-ls.js

- Remove ballerina/bi flag guard on LS download (needed for full builds)
- Make LS cache key version-aware (ballerina-ls-{tag})
- Skip download on cache hit instead of always using --replace
- Validate empty ls.tag with clear error message
- Reorder steps: read version → restore cache → conditional download

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/actions/build/action.yml:
- Around line 186-219: The cache key uses the floating tag from the
bal-ls-version step (steps.bal-ls-version.outputs.tag) which allows "latest" or
"prerelease" to be cached; change the workflow so the bal-ls-version step
resolves floating aliases to a concrete immutable tag or sets a flag when the
tag is floating, then use that in the ballerina-ls-cache step (id:
ballerina-ls-cache) to either (a) include the resolved immutable identifier in
the cache key or (b) skip caching/restore when the tag is floating (e.g., if tag
== "latest" or "prerelease" do not run actions/cache). Ensure the resolved tag
or floating-flag is emitted from bal-ls-version via GITHUB_OUTPUT so
ballerina-ls-cache can make the conditional decision.

In @.github/workflows/daily-build-ballerina.yml:
- Around line 48-66: The workflow currently interpolates the ls-version output
directly into the shell command which can expose the shell to metacharacters;
modify the "Read Ballerina LS version" step (ls-version) to validate TAG for
emptiness and allowed characters before echoing to GITHUB_OUTPUT, and change the
"Download Ballerina LS" step to pass the tag via an environment variable (e.g.,
env: TAG: ${{ steps.ls-version.outputs.tag }}) and reference it inside the run
script as "$TAG" (or use the env var in pnpm arguments) so the value is treated
as data not shell syntax; ensure you still set GITHUB_OUTPUT from the validated
TAG variable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bee876c5-b0fe-4bba-9119-7b5962d511b8

📥 Commits

Reviewing files that changed from the base of the PR and between 733a060 and 3c34db4.

📒 Files selected for processing (2)
  • .github/actions/build/action.yml
  • .github/workflows/daily-build-ballerina.yml

Comment thread .github/actions/build/action.yml Outdated
Comment thread .github/workflows/daily-build-ballerina.yml Outdated
When /releases/latest returns 404 (no stable releases), fall back to
fetching the most recent release including prereleases.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
workspaces/ballerina/ballerina-extension/scripts/download-ls.js (1)

252-267: ⚠️ Potential issue | 🟡 Minor

Stale JAR not removed when switching to a different pinned --tag without --replace.

When checkExistingJar(expectedVersion) returns false because the existing ls/ballerina-language-server-<old>.jar doesn’t match the requested version, the code proceeds to download the new JAR without removing the old one (the directory is only wiped when forceReplace is set at L261-264). The result is two ballerina-language-server-*.jar files coexisting in ls/.

verify-ls.js (glob match on ballerina-language-server*.jar) and checkExistingJar without expectedVersion will still report success, and the extension/packaging pipeline may then pick up the wrong JAR non-deterministically (e.g., readdirSync order, first/last match). CI is protected because it invokes with --replace, but local devs running download-ls --tag <new-version> after a previous download will hit this.

Consider pruning stale ballerina-language-server-*.jar files whenever a version mismatch is detected, or making --tag <specific> imply a cleanup of non-matching JARs.

♻️ Proposed fix
         if (!forceReplace && checkExistingJar(getExpectedVersion(requestedTag))) {
             process.exit(0);
         }

+        // Remove stale JARs so only the newly downloaded version remains in ls/.
+        if (!forceReplace && fs.existsSync(LS_DIR)) {
+            for (const file of fs.readdirSync(LS_DIR)) {
+                if (file.startsWith('ballerina-language-server-') && file.endsWith('.jar')) {
+                    fs.rmSync(path.join(LS_DIR, file), { force: true });
+                }
+            }
+        }
+
         const releaseLabel = requestedTag
             ? ` (tag: ${requestedTag})`
             : (usePrerelease ? ' (prerelease)' : '');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workspaces/ballerina/ballerina-extension/scripts/download-ls.js` around lines
252 - 267, The current flow leaves old ballerin­a-language-server-*.jar files in
LS_DIR when checkExistingJar(getExpectedVersion(requestedTag)) returns false and
forceReplace is not set; modify the logic around that check (functions/vars:
checkExistingJar, getExpectedVersion, requestedTag, LS_DIR, forceReplace) to
detect a version mismatch and prune stale JARs before downloading: list files in
LS_DIR matching /ballerina-language-server.*\.jar/, remove any that do not match
the expected filename for getExpectedVersion(requestedTag) (or remove all when
requestedTag is set and no match), then proceed to download; ensure this cleanup
happens only when a mismatched pinned --tag is provided or when expectedVersion
is known so CI behavior with --replace is preserved.
🧹 Nitpick comments (1)
workspaces/ballerina/ballerina-extension/scripts/download-ls.js (1)

186-193: Minor: dedupe the “specific tag” predicate.

requestedTag && requestedTag !== 'latest' && requestedTag !== 'prerelease' is repeated in getLatestRelease (L186), in main()’s validation (L237), and inverted in getExpectedVersion (L246). A tiny helper keeps these in sync and makes intent clearer:

const isSpecificTag = (tag) => Boolean(tag) && tag !== 'latest' && tag !== 'prerelease';

Non-blocking.

Also applies to: 245-250

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workspaces/ballerina/ballerina-extension/scripts/download-ls.js` around lines
186 - 193, Introduce a small predicate helper isSpecificTag(tag) that returns
true when tag is non-empty and not 'latest' or 'prerelease', then replace the
repeated expressions in getLatestRelease, main()'s validation, and
getExpectedVersion with calls to isSpecificTag to deduplicate logic and clarify
intent; update any inverted checks accordingly so semantics remain the same (use
!isSpecificTag(...) where previous code tested the inverse).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@workspaces/ballerina/ballerina-extension/scripts/download-ls.js`:
- Around line 195-197: The code currently sets effectivePrerelease =
(requestedTag === 'prerelease') || usePrerelease which lets the isPreRelease env
var override an explicit --tag value; change the logic so an explicit --tag
argument takes precedence: compute effectivePrerelease as true only when
requestedTag === 'prerelease', otherwise if no explicit --tag was passed fall
back to usePrerelease (e.g., treat requestedTag being undefined/null as the
condition). Also add a warning log when usePrerelease (env) is true but an
explicit non-'prerelease' requestedTag was provided to surface the conflict;
update the checks that validate combos (the existing validation around
requestedTag) accordingly.

---

Outside diff comments:
In `@workspaces/ballerina/ballerina-extension/scripts/download-ls.js`:
- Around line 252-267: The current flow leaves old
ballerin­a-language-server-*.jar files in LS_DIR when
checkExistingJar(getExpectedVersion(requestedTag)) returns false and
forceReplace is not set; modify the logic around that check (functions/vars:
checkExistingJar, getExpectedVersion, requestedTag, LS_DIR, forceReplace) to
detect a version mismatch and prune stale JARs before downloading: list files in
LS_DIR matching /ballerina-language-server.*\.jar/, remove any that do not match
the expected filename for getExpectedVersion(requestedTag) (or remove all when
requestedTag is set and no match), then proceed to download; ensure this cleanup
happens only when a mismatched pinned --tag is provided or when expectedVersion
is known so CI behavior with --replace is preserved.

---

Nitpick comments:
In `@workspaces/ballerina/ballerina-extension/scripts/download-ls.js`:
- Around line 186-193: Introduce a small predicate helper isSpecificTag(tag)
that returns true when tag is non-empty and not 'latest' or 'prerelease', then
replace the repeated expressions in getLatestRelease, main()'s validation, and
getExpectedVersion with calls to isSpecificTag to deduplicate logic and clarify
intent; update any inverted checks accordingly so semantics remain the same (use
!isSpecificTag(...) where previous code tested the inverse).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 45df0da8-acd3-4754-b2b6-dcb5291a1d0c

📥 Commits

Reviewing files that changed from the base of the PR and between 3c34db4 and 627f22e.

📒 Files selected for processing (1)
  • workspaces/ballerina/ballerina-extension/scripts/download-ls.js

Comment thread workspaces/ballerina/ballerina-extension/scripts/download-ls.js Outdated
kanushka and others added 4 commits April 20, 2026 20:20
…egrator.branch

- Rename config file to release.properties (now holds more than LS version)
- Add integrator.branch property for product-integrator PR target branch
- UpdateProductIntegrator job skipped when integrator.branch is empty
- Replace hardcoded 5.0.x with configurable branch from release.properties
- Update all references in action.yml, workflows, and docs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Build both release/bi-1.8.x and release/bi-1.9.x in parallel
- Manual trigger can pick a specific branch or build all
- Consolidated into single matrix job (build + release + integrator PR)
- Each branch reads its own release.properties for LS tag and integrator branch
- Notifications include branch name for clarity

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Branches: release/bi-1.8.x (5.9.x) and main (5.10.x)
- Job name: 'Ballerina daily build - <branch>'

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- ls.tag=v1.8.0.m2 (pin to specific LS release)
- integrator.branch=5.1.x (auto-PR to product-integrator 5.1.x)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kanushka and others added 3 commits April 20, 2026 20:43
- Don't cache floating LS aliases ('latest'/'prerelease') under static keys
  * Add 'cacheable' flag from bal-ls-version step
  * Skip cache restore/save for floating tags (they resolve differently over time)

- Avoid shell-interpolating the resolved LS tag
  * Validate TAG and integrator.branch against allowed characters before GITHUB_OUTPUT
  * Pass tag via LS_TAG env var instead of direct shell substitution

- Fix --tag env override issue in download-ls.js
  * Explicit --tag now takes precedence over isPreRelease env var
  * Add warning when env var conflicts with explicit --tag argument

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
For floating tags (latest/prerelease), fetch release info first to resolve
to concrete tag, then check if that specific version exists in cache.

Previously, checkExistingJar(undefined) would skip download if ANY JAR existed,
even if the concrete tag changed. Now:
- Concrete tags: quick check before fetching release info
- Floating tags: fetch first to get concrete tag, then version-specific check

This ensures we always get the correct version when using floating tags.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- After downloading a new JAR, delete any other ballerina-language-server-*.jar
  files in ls/ to prevent stale JARs being picked up at runtime
- Add null guard for releaseData.tag_name to avoid raw TypeError on malformed API responses
- Note: CI is unaffected as it uses --replace which already clears ls/ before download

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@kanushka
kanushka marked this pull request as draft April 21, 2026 05:02
- Remove isPreRelease/--prerelease from download-ls.js; tag resolution
  is now: --tag flag > BALLERINA_LS_TAG env var > default (latest)
- Restore download-ls in postbuild; remove verify-ls from build/rebuild
- Fix download-ls:prerelease to use --tag prerelease --replace
- Remove separate LS download/cache steps from action.yml; set
  BALLERINA_LS_TAG env var on Build repo step from bal-ls-version output
- Remove enableLSCache input from action.yml and callers (build.yml)
- Remove separate Download Ballerina LS step from daily-build-ballerina.yml;
  set BALLERINA_LS_TAG on rush build step instead

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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