PR1/15: monorepo aggregator + pic-sure-bom scaffold#260
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR establishes a JDK 25 Maven monorepo foundation with a standalone dependency BOM, a simplified root reactor, strengthened pre-commit checks, updated formatting behavior, and revised repository documentation. ChangesMonorepo foundation
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
New Issues (24)Checkmarx found the following issues in this Pull Request
Fixed Issues (5)Great job! The following issues were fixed in this Pull Request
Use @Checkmarx to interact with Checkmarx PR Assistant. |
Gcolon021
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@code-formatting/pre-commit.sh`:
- Line 98: Update the restaging flow around the `git add $FILES_TO_RESTAGE`
command so it does not replace partial staging with each file’s full
working-tree contents. Preserve the original index, apply only
formatter-produced changes to the already staged portions, and rescan the
resulting index before completing the commit.
- Around line 27-44: Make staged-path handling pathname-safe in
code-formatting/pre-commit.sh lines 27-44 by consuming STAGED without shell word
splitting or glob expansion, so JAVABIN detection and the large-file loop
validate filenames containing whitespace or glob characters. At
code-formatting/pre-commit.sh line 97, quote the complete -DspotlessFiles
property and escape each filename before constructing the regex. Preserve the
existing validation and formatting behavior for ordinary paths.
- Line 97: Quote the complete -DspotlessFiles argument in the pre-commit Maven
invocation so staged Java paths containing whitespace remain a single argument,
while preserving the existing SPOTLESS_MODULES and STAGED_JAVA_FILES_AS_REGEX
values.
- Line 54: Update the CHANGED_MIGRATIONS git diff command to disable rename
detection and inspect only modified or deleted paths, so migration renames are
evaluated using their original filenames rather than destinations. Preserve the
existing migration-path pattern matching and empty-result handling.
In `@platform/pom.xml`:
- Around line 10-17: Update the standalone BOM project version in
platform/pom.xml from the hardcoded value to ${revision}, and add a local
revision property defaulting to 3.0.0 alongside the existing properties. Keep
the BOM’s standalone behavior and reactor alignment with the root revision.
In `@pom.xml`:
- Around line 94-98: Update the Eclipse formatter version under the Spotless
Java configuration in pom.xml from 4.26 to a Java 25-compatible Eclipse JDT
release, ensuring the existing formatter file reference remains unchanged.
In `@README.md`:
- Around line 41-45: Update the README Maven command examples to reflect the
current reactor, which aggregates only platform: mark the whole-reactor and
gateway commands as future examples, or remove them until the corresponding
libraries/services modules are introduced. Keep the existing commands accurate
for the modules currently available.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dfdd4ce8-14c2-43d5-9a21-b14bcef741f1
📒 Files selected for processing (7)
.gitignore.sdkmanrcREADME.mdcode-formatting/pre-commit.shplatform/README.mdplatform/pom.xmlpom.xml
| STAGED=$(git diff --staged --name-only --diff-filter=ACMR) | ||
| JAVABIN=$(printf '%s\n' "$STAGED" | grep -i '\.javabin$' || true) | ||
| if [ -n "$JAVABIN" ]; then | ||
| echo '✖ Refusing to commit .javabin data file(s):' 1>&2 | ||
| printf ' %s\n' $JAVABIN 1>&2 | ||
| echo ' HPDS .javabin blobs are build/data artifacts and must not be committed.' 1>&2 | ||
| echo ' Unstage them: git reset HEAD <file>' 1>&2 | ||
| cd "$CWD"; exit 1 | ||
| fi | ||
|
|
||
| MAX_BYTES=$((5 * 1024 * 1024)) # 5 MB | ||
| LARGE="" | ||
| for f in $STAGED; do | ||
| sz=$(git cat-file -s ":$f" 2>/dev/null || echo 0) | ||
| if [ "$sz" -gt "$MAX_BYTES" ]; then | ||
| LARGE="$LARGE $f ($sz bytes)\n" | ||
| fi | ||
| done |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Handle staged filenames without shell word splitting. Multiple checks assume paths contain no whitespace or glob characters, causing safeguards or formatting to fail for valid filenames.
code-formatting/pre-commit.sh#L27-L44: consume staged names using a pathname-safe representation so large files cannot bypass validation.code-formatting/pre-commit.sh#L97-L97: quote the complete-DspotlessFilesproperty and escape filenames before embedding them in a regex.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 31-31: Double quote to prevent globbing and word splitting.
(SC2086)
📍 Affects 1 file
code-formatting/pre-commit.sh#L27-L44(this comment)code-formatting/pre-commit.sh#L97-L97
🤖 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 `@code-formatting/pre-commit.sh` around lines 27 - 44, Make staged-path
handling pathname-safe in code-formatting/pre-commit.sh lines 27-44 by consuming
STAGED without shell word splitting or glob expansion, so JAVABIN detection and
the large-file loop validate filenames containing whitespace or glob characters.
At code-formatting/pre-commit.sh line 97, quote the complete -DspotlessFiles
property and escape each filename before constructing the regex. Preserve the
existing validation and formatting behavior for ordinary paths.
Source: Linters/SAST tools
|
|
||
| # --- Flyway migration immutability ---------------------------------------- | ||
| # Applied migrations are immutable; changing one breaks Flyway checksum validation. | ||
| CHANGED_MIGRATIONS=$(git diff --staged --name-only --diff-filter=MRD | grep -E '(^|/)V[0-9][^/]*__[^/]*\.sql$' || true) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Detect migration renames by their original path.
With rename detection enabled, --name-only reports the destination, so renaming V1__init.sql to a nonmatching name can evade this check. Disable rename detection and inspect modifications/deletions:
Proposed fix
-CHANGED_MIGRATIONS=$(git diff --staged --name-only --diff-filter=MRD | grep -E '(^|/)V[0-9][^/]*__[^/]*\.sql$' || true)
+CHANGED_MIGRATIONS=$(git diff --staged --no-renames --name-only --diff-filter=DM | grep -E '(^|/)V[0-9][^/]*__[^/]*\.sql$' || true)📝 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.
| CHANGED_MIGRATIONS=$(git diff --staged --name-only --diff-filter=MRD | grep -E '(^|/)V[0-9][^/]*__[^/]*\.sql$' || true) | |
| CHANGED_MIGRATIONS=$(git diff --staged --no-renames --name-only --diff-filter=DM | grep -E '(^|/)V[0-9][^/]*__[^/]*\.sql$' || true) |
🤖 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 `@code-formatting/pre-commit.sh` at line 54, Update the CHANGED_MIGRATIONS git
diff command to disable rename detection and inspect only modified or deleted
paths, so migration renames are evaluated using their original filenames rather
than destinations. Preserve the existing migration-path pattern matching and
empty-result handling.
| [ -f "$d/pom.xml" ] && printf '%s\n' "$d" | ||
| done | sort -u | paste -sd',' -) | ||
| if [ -n "$SPOTLESS_MODULES" ]; then | ||
| mvn -pl "$SPOTLESS_MODULES" spotless:apply -DspotlessFiles=^.*$STAGED_JAVA_FILES_AS_REGEX |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Quote the complete Spotless property argument.
A staged Java path containing whitespace splits -DspotlessFiles into multiple Maven arguments.
- mvn -pl "$SPOTLESS_MODULES" spotless:apply -DspotlessFiles=^.*$STAGED_JAVA_FILES_AS_REGEX
+ mvn -pl "$SPOTLESS_MODULES" spotless:apply "-DspotlessFiles=^.*$STAGED_JAVA_FILES_AS_REGEX"📝 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.
| mvn -pl "$SPOTLESS_MODULES" spotless:apply -DspotlessFiles=^.*$STAGED_JAVA_FILES_AS_REGEX | |
| mvn -pl "$SPOTLESS_MODULES" spotless:apply "-DspotlessFiles=^.*$STAGED_JAVA_FILES_AS_REGEX" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 97-97: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 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 `@code-formatting/pre-commit.sh` at line 97, Quote the complete -DspotlessFiles
argument in the pre-commit Maven invocation so staged Java paths containing
whitespace remain a single argument, while preserving the existing
SPOTLESS_MODULES and STAGED_JAVA_FILES_AS_REGEX values.
Source: Linters/SAST tools
| done | sort -u | paste -sd',' -) | ||
| if [ -n "$SPOTLESS_MODULES" ]; then | ||
| mvn -pl "$SPOTLESS_MODULES" spotless:apply -DspotlessFiles=^.*$STAGED_JAVA_FILES_AS_REGEX | ||
| git add $FILES_TO_RESTAGE |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Do not stage previously unstaged content after the secret scan.
git add $FILES_TO_RESTAGE replaces partial staging with each file’s complete working-tree state. Unrelated changes—including secrets not seen by Gitleaks—can enter the commit after validation. Preserve the original index and update only formatter-produced staged content, then rescan the resulting index.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 98-98: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 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 `@code-formatting/pre-commit.sh` at line 98, Update the restaging flow around
the `git add $FILES_TO_RESTAGE` command so it does not replace partial staging
with each file’s full working-tree contents. Preserve the original index, apply
only formatter-produced changes to the already staged portions, and rescan the
resulting index before completing the commit.
Source: Linters/SAST tools
| <version>3.0.0</version> | ||
| <packaging>pom</packaging> | ||
| <name>PIC-SURE BOM</name> | ||
| <description>Bill of Materials for PIC-SURE API dependencies</description> | ||
| <properties> | ||
| <!-- Duplicated from the root parent by design (standalone BOM cannot inherit). | ||
| Bump these in lockstep with the root pom. --> | ||
| <spring-boot.version>3.5.9</spring-boot.version> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-files pom.xml platform/pom.xml
printf '\n--- pom.xml ---\n'
cat -n pom.xml
printf '\n--- platform/pom.xml ---\n'
cat -n platform/pom.xmlRepository: hms-dbmi/pic-sure
Length of output: 14212
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files:\n'
git ls-files pom.xml platform/pom.xml
printf '\n--- pom.xml ---\n'
cat -n pom.xml
printf '\n--- platform/pom.xml ---\n'
cat -n platform/pom.xmlRepository: hms-dbmi/pic-sure
Length of output: 14219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Root POM references to platform BOM:\n'
rg -n 'pic-sure-bom|platform/pom.xml|<revision>|<version>' pom.xml platform/pom.xml
printf '\nModule declarations in root POM:\n'
rg -n '<module>|<parent>|<dependencyManagement>|<relativePath>' pom.xmlRepository: hms-dbmi/pic-sure
Length of output: 2278
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- root pom version/import lines ---"
rg -n '\${revision}|pic-sure-bom|<module>|<version>' pom.xml platform/pom.xmlRepository: hms-dbmi/pic-sure
Length of output: 323
Align platform/pom.xml with ${revision}.
platform/pom.xml is a standalone BOM, so it needs its own ${revision} default. Hardcoding 3.0.0 breaks reactor resolution whenever the root is built with a different -Drevision, because the root imports pic-sure-bom:${revision} but this module still builds as 3.0.0. Use <version>${revision}</version> here and add a local <revision>3.0.0</revision> default.
🤖 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 `@platform/pom.xml` around lines 10 - 17, Update the standalone BOM project
version in platform/pom.xml from the hardcoded value to ${revision}, and add a
local revision property defaulting to 3.0.0 alongside the existing properties.
Keep the BOM’s standalone behavior and reactor alignment with the root revision.
| <java> | ||
| <eclipse> | ||
| <version>4.26</version> | ||
| <file>${maven.multiModuleProjectDirectory}/code-formatting/eclipse-formatter.xml</file> | ||
| </eclipse> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== pom.xml around formatter/toolchain ==\n'
nl -ba pom.xml | sed -n '70,120p'
printf '\n== Spotless / formatter references ==\n'
rg -n "spotless|eclipse-formatter|eclipse>4\.26|maven.compiler|java.version|release>25|source>25|target>25|toolchain" pom.xmlRepository: hms-dbmi/pic-sure
Length of output: 237
🌐 Web query:
Eclipse JDT 4.26 Java 19 support formatter language level Java 25 compatibility
💡 Result:
Eclipse JDT 4.26, which was released in November 2022 as part of the Eclipse 2022-12 simultaneous release, provides official support for Java 19 [1][2][3]. This support includes various Java 19 features such as Record Patterns (Preview), Pattern Matching for Switch (Third Preview), Virtual Threads (Preview), and Structured Concurrency (Incubator) [1]. Regarding Java 25, Eclipse JDT compatibility and feature support are handled in much later versions of the Eclipse IDE (such as Eclipse 2025-09/4.37 and later) [4]. Java 25 features, including Module Import Declarations, Compact Source Files, and Flexible Constructor Bodies, were officially integrated for Eclipse 2025-09 (4.37) [4]. Because Eclipse JDT 4.26 is a legacy version from 2022, it does not support Java 25; trying to use Java 25 as a project runtime in such an outdated environment will result in compatibility errors [5][6]. The formatter and language-level support for modern features like "import module" (Java 25) are specifically introduced in subsequent Eclipse releases, such as the 4.38 development cycle [7].
Citations:
- 1: https://eclipse.dev/eclipse/news/4.26/jdt.html
- 2: https://eclipse.dev/eclipse/news/4.26/
- 3: https://github.com/SonarSource/sonar-java/wiki/ECJ-based-parsing
- 4: https://www.eclipse.org/lists/jdt-dev/msg02699.html
- 5: [25] Top Level Bug for Java 25 Support for JDT.Core eclipse-jdt/eclipse.jdt.core#3822
- 6: Add Java 25 constants eclipse-jdt/eclipse.jdt.core#4259
- 7: https://github.com/eclipse-platform/www.eclipse.org-eclipse/blob/master/news/4.38/jdt.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the formatter file and any documented compatibility notes in-repo.
printf '\n== formatter file header ==\n'
nl -ba code-formatting/eclipse-formatter.xml | sed -n '1,80p'
printf '\n== repo mentions of formatter version ==\n'
rg -n "4\.26|eclipse-formatter.xml|Spotless|formatter" -S .Repository: hms-dbmi/pic-sure
Length of output: 224
🌐 Web query:
site:eclipse.dev/eclipse/news/4.26/jdt.php Java 19 support formatter
💡 Result:
Eclipse JDT 4.26 adds Java 19 support; the release notes list support for Record Patterns (preview), Pattern Matching for Switch (third preview), Virtual Threads (preview), and Structured Concurrency (incubator). I don’t see any formatter-specific Java 19 changes on that page. [1]
[1] Eclipse Project 4.26 - New and Noteworthy (JDT)
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== pom.xml formatter/toolchain section ==\n'
awk 'NR>=70 && NR<=120 {printf "%4d %s\n", NR, $0}' pom.xml
printf '\n== exact Spotless / Java references in pom.xml ==\n'
grep -nE 'spotless|eclipse-formatter|maven.compiler|java.version|release>|source>|target>|toolchain' pom.xml || true
printf '\n== formatter file first lines ==\n'
awk 'NR>=1 && NR<=40 {printf "%4d %s\n", NR, $0}' code-formatting/eclipse-formatter.xmlRepository: hms-dbmi/pic-sure
Length of output: 7772
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file list hints ==\n'
find . -maxdepth 2 \( -name 'pom.xml' -o -path './code-formatting/*' \) | sed 's#^\./##'
printf '\n== pom.xml version snippets ==\n'
python3 - <<'PY'
from pathlib import Path
text = Path('pom.xml').read_text()
for key in ['25', '4.26', 'spotless', 'eclipse-formatter', 'maven.compiler', 'toolchain']:
print(f'-- {key} --')
for i, line in enumerate(text.splitlines(), 1):
if key in line:
print(f'{i}: {line}')
PYRepository: hms-dbmi/pic-sure
Length of output: 1252
Use a Java 25-compatible Eclipse formatter Spotless pins Eclipse JDT 4.26 here, which only supports Java 19. Bump the formatter version to one that parses Java 25 syntax so formatting doesn’t fail on valid source.
🤖 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 `@pom.xml` around lines 94 - 98, Update the Eclipse formatter version under the
Spotless Java configuration in pom.xml from 4.26 to a Java 25-compatible Eclipse
JDT release, ensuring the existing formatter file reference remains unchanged.
| # whole new reactor (platform + libs + services); legacy is NOT included | ||
| mvn verify | ||
|
|
||
| To run the app for development, go into the pic-sure-api-wildfly folder and use this: | ||
| # just the gateway (and what it needs) | ||
| mvn -pl services/pic-sure-gateway -am verify |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the reactor that exists in this PR.
The root currently aggregates only platform, so mvn verify does not build libraries/services and the gateway -pl command cannot select a reactor module yet. Mark these as future commands or add them when those modules are introduced.
Proposed adjustment
-# whole new reactor (platform + libs + services); legacy is NOT included
+# current reactor (platform only); legacy is NOT included
mvn verify
-# just the gateway (and what it needs)
-mvn -pl services/pic-sure-gateway -am verify
+# Gateway build command will be added when that module joins the reactor.📝 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.
| # whole new reactor (platform + libs + services); legacy is NOT included | |
| mvn verify | |
| To run the app for development, go into the pic-sure-api-wildfly folder and use this: | |
| # just the gateway (and what it needs) | |
| mvn -pl services/pic-sure-gateway -am verify | |
| # current reactor (platform only); legacy is NOT included | |
| mvn verify | |
| # Gateway build command will be added when that module joins the reactor. |
🤖 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 `@README.md` around lines 41 - 45, Update the README Maven command examples to
reflect the current reactor, which aggregates only platform: mark the
whole-reactor and gateway commands as future examples, or remove them until the
corresponding libraries/services modules are introduced. Keep the existing
commands accurate for the modules currently available.
| ## Gateway migration | ||
|
|
||
| Both of these will pause the build allowing you to connect your debuggers. | ||
| Design docs live in `docs/superpowers/` (local-only, gitignored). The migration is tracked under Jira epic ALS-10463; work happens on the long-lived `pic_sure_api_rewrite` branch, merged when complete. |
There was a problem hiding this comment.
This seems inaccurate: you're merging to main~
There was a problem hiding this comment.
I'll fix that. My long lived branch is named pic_sure_api_rewrite. I don't intend to merge this to main any time soon. I'll create a clean branch off of main I can merge it to.
cbe65b4 to
b072328
Compare
Gcolon021
changed the title
* Initial Commit * Refactor LoggingClient to use Sender interface (#1) * Refactor LoggingClient to use Sender interface * Refactor LoggingClient to use config in JdkHttpSender * Add Sender interface for logging functionality * Implement JdkHttpSender for asynchronous HTTP requests * Add NoOpSender class for logging * Refactor LoggingClientTest for clarity and safety * Refactor test method signatures for consistency * Setup CI (#2) * add package wf * Format * Add wf files Update README.md * Add @AuditEvent annotation for declarative endpoint categorization * Update LoggingClientFactory.java * Handle session ids as first class * format * Initial commit * ALS-11850: Initial commit. Move common HPDS and PIC-SURE code to common repo (#1) * build: vendor pic-sure-commons + pic-sure-logging-client (subtree import, pre-v1v3 state) --------- Co-authored-by: James <Jamestp19@gmail.com> Co-authored-by: ramari16 <ramari16@gmail.com>
4 participants
Establishes the multi-module Maven aggregator (
pic-sure-api) and theplatform/parent BOM (pic-sure-bom, Java 25). Aggregator lists onlyplatformat this step; modules are added by later PRs. No behavior.📚 Stacked PR series (1–7) — part of decomposing the
pic_sure_api_rewritemonorepo rewrite into sequential, independently-green PRs. Review and merge bottom-up (this PR's base is the PR below it; GitHub auto-retargets tomainas each merges). Every PR builds + tests green on its own under JDK 25.Summary by CodeRabbit
New Features
Documentation
Chores