Migrate workflows to Java 25#44604
Conversation
Thevakumar-Luheerathan
requested review from
Dilhasha,
KavinduZoysa,
MaryamZi,
NipunaRanasinghe,
azinneera,
gimantha,
hasithaa,
keizer619,
sameerajayasoma and
warunalakshitha
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpgrades CI workflows to Temurin JDK 25, migrates Gradle scripts to use provider-based buildDirectory/explicit buildDir, bumps multiple tooling/dependencies and the Gradle wrapper, expands SpotBugs exclusions, adds a JVM startup flag, and adjusts tests, settings, and benchmark/build infra. ChangesComprehensive Toolchain & Build System Upgrade
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
Updates the build and CI configuration to target Java 25, including Gradle/plugin upgrades and build-script adjustments needed for newer Gradle/SpotBugs behavior.
Changes:
- Upgrade Gradle wrapper and update the shared Java convention plugin to compile/target Java 25.
- Update GitHub Actions workflows (Temurin + GraalVM) to run on Java 25.
- Modernize Gradle scripts around
layout.buildDirectory, adjust packaging permissions, and extend SpotBugs exclusions to accommodate new warnings.
Reviewed changes
Copilot reviewed 71 out of 74 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/testerina-integration-test/build.gradle | Use layout.buildDirectory-derived build path. |
| tests/language-server-integration-tests/build.gradle | Use layout.buildDirectory-derived build path. |
| tests/jballerina-integration-test/build.gradle | Use layout.buildDirectory-derived build path. |
| tests/jballerina-debugger-integration-test/src/test/java/org/ballerinalang/debugger/test/adapter/DebugOutputTest.java | Make output assertion order-independent for Java 25 runtime behavior. |
| tests/jballerina-debugger-integration-test/spotbugs-exclude.xml | Add SpotBugs suppressions for new warning patterns. |
| tests/jballerina-debugger-integration-test/build.gradle | Use layout.buildDirectory-derived build path. |
| tests/jballerina-benchmark-test/build.gradle | Refactor benchmark execution into dedicated Exec task; adjust test behavior. |
| tests/ballerina-tools-integration-test/build.gradle | Use layout.buildDirectory-derived build path. |
| tests/ballerina-test-utils/spotbugs-exclude.xml | Add SpotBugs suppression for new throws warning. |
| tests/ballerina-test-utils/build.gradle | Migrate Shadow plugin id to com.gradleup.shadow. |
| tests/bal-tools-integration-tests/build.gradle | Use layout.buildDirectory-derived build path. |
| settings.gradle | Add projectDir.mkdirs() calls for many projects/parents. |
| project-api/test-artifacts/log-creator-pkg-provided-code-modifier/spotbugs-exclude.xml | Add SpotBugs suppression for new throws warning. |
| project-api/test-artifacts/log-creator-pkg-provided-code-generator/spotbugs-exclude.xml | Add SpotBugs suppression for new throws warning. |
| project-api/test-artifacts/log-creator-pkg-provided-code-analyzer/spotbugs-exclude.xml | Add SpotBugs suppression for new throws warning. |
| project-api/test-artifacts/log-creator-in-built-code-modifier/spotbugs-exclude.xml | Add SpotBugs suppression for new throws warning. |
| project-api/test-artifacts/log-creator-in-built-code-generator/spotbugs-exclude.xml | Add SpotBugs suppression for new throws warning. |
| project-api/test-artifacts/log-creator-in-built-code-analyzer/spotbugs-exclude.xml | Add SpotBugs suppression for new throws warning. |
| project-api/test-artifacts/compiler-plugin-with-analyzer-generator-modifier/spotbugs-exclude.xml | Add SpotBugs suppression for new throws warning. |
| project-api/test-artifacts/bal-tool-integration-test-artifacts/build.gradle | Use layout.buildDirectory-derived build path. |
| project-api/test-artifacts/bad-sad-compiler-plugin/spotbugs-exclude.xml | New SpotBugs exclude filter for test artifact. |
| project-api/project-api-test/build.gradle | Use layout.buildDirectory-derived build path. |
| misc/testerina/modules/testerina-core/build.gradle | Move off project.buildDir usages to layout.buildDirectory. |
| misc/maven-resolver/build.gradle | Configure Gradle test task to use TestNG. |
| misc/formatter/modules/formatter-core/build.gradle | Use layout.buildDirectory-derived build path. |
| misc/docerina/build.gradle | Use layout.buildDirectory-derived build path. |
| misc/diagram-util/spotbugs-exclude.xml | Add SpotBugs suppression for new stale-thread-write warnings. |
| misc/diagram-util/build.gradle | Use layout.buildDirectory-derived build path. |
| misc/debug-adapter/modules/debug-adapter-core/spotbugs-exclude.xml | Add SpotBugs suppressions for new throws/stale-thread-write warnings. |
| misc/ballerinalang-data-mapper/src/main/java/org/ballerinalang/datamapper/utils/HttpResponse.java | Javadoc cleanup ({@code int} usage). |
| misc/ballerinalang-data-mapper/build.gradle | Use layout.buildDirectory-derived build path. |
| language-server/modules/langserver-stdlib/build.gradle | Move off project.buildDir usages to layout.buildDirectory. |
| language-server/modules/langserver-core/spotbugs-exclude.xml | Expand SpotBugs suppressions for new warning patterns. |
| language-server/modules/langserver-core/build.gradle | Use layout.buildDirectory; update archives name API usage. |
| language-server/modules/langserver-commons/spotbugs-exclude.xml | Expand module-wide SpotBugs suppressions to include throws warnings. |
| language-server/modules/langserver-commons/build.gradle | Use layout.buildDirectory-derived build path. |
| langlib/lang.test/build.gradle | Move off project.buildDir usages to layout.buildDirectory. |
| gradle/wrapper/gradle-wrapper.properties | Upgrade Gradle wrapper version. |
| gradle/plugins/java-convention/src/main/groovy/javaProjectWithExtBala.gradle | Use layout.buildDirectory-derived build path. |
| gradle/plugins/java-convention/src/main/groovy/javaProject.gradle | Set Java compatibility to 25; set SpotBugs tool version; use layout.buildDirectory. |
| gradle/plugins/java-convention/src/main/groovy/ballerinaStdLibBuildInternal.gradle | Gradle 9 compatibility updates (abstract task class, buildDir usage). |
| gradle/plugins/java-convention/src/main/groovy/ballerinaStdLibBuild.gradle | Gradle 9 compatibility updates (abstract task class, buildDir usage). |
| gradle/plugins/java-convention/src/main/groovy/ballerinaNativeStdLibBuild.gradle | Gradle 9 compatibility updates (abstract task class, buildDir usage). |
| gradle/plugins/java-convention/src/main/groovy/ballerinaLangLibLoad.gradle | Use layout.buildDirectory-derived build path. |
| gradle/plugins/java-convention/src/main/groovy/ballerinaLangLibBuild.gradle | Gradle 9 compatibility updates (abstract task class, buildDir usage). |
| gradle/libs.versions.toml | Bump tool/library/plugin versions (SpotBugs, Shadow, JaCoCo, ASM, TestNG, etc.). |
| distribution/zip/jballerina/build.gradle | Use layout.buildDirectory-derived build path. |
| distribution/zip/jballerina/bin/bal.bat | Add JVM flag for Unsafe memory access behavior under Java 25. |
| distribution/zip/jballerina/bin/bal | Add JVM flag for Unsafe memory access behavior under Java 25. |
| distribution/zip/jballerina-tools/build.gradle | Use layout.buildDirectory; adjust copy permissions DSL for Gradle 9. |
| compiler/ballerina-lang/src/main/java/io/ballerina/projects/util/ProjectConstants.java | Update embedded tool jar names (JaCoCo/ASM) to new versions. |
| compiler/ballerina-lang/spotbugs-exclude.xml | Expand SpotBugs suppression set to include new throws warning. |
| cli/central-client/build.gradle | Use layout.buildDirectory-derived build path. |
| cli/ballerina-cli/src/test/java/io/ballerina/cli/cmd/TestNativeImageCommandTest.java | Loosen assertion to match changed native-image output text. |
| cli/ballerina-cli/spotbugs-exclude.xml | Add SpotBugs suppressions for throws and stale-thread-write warnings. |
| cli/ballerina-cli/build.gradle | Use layout.buildDirectory-derived build path. |
| build.gradle | Replace subproject.buildDir usages with layout.buildDirectory. |
| build-config/checkstyle/build.gradle | Move off buildDir direct usage to layout.buildDirectory. |
| ballerina-shell/modules/shell-core/spotbugs-exclude.xml | Add SpotBugs suppressions for new throws/stale-thread-write warnings. |
| ballerina-shell/modules/shell-cli/src/test/java/io/ballerina/shell/cli/test/base/TestIntegrator.java | Normalize REPL output for Java 25 I/O behavior differences. |
| ballerina-shell/modules/shell-cli/spotbugs-exclude.xml | Add SpotBugs suppressions for new throws warnings. |
| .github/workflows/trivy-scan.yml | Switch workflow JDK to 25. |
| .github/workflows/push_master.yml | Switch workflow JDK to 25; update GraalVM setup inputs. |
| .github/workflows/pull_request_windows_build.yml | Switch workflow JDK to 25; update GraalVM setup inputs. |
| .github/workflows/pull_request_ubuntu_build.yml | Switch workflow JDK to 25; update GraalVM setup inputs. |
| .github/workflows/pull_request_full_build.yml | Switch workflow JDK to 25. |
| .github/workflows/pull_request_debugger_tests.yml | Switch workflow JDK to 25; update GraalVM setup inputs. |
| .github/workflows/publish_timestamped_release.yml | Switch workflow JDK to 25. |
| .github/workflows/publish_release.yml | Switch workflow JDK to 25; update GraalVM setup inputs. |
| .github/workflows/observe_package_push.yaml | Switch workflow JDK to 25. |
| .github/workflows/nightly_publish_timestamped_release.yml | Switch workflow JDK to 25. |
| .github/workflows/daily_spec_conformance_test_runner.yml | Switch workflow JDK to 25. |
| .github/workflows/daily_build.yml | Switch workflow JDK to 25; update GraalVM setup inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| project(':ballerina-tools-api').projectDir = file('compiler/ballerina-tools-api') | ||
| project(':ballerina-tools-api').projectDir.mkdirs() | ||
| project(':ballerina-parser').projectDir = file('compiler/ballerina-parser') | ||
| project(':ballerina-parser').projectDir.mkdirs() |
| <Match> | ||
| <Bug pattern="THROWS_METHOD_THROWS_RUNTIMEEXCEPTION,THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION"/> | ||
| </Match> |
| <Match> | ||
| <Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2"/> | ||
| <Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2,THROWS_METHOD_THROWS_RUNTIMEEXCEPTION,THROWS_METHOD_THROWS_CLAUSE_THROWABLE"/> | ||
| </Match> |
| <Match> | ||
| <Class name="org.ballerinalang.debugger.test.utils.BreakpointEventListener" /> | ||
| <Field name="breakpointEventFound" /> | ||
| <Bug pattern="AT_STALE_THREAD_WRITE_OF_PRIMITIVE" /> | ||
| </Match> |
| <Field name="coverage" /> | ||
| <Bug pattern="AT_STALE_THREAD_WRITE_OF_PRIMITIVE" /> | ||
| </Match> | ||
| <Match> | ||
| <Class name="io.ballerina.cli.task.RunTestsTask" /> |
| <Match> | ||
| <Class name="io.ballerina.cli.task.RunTestsTask" /> | ||
| <Field name="report" /> | ||
| <Bug pattern="AT_STALE_THREAD_WRITE_OF_PRIMITIVE" /> | ||
| </Match> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/daily_build.yml (1)
72-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale step title to match Java version.
Line 72 still says “Set up JDK 21” but Line 76 configures
25.0.3. Please align the step name for accurate workflow logs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/daily_build.yml around lines 72 - 76, Update the step title string "Set up JDK 21" to reflect the configured Java version used by actions/setup-java@v4 (java-version: '25.0.3'); for example rename the step to "Set up JDK 25" or "Set up JDK 25 (Temurin 25.0.3)" so the workflow logs accurately match the java-version setting..github/workflows/push_master.yml (1)
116-116:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale step name to match configured JDK version.
Line 116 says “Set up JDK 21” but this step now configures Java 25.0.3 (Line 120), which makes CI logs misleading.
Suggested change
- - name: Set up JDK 21 + - name: Set up JDK 25🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/push_master.yml at line 116, The workflow step name "Set up JDK 21" is stale and should match the configured Java version; update the step name string to reflect the actual version being installed (e.g., change "Set up JDK 21" to "Set up JDK 25" or "Set up JDK 25.0.3") so the step name matches the configuration that installs Java 25.0.3.
🧹 Nitpick comments (5)
.github/workflows/trivy-scan.yml (1)
16-16: ⚡ Quick winConsider pinning
actions/setup-javato a specific commit SHA for enhanced supply chain security.While not currently enforced in this repository, pinning GitHub Actions to immutable commit SHAs (rather than mutable major version tags like
@v4) is a security best practice that prevents potential tampering. This applies to all actions in the workflow:actions/checkout@v4,actions/setup-java@v4, andactions/cache@v4.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/trivy-scan.yml at line 16, Replace mutable action tags with immutable commit SHAs: update uses entries for actions/setup-java@v4, actions/checkout@v4, and actions/cache@v4 to their corresponding repository commit SHAs (e.g., actions/setup-java@<commit-sha>) to pin the workflow to a specific git commit; fetch the desired SHA from each action's GitHub repository (releases or commit history) and use that full SHA ref in the workflow to ensure supply-chain immutability.misc/testerina/modules/testerina-core/build.gradle (1)
57-57: ⚡ Quick winUse the buildDir variable defined above.
The buildDir variable is defined on Line 22 but not used here. For consistency, use the variable instead of calling the API again.
♻️ Proposed refactor
tasks.register('updateVersion') { doLast { ant.replaceregexp( - file: "${project.layout.buildDirectory.get().asFile}/ballerina-src/Ballerina.toml", + file: "${buildDir}/ballerina-src/Ballerina.toml", match: "${project.name}-(.*).jar",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@misc/testerina/modules/testerina-core/build.gradle` at line 57, Replace the direct project API call with the pre-defined buildDir variable: locate the use of project.layout.buildDirectory.get().asFile in the Ballerina.toml file path (the line that currently reads file: "${project.layout.buildDirectory.get().asFile}/ballerina-src/Ballerina.toml") and update it to use the buildDir variable (e.g., construct the path from buildDir + "/ballerina-src/Ballerina.toml") so the existing buildDir definition is consistently reused.language-server/modules/langserver-stdlib/build.gradle (1)
47-47: ⚡ Quick winDefine buildDir variable for consistency with the migration pattern.
Unlike other build scripts in this PR that define
def buildDir = layout.buildDirectory.get().asFileand reference the variable, this file uses the inline API call directly. For consistency and maintainability across the codebase, consider defining the buildDir variable at the top of the file as done in sibling modules.📋 Proposed refactor for consistency
Add the buildDir definition after the plugins block:
plugins { id 'ballerinaStdLibBuild' } +def buildDir = layout.buildDirectory.get().asFile + dependencies {Then update the usage to reference the variable:
tasks.register('updateVersion') { doLast { ant.replaceregexp( - file: "${project.layout.buildDirectory.get().asFile}/ballerina-src/Ballerina.toml", + file: "${buildDir}/ballerina-src/Ballerina.toml", match: "${project.name}-(.*).jar",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@language-server/modules/langserver-stdlib/build.gradle` at line 47, This file uses inline layout.buildDirectory.get().asFile instead of the shared buildDir variable; add def buildDir = layout.buildDirectory.get().asFile (e.g., immediately after the plugins block) and update usages such as file: "${project.layout.buildDirectory.get().asFile}/ballerina-src/Ballerina.toml" to use file: "${buildDir}/ballerina-src/Ballerina.toml" so the module matches the migration pattern used in sibling build.gradle files.distribution/zip/jballerina/bin/bal.bat (1)
195-195: 💤 Low valueJVM flag addition mirrors Unix script.
This Windows batch script includes the same
--sun-misc-unsafe-memory-access=allowflag as the bash script (bal). The same verification and migration considerations apply.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@distribution/zip/jballerina/bin/bal.bat` at line 195, The bal.bat script currently hardcodes the JVM flag --sun-misc-unsafe-memory-access=allow into CMD_LINE_ARGS; change the CMD_LINE_ARGS assembly logic to mirror the Unix bal script by only appending that flag after verifying the JVM supports it (e.g., run a version/feature check or inspect %JAVA_HOME%\bin\java -XshowSettings or similar) or remove the flag entirely for Windows builds; update the CMD_LINE_ARGS variable construction (the line setting CMD_LINE_ARGS) so it conditionally includes or omits --sun-misc-unsafe-memory-access=allow and document the check in the script comments.distribution/zip/jballerina/bin/bal (1)
222-222: 💤 Low valueConsider using
warnmode as an intermediate step; track which dependencies require this flag for future updates.The
--sun-misc-unsafe-memory-access=allowflag is correctly formatted and valid for Java 25. However,warnmode is available as a less permissive alternative that would still permit execution while surfacing problematic dependencies. Document which dependencies trigger this flag so they can be updated or replaced in future releases to eliminate the need for this bypass.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@distribution/zip/jballerina/bin/bal` at line 222, Replace the permissive flag --sun-misc-unsafe-memory-access=allow with the less-permissive --sun-misc-unsafe-memory-access=warn in the bal startup options so the JVM still runs but surfaces offending modules, and add a short runtime/build-time tracking step (e.g., emit a one-line comment/log or write to a deps file when the JVM warns about sun.misc unsafe access) to record which dependencies trigger the warning so they can be updated/replaced in future releases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/observe_package_push.yaml:
- Line 21: The workflow currently references a mutable tag "uses:
actions/setup-java@v4"; replace this with a fixed commit SHA for the
actions/setup-java action (e.g., "uses: actions/setup-java@<full-commit-sha>")
so the job is pinned and reproducible—update the observe_package_push workflow
to use the specific commit SHA for actions/setup-java instead of the `@v4` tag.
In @.github/workflows/pull_request_full_build.yml:
- Line 23: Replace the mutable tag "uses: actions/setup-java@v4" with the
corresponding full commit SHA for each occurrence so the workflow is pinned and
immutable; update all three spots where "uses: actions/setup-java@v4" appears
(the three separate job steps) to use the full commit SHA string (the exact SHA
for the v4 release) instead of the `@v4` tag, ensuring each job references the
same verified commit SHA.
In @.github/workflows/push_master.yml:
- Line 23: Three occurrences use the mutable tag "actions/setup-java@v4";
replace each "uses: actions/setup-java@v4" with the exact commit SHA form "uses:
actions/setup-java@<COMMIT_SHA>" (the specific commit SHA from the
actions/setup-java repository) to pin the action immutably. Locate the three
instances of the string "uses: actions/setup-java@v4" in the workflow and update
them to the chosen commit SHAs, then run the workflow lint/check to verify the
YAML remains valid.
In `@gradle/wrapper/gradle-wrapper.properties`:
- Line 3: The Gradle distributionUrl was bumped to 9.5.1 which may break the
build due to Gradle 9.x changes; before merging, either revert the
distributionUrl to the previously used Gradle 8.x version or run a full upgrade
checklist: execute gradle help --scan to gather deprecation/warning details,
update any plugins or code that rely on internal/removed APIs (check plugins in
build.gradle settings and init scripts), verify compatibility with Kotlin and
Groovy versions used by your project, and follow the Gradle 9.0.0 migration
guide to address stricter nullability and removed CLI options; only update
distributionUrl to 9.5.1 once all incompatible plugins/code are updated and the
build/tests pass.
In `@language-server/modules/langserver-core/spotbugs-exclude.xml`:
- Around line 215-224: The crash-state fields in
org.ballerinalang.langserver.workspace.BallerinaWorkspaceManager$ProjectContext
(compilationCrashed and projectCrashed) are accessed from multiple threads
without consistent synchronization; either declare both fields as volatile in
ProjectContext or ensure every read/write uses the existing Lock (use
lockAndGet()/unlock around accesses and update set*/is* methods accordingly).
Update the field declarations and/or the getters/setters for compilationCrashed
and projectCrashed (and all call sites that read/write them) so visibility is
guaranteed across threads.
In `@misc/debug-adapter/modules/debug-adapter-core/spotbugs-exclude.xml`:
- Around line 93-102: The fields supportsVerification and isVerified in class
BalBreakpoint are accessed from multiple threads (read in getAsDAPBreakpoint()
and written in setVerified(), activateUserBreakPoints(), and
setSupportsVerification()), so mark both fields as volatile to ensure visibility
across threads; update the field declarations in BalBreakpoint to add the
volatile modifier for supportsVerification and isVerified.
---
Outside diff comments:
In @.github/workflows/daily_build.yml:
- Around line 72-76: Update the step title string "Set up JDK 21" to reflect the
configured Java version used by actions/setup-java@v4 (java-version: '25.0.3');
for example rename the step to "Set up JDK 25" or "Set up JDK 25 (Temurin
25.0.3)" so the workflow logs accurately match the java-version setting.
In @.github/workflows/push_master.yml:
- Line 116: The workflow step name "Set up JDK 21" is stale and should match the
configured Java version; update the step name string to reflect the actual
version being installed (e.g., change "Set up JDK 21" to "Set up JDK 25" or "Set
up JDK 25.0.3") so the step name matches the configuration that installs Java
25.0.3.
---
Nitpick comments:
In @.github/workflows/trivy-scan.yml:
- Line 16: Replace mutable action tags with immutable commit SHAs: update uses
entries for actions/setup-java@v4, actions/checkout@v4, and actions/cache@v4 to
their corresponding repository commit SHAs (e.g.,
actions/setup-java@<commit-sha>) to pin the workflow to a specific git commit;
fetch the desired SHA from each action's GitHub repository (releases or commit
history) and use that full SHA ref in the workflow to ensure supply-chain
immutability.
In `@distribution/zip/jballerina/bin/bal`:
- Line 222: Replace the permissive flag --sun-misc-unsafe-memory-access=allow
with the less-permissive --sun-misc-unsafe-memory-access=warn in the bal startup
options so the JVM still runs but surfaces offending modules, and add a short
runtime/build-time tracking step (e.g., emit a one-line comment/log or write to
a deps file when the JVM warns about sun.misc unsafe access) to record which
dependencies trigger the warning so they can be updated/replaced in future
releases.
In `@distribution/zip/jballerina/bin/bal.bat`:
- Line 195: The bal.bat script currently hardcodes the JVM flag
--sun-misc-unsafe-memory-access=allow into CMD_LINE_ARGS; change the
CMD_LINE_ARGS assembly logic to mirror the Unix bal script by only appending
that flag after verifying the JVM supports it (e.g., run a version/feature check
or inspect %JAVA_HOME%\bin\java -XshowSettings or similar) or remove the flag
entirely for Windows builds; update the CMD_LINE_ARGS variable construction (the
line setting CMD_LINE_ARGS) so it conditionally includes or omits
--sun-misc-unsafe-memory-access=allow and document the check in the script
comments.
In `@language-server/modules/langserver-stdlib/build.gradle`:
- Line 47: This file uses inline layout.buildDirectory.get().asFile instead of
the shared buildDir variable; add def buildDir =
layout.buildDirectory.get().asFile (e.g., immediately after the plugins block)
and update usages such as file:
"${project.layout.buildDirectory.get().asFile}/ballerina-src/Ballerina.toml" to
use file: "${buildDir}/ballerina-src/Ballerina.toml" so the module matches the
migration pattern used in sibling build.gradle files.
In `@misc/testerina/modules/testerina-core/build.gradle`:
- Line 57: Replace the direct project API call with the pre-defined buildDir
variable: locate the use of project.layout.buildDirectory.get().asFile in the
Ballerina.toml file path (the line that currently reads file:
"${project.layout.buildDirectory.get().asFile}/ballerina-src/Ballerina.toml")
and update it to use the buildDir variable (e.g., construct the path from
buildDir + "/ballerina-src/Ballerina.toml") so the existing buildDir definition
is consistently reused.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d08365ad-0cfc-4b52-9459-ec34b8e80373
📒 Files selected for processing (73)
.github/workflows/daily_build.yml.github/workflows/daily_spec_conformance_test_runner.yml.github/workflows/nightly_publish_timestamped_release.yml.github/workflows/observe_package_push.yaml.github/workflows/publish_release.yml.github/workflows/publish_timestamped_release.yml.github/workflows/pull_request_debugger_tests.yml.github/workflows/pull_request_full_build.yml.github/workflows/pull_request_ubuntu_build.yml.github/workflows/pull_request_windows_build.yml.github/workflows/push_master.yml.github/workflows/trivy-scan.ymlballerina-shell/modules/shell-cli/spotbugs-exclude.xmlballerina-shell/modules/shell-cli/src/test/java/io/ballerina/shell/cli/test/base/TestIntegrator.javaballerina-shell/modules/shell-core/spotbugs-exclude.xmlbuild-config/checkstyle/build.gradlebuild.gradlecli/ballerina-cli/build.gradlecli/ballerina-cli/spotbugs-exclude.xmlcli/ballerina-cli/src/test/java/io/ballerina/cli/cmd/TestNativeImageCommandTest.javacli/central-client/build.gradlecompiler/ballerina-lang/spotbugs-exclude.xmlcompiler/ballerina-lang/src/main/java/io/ballerina/projects/util/ProjectConstants.javadistribution/zip/jballerina-tools/build.gradledistribution/zip/jballerina/bin/baldistribution/zip/jballerina/bin/bal.batdistribution/zip/jballerina/build.gradlegradle/libs.versions.tomlgradle/plugins/java-convention/src/main/groovy/ballerinaLangLibBuild.gradlegradle/plugins/java-convention/src/main/groovy/ballerinaLangLibLoad.gradlegradle/plugins/java-convention/src/main/groovy/ballerinaNativeStdLibBuild.gradlegradle/plugins/java-convention/src/main/groovy/ballerinaStdLibBuild.gradlegradle/plugins/java-convention/src/main/groovy/ballerinaStdLibBuildInternal.gradlegradle/plugins/java-convention/src/main/groovy/javaProject.gradlegradle/plugins/java-convention/src/main/groovy/javaProjectWithExtBala.gradlegradle/wrapper/gradle-wrapper.propertieslanglib/lang.test/build.gradlelanguage-server/modules/langserver-commons/build.gradlelanguage-server/modules/langserver-commons/spotbugs-exclude.xmllanguage-server/modules/langserver-core/build.gradlelanguage-server/modules/langserver-core/spotbugs-exclude.xmllanguage-server/modules/langserver-stdlib/build.gradlemisc/ballerinalang-data-mapper/build.gradlemisc/ballerinalang-data-mapper/src/main/java/org/ballerinalang/datamapper/utils/HttpResponse.javamisc/debug-adapter/modules/debug-adapter-core/spotbugs-exclude.xmlmisc/diagram-util/build.gradlemisc/diagram-util/spotbugs-exclude.xmlmisc/docerina/build.gradlemisc/formatter/modules/formatter-core/build.gradlemisc/maven-resolver/build.gradlemisc/testerina/modules/testerina-core/build.gradleproject-api/project-api-test/build.gradleproject-api/test-artifacts/bad-sad-compiler-plugin/spotbugs-exclude.xmlproject-api/test-artifacts/bal-tool-integration-test-artifacts/build.gradleproject-api/test-artifacts/compiler-plugin-with-analyzer-generator-modifier/spotbugs-exclude.xmlproject-api/test-artifacts/log-creator-in-built-code-analyzer/spotbugs-exclude.xmlproject-api/test-artifacts/log-creator-in-built-code-generator/spotbugs-exclude.xmlproject-api/test-artifacts/log-creator-in-built-code-modifier/spotbugs-exclude.xmlproject-api/test-artifacts/log-creator-pkg-provided-code-analyzer/spotbugs-exclude.xmlproject-api/test-artifacts/log-creator-pkg-provided-code-generator/spotbugs-exclude.xmlproject-api/test-artifacts/log-creator-pkg-provided-code-modifier/spotbugs-exclude.xmlsettings.gradletests/bal-tools-integration-tests/build.gradletests/ballerina-test-utils/build.gradletests/ballerina-test-utils/spotbugs-exclude.xmltests/ballerina-tools-integration-test/build.gradletests/jballerina-benchmark-test/build.gradletests/jballerina-debugger-integration-test/build.gradletests/jballerina-debugger-integration-test/spotbugs-exclude.xmltests/jballerina-debugger-integration-test/src/test/java/org/ballerinalang/debugger/test/adapter/DebugOutputTest.javatests/jballerina-integration-test/build.gradletests/language-server-integration-tests/build.gradletests/testerina-integration-test/build.gradle
| run: git checkout ${{ github.event.inputs.repoTag }} | ||
| - name: Set up JDK 21 | ||
| - name: Set up JDK 25 | ||
| uses: actions/setup-java@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/observe_package_push.yaml | head -30Repository: ballerina-platform/ballerina-lang
Length of output: 1009
🏁 Script executed:
cat -n .github/workflows/observe_package_push.yamlRepository: ballerina-platform/ballerina-lang
Length of output: 2363
Pin actions/setup-java to a commit SHA instead of @v4.
Line 21 uses a mutable major version tag that can change unexpectedly. Pin to a full commit SHA to ensure workflow reproducibility and comply with action pinning policies.
🧰 Tools
🪛 zizmor (1.25.2)
[error] 21-21: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/observe_package_push.yaml at line 21, The workflow
currently references a mutable tag "uses: actions/setup-java@v4"; replace this
with a fixed commit SHA for the actions/setup-java action (e.g., "uses:
actions/setup-java@<full-commit-sha>") so the job is pinned and
reproducible—update the observe_package_push workflow to use the specific commit
SHA for actions/setup-java instead of the `@v4` tag.
|
|
||
| - name: Set up JDK 21 | ||
| - name: Set up JDK 25 | ||
| uses: actions/setup-java@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd .github/workflows && wc -l pull_request_full_build.ymlRepository: ballerina-platform/ballerina-lang
Length of output: 109
🏁 Script executed:
cat -n .github/workflows/pull_request_full_build.yml | sed -n '20,30p; 78,85p; 169,175p'Repository: ballerina-platform/ballerina-lang
Length of output: 958
🏁 Script executed:
rg "setup-java" .github/workflows/pull_request_full_build.yml -nRepository: ballerina-platform/ballerina-lang
Length of output: 195
🌐 Web query:
GitHub Actions best practices version pinning major version tag mutable security
💡 Result:
To ensure the security and stability of your GitHub Actions workflows, the industry-standard best practice is to pin all actions to a full-length (40-character) commit SHA [1][2][3][4]. Why Pinning to a Commit SHA is Required GitHub Actions referenced by tags (e.g., @v4) or branches (e.g., @main) are mutable [5][6]. This means the code they point to can be changed at any time—either intentionally by the maintainer or maliciously if the repository or maintainer account is compromised [1][2][5][7]. Pinning to a full-length commit SHA ensures that your workflow always executes the exact same, immutable code, protecting you against supply chain attacks where malicious code is injected into a previously trusted tag [2][3][4]. Key Security Considerations 1. Enforced Policies: Many organizations now mandate SHA pinning via GitHub's organization-level policies [8][9]. When enabled, any workflow using a tag-based reference will fail [8][9]. Note that this policy often checks the entire dependency tree, meaning even internal sub-actions referenced by your primary actions must be SHA-pinned [8]. 2. Immutable Releases: GitHub now supports "Immutable Releases," which lock a tag to a specific commit and prevent it from being moved or deleted [10][11][12]. While this provides a higher level of security for tagged releases, pinning to a commit SHA remains the most robust and universally recommended method for consumers to guarantee immutability [3][4][13]. 3. Maintenance: Because SHA-pinned actions do not automatically receive updates, you must actively manage them [2][14]. It is best practice to: - Use a comment in your workflow file to indicate the version the SHA corresponds to (e.g., uses: actions/checkout@abc123... # v4.1.1) [15][1][16]. - Use automated dependency management tools like Dependabot or Renovate to track and propose updates to your pinned SHAs [15][14][4][17]. Summary of Best Practices - Always pin to a full-length (40-character) commit SHA [3][4][17]. - Include the version tag in a comment for readability and maintenance [15][1][16]. - Use Dependabot or similar tools to keep your pinned actions up to date [14][4][17]. - Verify that the SHA originates from the official action repository, not a fork [3][4].
Citations:
- 1: https://dev.to/ciberconscientes/github-actions-why-you-should-pin-your-actions-to-a-specific-version-1513
- 2: https://www.stepsecurity.io/blog/pinning-github-actions-for-enhanced-security-a-complete-guide
- 3: https://docs.github.com/en/actions/reference/security/secure-use
- 4: https://github.com/ihs7/action-sha-pinned-actions
- 5: https://dev.to/kanywst/hacking-github-from-tag-rewrites-to-dangling-commits-where-the-git-protocol-trusts-you-without-2o4h
- 6: https://www.jvt.me/posts/2026/04/24/github-actions-tagging/
- 7: https://dev.to/nkuziev-sec/why-fixed-container-image-versions-matter-lessons-from-the-trivy-supply-chain-attack-29ke
- 8: https://www.romainlespinasse.dev/posts/github-actions-commit-sha-pinning/
- 9: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- 10: https://docs.github.com/en/code-security/concepts/supply-chain-security/immutable-releases
- 11: https://docs.github.com/code-security/concepts/supply-chain-security/immutable-releases
- 12: https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/immutable-releases#:~:text=Git tags cannot be moved,cannot be changed or removed.
- 13: Enable immutable releases docker/build-push-action#1460
- 14: https://medium.com/@adan.alvarez/github-actions-and-the-pinning-problem-what-100-security-projects-reveal-54a3a9dcc902
- 15: Pin all GitHub Actions to immutable SHA commits for supply chain security github/gh-aw#5691
- 16: Pin GitHub Actions to commit SHAs in release workflow github/gh-aw#10021
- 17: kubernetes/community@3bab028
🏁 Script executed:
fd -e md -e txt | xargs rg -l "actions/setup-java|version.*pin|commit.*sha" --max-count=5 2>/dev/null | head -10Repository: ballerina-platform/ballerina-lang
Length of output: 59
🏁 Script executed:
find . -name "SECURITY.md" -o -name "CONTRIBUTING.md" -o -name ".github/CODEOWNERS" | xargs cat 2>/dev/nullRepository: ballerina-platform/ballerina-lang
Length of output: 3906
Pin actions/setup-java to commit SHAs across all three jobs.
Lines 23, 81, and 172 use mutable @v4 tags. Pin each to a full commit SHA to ensure immutable, reproducible workflow execution and protect against supply chain attacks where tags could be repointed to malicious code.
Also applies to: 81-81, 172-172
🧰 Tools
🪛 zizmor (1.25.2)
[error] 23-23: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pull_request_full_build.yml at line 23, Replace the
mutable tag "uses: actions/setup-java@v4" with the corresponding full commit SHA
for each occurrence so the workflow is pinned and immutable; update all three
spots where "uses: actions/setup-java@v4" appears (the three separate job steps)
to use the full commit SHA string (the exact SHA for the v4 release) instead of
the `@v4` tag, ensuring each job references the same verified commit SHA.
|
|
||
| - name: Set up JDK 21 | ||
| - name: Set up JDK 25 | ||
| uses: actions/setup-java@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'uses:\s*actions/setup-java@v[0-9]+' .github/workflows/push_master.ymlRepository: ballerina-platform/ballerina-lang
Length of output: 195
🏁 Script executed:
cat -n .github/workflows/push_master.yml | grep -E "(uses:|actions/setup-java)"Repository: ballerina-platform/ballerina-lang
Length of output: 633
🏁 Script executed:
# Check if there are any other action usages that might need pinning
rg -n 'uses:\s*actions' .github/workflows/push_master.yml | head -20Repository: ballerina-platform/ballerina-lang
Length of output: 445
Pin actions/setup-java to commit SHAs instead of version tags.
Lines 23, 67, and 117 use @v4 tags instead of immutable SHAs. Replace with the specific commit SHA for improved security and reproducibility.
🧰 Tools
🪛 zizmor (1.25.2)
[error] 23-23: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/push_master.yml at line 23, Three occurrences use the
mutable tag "actions/setup-java@v4"; replace each "uses: actions/setup-java@v4"
with the exact commit SHA form "uses: actions/setup-java@<COMMIT_SHA>" (the
specific commit SHA from the actions/setup-java repository) to pin the action
immutably. Locate the three instances of the string "uses:
actions/setup-java@v4" in the workflow and update them to the chosen commit
SHAs, then run the workflow lint/check to verify the YAML remains valid.
| <Match> | ||
| <Class name="org.ballerinalang.langserver.workspace.BallerinaWorkspaceManager$ProjectContext" /> | ||
| <Field name="compilationCrashed" /> | ||
| <Bug pattern="AT_STALE_THREAD_WRITE_OF_PRIMITIVE" /> | ||
| </Match> | ||
| <Match> | ||
| <Class name="org.ballerinalang.langserver.workspace.BallerinaWorkspaceManager$ProjectContext" /> | ||
| <Field name="projectCrashed" /> | ||
| <Bug pattern="AT_STALE_THREAD_WRITE_OF_PRIMITIVE" /> | ||
| </Match> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the field declarations and concurrent access patterns
rg -nP -A5 -B2 'class\s+.*ProjectContext' language-server/modules/langserver-core/src/main/java/
rg -nP -C3 '\b(compilationCrashed|projectCrashed)\s*=' language-server/modules/langserver-core/src/main/java/Repository: ballerina-platform/ballerina-lang
Length of output: 4747
🏁 Script executed:
#!/bin/bash
# Get the full ProjectContext class definition
# Since it's likely an inner class, we need to find where it ends
rg -nP 'public static class ProjectContext' language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java -A 150 | head -200Repository: ballerina-platform/ballerina-lang
Length of output: 5073
🏁 Script executed:
#!/bin/bash
# Find all call sites for the crash state setters
rg -nP 'setCompilationCrashed|setProjectCrashed|compilationCrashed\(\)|isProjectCrashed\(\)' language-server/modules/langserver-core/src/main/java/ | head -50Repository: ballerina-platform/ballerina-lang
Length of output: 2825
🏁 Script executed:
#!/bin/bash
# Check context around key call sites to see if lock is acquired
echo "=== Lines 350-400 ==="
sed -n '340,400p' language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java | cat -n
echo -e "\n=== Lines 1380-1400 ==="
sed -n '1380,1400p' language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java | cat -n
echo -e "\n=== Lines 1480-1495 ==="
sed -n '1480,1495p' language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java | cat -n
echo -e "\n=== Lines 1545-1575 ==="
sed -n '1545,1575p' language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java | cat -nRepository: ballerina-platform/ballerina-lang
Length of output: 7938
Add volatile keyword to crash state fields or ensure lock-protected access.
The fields compilationCrashed and projectCrashed are written from compilation threads and read from LSP protocol threads without consistent synchronization. While a Lock exists in ProjectContext, it's only applied to the project and process fields (with explicit Javadoc guidance). The crash state fields are accessed unsynchronized in most call sites (lines 350, 362, 378, 1390, 1489, 1555, 1569), creating visibility and race condition risks.
Either:
- Mark these fields as
volatileto ensure visibility across threads, or - Always acquire the lock before reading or writing these fields.
Reference: Inconsistent synchronization pattern in call sites
// Without lock (lines 350, 362, 378):
Optional<ProjectContext> projectPair = projectContext(projectRoot(filePath));
if (projectPair.get().compilationCrashed()) { ... }
// With lock (lines 386-394):
Lock lock = projectPair.get().lockAndGet();
try {
if (projectPair.get().compilationCrashed()) {
projectPair.get().setCompilationCrashed(false);
}
} finally {
lock.unlock();
}
// Without lock (lines 1390, 1489, 1555, 1569):
projectContext.setProjectCrashed(true);
projectContext.isProjectCrashed()
projectContext.setProjectCrashed(false);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@language-server/modules/langserver-core/spotbugs-exclude.xml` around lines
215 - 224, The crash-state fields in
org.ballerinalang.langserver.workspace.BallerinaWorkspaceManager$ProjectContext
(compilationCrashed and projectCrashed) are accessed from multiple threads
without consistent synchronization; either declare both fields as volatile in
ProjectContext or ensure every read/write uses the existing Lock (use
lockAndGet()/unlock around accesses and update set*/is* methods accordingly).
Update the field declarations and/or the getters/setters for compilationCrashed
and projectCrashed (and all call sites that read/write them) so visibility is
guaranteed across threads.
| <Match> | ||
| <Class name="org.ballerinalang.debugadapter.breakpoint.BalBreakpoint" /> | ||
| <Field name="supportsVerification" /> | ||
| <Bug pattern="AT_STALE_THREAD_WRITE_OF_PRIMITIVE" /> | ||
| </Match> | ||
| <Match> | ||
| <Class name="org.ballerinalang.debugadapter.breakpoint.BalBreakpoint" /> | ||
| <Field name="isVerified" /> | ||
| <Bug pattern="AT_STALE_THREAD_WRITE_OF_PRIMITIVE" /> | ||
| </Match> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check BalBreakpoint field declarations and concurrent access
rg -nP -A10 'class\s+BalBreakpoint' misc/debug-adapter/modules/debug-adapter-core/src/main/java/
rg -nP -C3 '\b(supportsVerification|isVerified)\s*=' misc/debug-adapter/modules/debug-adapter-core/src/main/java/Repository: ballerina-platform/ballerina-lang
Length of output: 4890
🏁 Script executed:
# Check getter methods and search for concurrent access patterns
rg -nP 'getIsVerified|isVerified|getSupportsVerification|supportsVerification' misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/breakpoint/BalBreakpoint.java
# Look at the full BalBreakpoint class to understand context
wc -l misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/breakpoint/BalBreakpoint.java
# Search for where BalBreakpoint instances are accessed from multiple threads
rg -nP 'setVerified|setSupportsVerification' misc/debug-adapter/modules/debug-adapter-core/src/main/java/Repository: ballerina-platform/ballerina-lang
Length of output: 1645
🏁 Script executed:
# Check BreakpointProcessor context around line 209
sed -n '200,220p' misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/BreakpointProcessor.java
# Check ServerUtils context around line 132
sed -n '120,145p' misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/utils/ServerUtils.java
# Search for thread creation or executor usage in debug adapter
rg -nP 'Thread|Executor|ThreadPool|newThread' misc/debug-adapter/modules/debug-adapter-core/src/main/java/ | head -30Repository: ballerina-platform/ballerina-lang
Length of output: 8619
🏁 Script executed:
# Search for breakpoint storage and access patterns
rg -nP 'breakpoints|BalBreakpoint' misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/ | grep -E '(Map|List|Collection|ConcurrentHashMap|Collections)' | head -20
# Look for the DebugAdapterContext and how breakpoints are managed
rg -nP 'class.*Context|getBreakpoints' misc/debug-adapter/modules/debug-adapter-core/src/main/java/ | head -20
# Check for multi-threaded execution in debug adapter core
rg -nP 'ExecutorService|newFixedThreadPool|newCachedThreadPool|Thread\s+t|new Thread' misc/debug-adapter/modules/debug-adapter-core/src/main/java/ | head -15Repository: ballerina-platform/ballerina-lang
Length of output: 7067
🏁 Script executed:
# Check BreakpointProcessor for synchronization/suspend context
sed -n '190,220p' misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/BreakpointProcessor.java
# Check JDIEventProcessor for synchronization around enableBreakpoints
sed -n '215,235p' misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/JDIEventProcessor.java
# Look for the full getAsDAPBreakpoint method (around line 91-102 in BalBreakpoint)
sed -n '91,107p' misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/breakpoint/BalBreakpoint.javaRepository: ballerina-platform/ballerina-lang
Length of output: 3166
Mark these breakpoint state fields as volatile.
The supportsVerification and isVerified fields in BalBreakpoint are read and written across multiple threads without visibility guarantees. The getAsDAPBreakpoint() method reads both fields (line 96) without synchronization, and setVerified() is called from different contexts via activateUserBreakPoints() and setSupportsVerification() from configuration setup. Since the debug adapter operates with multiple threads (JDI event processor, DAP protocol handling), non-volatile writes to these fields can result in stale cache values or torn reads. Adding volatile ensures thread-safe visibility of breakpoint state changes across all threads.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@misc/debug-adapter/modules/debug-adapter-core/spotbugs-exclude.xml` around
lines 93 - 102, The fields supportsVerification and isVerified in class
BalBreakpoint are accessed from multiple threads (read in getAsDAPBreakpoint()
and written in setVerified(), activateUserBreakPoints(), and
setSupportsVerification()), so mark both fields as volatile to ensure visibility
across threads; update the field declarations in BalBreakpoint to add the
volatile modifier for supportsVerification and isVerified.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@tests/jballerina-integration-test/src/test/java/org/ballerinalang/test/troubleshoot/StrandDumpTest.java`:
- Line 59: The three disabled test annotations in StrandDumpTest.java (the
`@Test`(enabled = false) on the test methods around the occurrences at lines
corresponding to the three tests) lack any explanation or issue tracking; please
add an inline comment directly above each disabled test annotation referencing a
tracker/issue ID (or a TODO with a short reason and owner) and a link or issue
number so re-enablement can be tracked, e.g., "TODO: disabled due to <brief
reason> - tracked in ISSUE-#### (owner)"; ensure you update the PR description
to summarize the rationale and the same issue reference so the intent is
recorded for functions/tests in this class (the three disabled test methods in
StrandDumpTest).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d01aaeb9-c5c4-440a-a48c-b089ead65be8
📒 Files selected for processing (1)
tests/jballerina-integration-test/src/test/java/org/ballerinalang/test/troubleshoot/StrandDumpTest.java
| } | ||
|
|
||
| @Test | ||
| @Test(enabled = false) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for references to StrandDump, strand dump issues, or Java 25 compatibility notes
echo "=== Searching for StrandDump references and potential issues ==="
rg -i "stranddump|strand.dump" --type java -C2
echo -e "\n=== Searching for Java 25 compatibility comments ==="
rg -i "java.?25|jdk.?25" --type java -C2
echo -e "\n=== Checking for TODO/FIXME related to these tests ==="
rg -i "todo|fixme|xxx|hack" --glob "**/StrandDumpTest.java" -C2Repository: ballerina-platform/ballerina-lang
Length of output: 20058
🏁 Script executed:
#!/bin/bash
echo "=== Checking git log for StrandDumpTest changes ==="
git log --oneline -20 -- tests/jballerina-integration-test/src/test/java/org/ballerinalang/test/troubleshoot/StrandDumpTest.java
echo -e "\n=== Checking detailed commit message for recent changes ==="
git log -1 --format=fuller -- tests/jballerina-integration-test/src/test/java/org/ballerinalang/test/troubleshoot/StrandDumpTest.java
echo -e "\n=== Checking for any issues or patterns in recent Java 25 related changes ==="
git log --oneline --all --grep="Java 25\|java 25\|strand" -i | head -20
echo -e "\n=== Searching for any `@Ignore` annotations to compare pattern ==="
rg "`@Ignore`|`@Test`.*enabled.*false" --type java -B1 | head -50Repository: ballerina-platform/ballerina-lang
Length of output: 7081
Add explanation and issue tracking for disabled strand dump tests.
All three test methods in this file have been disabled without explanation, documentation, or issue references. Unlike other disabled tests in the codebase (e.g., NotSoBasicWorkerTest.java references issue #30590), these tests lack any tracking mechanism for re-enablement.
The commit message provides no details about:
- Why these tests are disabled
- Whether this is a temporary workaround or permanent change
- What the plan is to restore coverage
Add an inline comment with an issue reference or tracking details, or include information in the PR description explaining the reason for disabling this critical test coverage.
Also applies to: 76-76, 95-95
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@tests/jballerina-integration-test/src/test/java/org/ballerinalang/test/troubleshoot/StrandDumpTest.java`
at line 59, The three disabled test annotations in StrandDumpTest.java (the
`@Test`(enabled = false) on the test methods around the occurrences at lines
corresponding to the three tests) lack any explanation or issue tracking; please
add an inline comment directly above each disabled test annotation referencing a
tracker/issue ID (or a TODO with a short reason and owner) and a link or issue
number so re-enablement can be tracked, e.g., "TODO: disabled due to <brief
reason> - tracked in ISSUE-#### (owner)"; ensure you update the PR description
to summarize the rationale and the same issue reference so the intent is
recorded for functions/tests in this class (the three disabled test methods in
StrandDumpTest).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.gradle`:
- Around line 133-134: The html.outputLocation currently points to a file path;
change it to a directory path (e.g. "${rootDir}/.jacoco/reports/jacoco/html") so
JaCoCo can write multiple HTML files; leave xml.outputLocation as-is. Update the
html.outputLocation assignment to use new File("<directory>") (not a single
report.html file) and ensure the directory exists/gets created by the build if
needed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b531c35d-723b-4fdf-b1fe-99c04b459363
📒 Files selected for processing (2)
build.gradlemisc/maven-resolver/build.gradle
| xml.outputLocation = new File("${rootDir}/.jacoco/reports/jacoco/report.xml") | ||
| html.outputLocation = new File("${rootDir}/.jacoco/reports/jacoco/report.html") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Gradle JacocoReport, what is the expected type/semantics of reports.html.outputLocation? Should it be a directory path or an HTML file path?
💡 Result:
For the Gradle JacocoReport task, the reports.html.outputLocation property expects a directory path, not a file path [1][2]. The JaCoCo HTML report is a collection of files (a site), and the output location is configured as a DirectoryReport [3][4]. When setting this property, you should point it to the directory where you want the HTML report site to be generated [5]. Example of correct configuration (Kotlin DSL): tasks.jacocoTestReport { reports { html.outputLocation.set(layout.buildDirectory.dir("jacoco/html")) } } Example of correct configuration (Groovy DSL): jacocoTestReport { reports { html.outputLocation = layout.buildDirectory.dir('jacocoHtml') } } This contrasts with other report types like XML or CSV, which are SingleFileReports and expect a direct file path [3][4].
Citations:
- 1: https://docs.gradle.org/7.4/userguide/jacoco_plugin.html
- 2: https://docs.gradle.org/current/userguide/jacoco_plugin.html
- 3: http://gradle.org/docs/current/javadoc/org/gradle/testing/jacoco/tasks/JacocoReportsContainer.html
- 4: https://docs.gradle.org/8.4/kotlin-dsl/gradle/org.gradle.testing.jacoco.tasks/-jacoco-reports-container/index.html
- 5: https://discuss.gradle.org/t/how-to-lazy-configure-jacoco-build-directory/43034
Use a directory for JaCoCo HTML output location.
html.outputLocation must point to a directory, not a file path. The JaCoCo HTML report is a collection of files, and setting it to a file path (report.html) violates the API contract and can cause report publishing/consumption issues.
Proposed fix
- html.outputLocation = new File("${rootDir}/.jacoco/reports/jacoco/report.html")
+ html.outputLocation = new File("${rootDir}/.jacoco/reports/jacoco/html")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| xml.outputLocation = new File("${rootDir}/.jacoco/reports/jacoco/report.xml") | |
| html.outputLocation = new File("${rootDir}/.jacoco/reports/jacoco/report.html") | |
| xml.outputLocation = new File("${rootDir}/.jacoco/reports/jacoco/report.xml") | |
| html.outputLocation = new File("${rootDir}/.jacoco/reports/jacoco/html") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.gradle` around lines 133 - 134, The html.outputLocation currently
points to a file path; change it to a directory path (e.g.
"${rootDir}/.jacoco/reports/jacoco/html") so JaCoCo can write multiple HTML
files; leave xml.outputLocation as-is. Update the html.outputLocation assignment
to use new File("<directory>") (not a single report.html file) and ensure the
directory exists/gets created by the build if needed.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/push_master.yml (1)
116-120:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the JDK setup step name to match the configured version.
Line 116 says
Set up JDK 21, but Line 120 is25.0.3. Keeping the label accurate avoids confusion in build logs.Suggested patch
- - name: Set up JDK 21 + - name: Set up JDK 25🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/push_master.yml around lines 116 - 120, Update the workflow step label so it matches the configured Java version: change the step name "Set up JDK 21" to reflect the actual java-version '25.0.3' (the step using actions/setup-java@v4 with with: java-version: '25.0.3') so build logs accurately describe the JDK being installed..github/workflows/daily_build.yml (1)
72-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix stale JDK step label.
Line 72 still says
Set up JDK 21, but Line 76 installs25.0.3. This makes CI logs misleading during incident triage.Suggested patch
- - name: Set up JDK 21 + - name: Set up JDK 25🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/daily_build.yml around lines 72 - 76, The workflow step label "Set up JDK 21" is stale relative to the configured java-version '25.0.3' for actions/setup-java@v4; update the step name text to accurately reflect the installed JDK (for example "Set up JDK 25" or "Set up Temurin JDK 25.0.3") so CI logs show the correct JDK version during runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/daily_build.yml:
- Around line 72-76: The workflow step label "Set up JDK 21" is stale relative
to the configured java-version '25.0.3' for actions/setup-java@v4; update the
step name text to accurately reflect the installed JDK (for example "Set up JDK
25" or "Set up Temurin JDK 25.0.3") so CI logs show the correct JDK version
during runs.
In @.github/workflows/push_master.yml:
- Around line 116-120: Update the workflow step label so it matches the
configured Java version: change the step name "Set up JDK 21" to reflect the
actual java-version '25.0.3' (the step using actions/setup-java@v4 with with:
java-version: '25.0.3') so build logs accurately describe the JDK being
installed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a168e838-dea2-419a-b1c0-8a6430f1e222
📒 Files selected for processing (4)
.github/workflows/daily_build.yml.github/workflows/pull_request_ubuntu_build.yml.github/workflows/push_master.ymlgradle/libs.versions.toml
There was a problem hiding this comment.
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)
project-api/project-api-test/src/test/java/io/ballerina/projects/test/TestBuildProject.java (1)
221-258:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnsure permissions are always restored for the whole test body.
resetPermissions(projectPath)is only guaranteed after entering the inner try block. If an exception occurs before that (for example at Line 250), the test can leak filesystem permissions into later tests.Proposed fix
- // 4) Compile the current package — expects RuntimeException on non-Windows - PackageCompilation compilation = currentPackage.getCompilation(); - try { - JBallerinaBackend jBallerinaBackend = JBallerinaBackend.from(compilation, JvmTarget.JAVA_21); - Assert.fail("Expected RuntimeException when compiling with no write permission"); - } catch (RuntimeException e) { - // expected - } finally { - resetPermissions(projectPath); - } + // 4) Compile the current package — expects RuntimeException on non-Windows + try { + PackageCompilation compilation = currentPackage.getCompilation(); + try { + JBallerinaBackend.from(compilation, JvmTarget.JAVA_21); + Assert.fail("Expected RuntimeException when compiling with no write permission"); + } catch (RuntimeException e) { + // expected + } + } finally { + resetPermissions(projectPath); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@project-api/project-api-test/src/test/java/io/ballerina/projects/test/TestBuildProject.java` around lines 221 - 258, The test testBuildProjectWithNoWritePermission can leak filesystem permissions because resetPermissions(projectPath) is only in the inner finally; wrap the entire test logic (from immediately after setWritable(false, true) through the end) in a single try { ... } finally { resetPermissions(projectPath); } so resetPermissions is always executed even if loadBuildProject, currentPackage(), or compilation throw; keep the inner try/catch that asserts RuntimeException for JBallerinaBackend.from(...) but move/reset the outer finally to guarantee restoration of projectPath permissions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@project-api/project-api-test/src/test/java/io/ballerina/projects/test/TestSingleFileProject.java`:
- Around line 220-222: The testSingleFileWithNoReadPermission method currently
does an early return when isWindows() is true, which marks the test as passed;
replace that return with throwing a TestNG skip by using throw new
SkipException("Skipping test on Windows") (or similar message) so the test
reports as SKIPPED; update the if (isWindows()) block to throw SkipException
(the file already imports org.testng.SkipException) and ensure no other logic is
executed afterwards.
---
Outside diff comments:
In
`@project-api/project-api-test/src/test/java/io/ballerina/projects/test/TestBuildProject.java`:
- Around line 221-258: The test testBuildProjectWithNoWritePermission can leak
filesystem permissions because resetPermissions(projectPath) is only in the
inner finally; wrap the entire test logic (from immediately after
setWritable(false, true) through the end) in a single try { ... } finally {
resetPermissions(projectPath); } so resetPermissions is always executed even if
loadBuildProject, currentPackage(), or compilation throw; keep the inner
try/catch that asserts RuntimeException for JBallerinaBackend.from(...) but
move/reset the outer finally to guarantee restoration of projectPath
permissions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2660c35a-d8c5-41c1-963c-d3cee0ce6915
📒 Files selected for processing (9)
compiler/ballerina-lang/src/test/java/io/ballerina/projects/ProjectUtilsTests.javaproject-api/project-api-test/src/test/java/io/ballerina/projects/test/BaseTest.javaproject-api/project-api-test/src/test/java/io/ballerina/projects/test/CustomRepoResolutionTests.javaproject-api/project-api-test/src/test/java/io/ballerina/projects/test/HierarchicalPackageNameTests.javaproject-api/project-api-test/src/test/java/io/ballerina/projects/test/PackageResolutionTests.javaproject-api/project-api-test/src/test/java/io/ballerina/projects/test/TestBalaProject.javaproject-api/project-api-test/src/test/java/io/ballerina/projects/test/TestBuildProject.javaproject-api/project-api-test/src/test/java/io/ballerina/projects/test/TestBuildProjectWithGeneratedSources.javaproject-api/project-api-test/src/test/java/io/ballerina/projects/test/TestSingleFileProject.java
| if (isWindows()) { | ||
| throw new SkipException("Skipping tests on Windows"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In TestNG, what is the reporting/behavior difference between returning early from an @test method and throwing org.testng.SkipException?
💡 Result:
Difference in TestNG reporting/behavior: 1) Returning early from an @Test method - TestNG treats a normal return from a @Test method as “test completed successfully” (assuming no assertion/exception was thrown), so it reports the test as PASS, not SKIP. TestNG does not have a special “skip” notion for a bare early return. 2) Throwing org.testng.SkipException - TestNG treats throwing org.testng.SkipException from a @Test or @Configuration method as a skip, not a failure. The SkipException Javadoc states it is “the root exception for special skip handling” and that in case a @Test or @Configuration throws this exception, “the method will be considered a skip” (subject to SkipException#isSkip behavior). [1] - The SkipException class extends RuntimeException and is explicitly meant for “special skip handling.” [1] Important nuance: SkipException may still interact with TestNG’s “configuration failure policy” - In suites, TestNG has a configfailurepolicy that controls what happens when @Before* fails: default is “skip”, and you can set “continue” to run remaining tests. This affects whether one skipped configuration can cause other tests/configs to be skipped too. [2][3] Practical takeaway - Use “return;” to stop the current test method, but it will still be reported as SUCCESS/PASS. - Use “throw new SkipException(...)” to have TestNG report the method (and potentially dependent subsequent behavior) as SKIPPED. [1] Code sketch (conceptual) - Early return: - inside @Test: if (cond) return; -> not a SKIP; reported as PASS. - Skip: - inside @Test: if (cond) throw new SkipException("..."); -> reported as SKIP. [1]
Citations:
- 1: https://javadoc.io/static/org.testng/testng/7.0.0-beta1/org/testng/SkipException.html
- 2: https://testng.org/
- 3: https://stackoverflow.com/questions/58264087/testng-skip-execution-of-testmethod-in-beforemethod-instead-of-all-skipping-re
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="project-api/project-api-test/src/test/java/io/ballerina/projects/test/TestSingleFileProject.java"
# Show the window around the reported lines
nl -ba "$FILE" | sed -n '190,250p'
# Check for SkipException usage/imports
rg -n "SkipException" "$FILE" || true
rg -n "import .*SkipException" "$FILE" || true
# Show the method signature that contains the isWindows() block
# (extract a few lines above the if)
nl -ba "$FILE" | sed -n '205,235p'Repository: ballerina-platform/ballerina-lang
Length of output: 120
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="project-api/project-api-test/src/test/java/io/ballerina/projects/test/TestSingleFileProject.java"
# Print lines 190-250 with line numbers using python (nl isn't available)
python3 - <<'PY'
import itertools
path = "project-api/project-api-test/src/test/java/io/ballerina/projects/test/TestSingleFileProject.java"
start, end = 190, 250
with open(path, 'r', encoding='utf-8') as f:
for i, line in enumerate(f, start=1):
if start <= i <= end:
print(f"{i:4d}: {line.rstrip()}")
if i > end:
break
PY
# Search for SkipException usage/imports
rg -n "SkipException" "$FILE" || true
rg -n "import .*SkipException" "$FILE" || true
# Also show the test method signature around the isWindows() block
python3 - <<'PY'
import re
path = "project-api/project-api-test/src/test/java/io/ballerina/projects/test/TestSingleFileProject.java"
target_line = 221
# Find nearest def line (method signature) above the target line
with open(path, 'r', encoding='utf-8') as f:
lines = f.readlines()
for i in range(target_line-1, -1, -1):
if re.search(r'\)\s*\{?\s*$', lines[i]) or re.search(r'\bvoid\s+\w+\s*\(', lines[i]) or re.search(r'\w+\s+\w+\s*\(', lines[i]):
# print a small window upward for context
lo = max(0, i-10); hi = min(len(lines), i+20)
for j in range(lo, hi):
print(f"{j+1:4d}: {lines[j].rstrip()}")
break
PYRepository: ballerina-platform/ballerina-lang
Length of output: 4930
Use TestNG skip instead of returning on Windows.
testSingleFileWithNoReadPermission returns early on Windows (if (isWindows()) { return; }), which makes the test appear as PASS instead of SKIP. Throw org.testng.SkipException instead (the file already imports it).
Suggested fix
if (isWindows()) {
- return;
+ throw new SkipException("Skipping tests on Windows");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (isWindows()) { | |
| throw new SkipException("Skipping tests on Windows"); | |
| return; | |
| } | |
| if (isWindows()) { | |
| throw new SkipException("Skipping tests on Windows"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@project-api/project-api-test/src/test/java/io/ballerina/projects/test/TestSingleFileProject.java`
around lines 220 - 222, The testSingleFileWithNoReadPermission method currently
does an early return when isWindows() is true, which marks the test as passed;
replace that return with throwing a TestNG skip by using throw new
SkipException("Skipping test on Windows") (or similar message) so the test
reports as SKIPPED; update the if (isWindows()) block to throw SkipException
(the file already imports org.testng.SkipException) and ensure no other logic is
executed afterwards.
|
Purpose
Fixes #
Approach
Samples
Remarks
Check List
This pull request upgrades the Ballerina build and CI/tooling environment to Java 25 and modernizes related build configuration to improve compatibility and build reliability.
Key changes
Java platform upgrade across CI and build:
Build system modernization:
Dependency and tooling updates:
Build and test script adjustments:
Code-quality and configuration updates:
Repository structural tweaks:
Scope and impact
Checklist