Feature/jdk25 support#4363
Conversation
WalkthroughAdds runtime Java version detection and selects between legacy Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Action as ActLaunchJavaWSApplication
participant JavaExe as java.exe
participant AgentStore as StaticDrivers/
User->>Action: Trigger Launch/Attach
Action->>JavaExe: run "java -version" (GetJavaVersion)
JavaExe-->>Action: version output
Action->>Action: ExtractVersion / ExtractMajorVersion
Action->>AgentStore: check for appropriate jar file (GingerAgent / JavaAgent_V25)
AgentStore-->>Action: return selected jar path (JarFilePath)
Action->>Action: assemble launch/attach command using JarFilePath
Action-->>User: execute launch/attach with chosen agent
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-08-26T07:40:08.345ZApplied to files:
📚 Learning: 2024-07-26T22:04:12.930ZApplied to files:
🔇 Additional comments (6)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml (1)
14-21: New Java Agent version UI is wired correctly; consider improving placement/textThe extra row and the
xTextBoxJavaAgentJDKVersionblock are structurally fine and match the code‑behind binding. Functionally this should work as intended.Two optional tweaks to consider:
- Place this block inside the “Attach Ginger Java Agent” section (e.g., within
LaunchWithAgentArgsPnl) so the version field is visually grouped with other agent‑related settings.- Make the label/tooltip more explicit, e.g. “Java Agent JAR file name (e.g. GingerAgentJDK8.jar or GingerAgentJDK25.jar)” to reduce ambiguity about what users should enter.
Also applies to: 182-199
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs (2)
581-585: Redundantjava.exeexistence check in Java path validationThe Java bin validation currently checks
java.exetwice:if (Directory.Exists(mJavaWSEXEPath_Calc) == false || File.Exists(Path.Combine(mJavaWSEXEPath_Calc, "java.exe")) == false || File.Exists(Path.Combine(mJavaWSEXEPath_Calc, "java.exe")) == false)The third clause is redundant. You can simplify it to improve readability without changing behavior:
- if (Directory.Exists(mJavaWSEXEPath_Calc) == false || File.Exists(Path.Combine(mJavaWSEXEPath_Calc, "java.exe")) == false || File.Exists(Path.Combine(mJavaWSEXEPath_Calc, "java.exe")) == false) + if (!Directory.Exists(mJavaWSEXEPath_Calc) || + !File.Exists(Path.Combine(mJavaWSEXEPath_Calc, "java.exe")))If you do intend to validate additional executables (e.g.
javaws.exefor JNLP), it would be clearer to add them explicitly rather than duplicatejava.exe.
1147-1166:IsInstrumentationModuleLoadedstill searches forGingerAgent.jardespite versioned jarsThis helper still invokes
handle.exeand searches for the hard‑coded tokenGingerAgent.jar:var processHandle = Process.Start(new ProcessStartInfo() { FileName = handleExePath, Arguments = String.Format(" -accepteula -p {0} GingerAgent.jar", id), UseShellExecute = false, RedirectStandardOutput = true }); string cliOut = processHandle.StandardOutput.ReadToEnd(); ... if (cliOut.Contains("GingerAgent.jar")) { return true; }With the move to
GingerAgentJDK8.jar/GingerAgentJDK25.jarand selection viaJavaAgentJDKVersion, this is now inconsistent and will likely cause the method to always report “not loaded” for the new jars. That impacts logic inWaitForAppWindowTitleand other call sites that rely on this check to avoid re‑attaching.Recommend basing the search token on the calculated agent jar name:
- var processHandle = Process.Start(new ProcessStartInfo() { FileName = handleExePath, Arguments = String.Format(" -accepteula -p {0} GingerAgent.jar", id), UseShellExecute = false, RedirectStandardOutput = true }); + var processHandle = Process.Start(new ProcessStartInfo() + { + FileName = handleExePath, + Arguments = string.Format(" -accepteula -p {0} {1}", id, mJavaAgentJDKVersion_calc), + UseShellExecute = false, + RedirectStandardOutput = true + }); string cliOut = processHandle.StandardOutput.ReadToEnd(); @@ - if (cliOut.Contains("GingerAgent.jar")) + if (cliOut.Contains(mJavaAgentJDKVersion_calc)) { return true; }This keeps existing behavior but makes it compatible with the new JDK‑specific agent jars and the
JavaAgentJDKVersionsetting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (12)
Ginger/Ginger/GingerAgentStarter.xmlis excluded by!**/*.xmlGinger/Ginger/StaticDrivers/GingerAgent.jaris excluded by!**/*.jar,!**/*.jarGinger/Ginger/StaticDrivers/GingerAgentJDK25.jaris excluded by!**/*.jar,!**/*.jarGinger/Ginger/StaticDrivers/GingerAgentJDK8.jaris excluded by!**/*.jar,!**/*.jarGinger/Ginger/StaticDrivers/GingerAgentStarter.jaris excluded by!**/*.jar,!**/*.jarGinger/GingerCore/Drivers/JavaDriverLib/GingerAgentBuild.xmlis excluded by!**/*.xmlGinger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent.docxis excluded by!**/*.docx,!**/*.docxGinger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/GingerAgent.jaris excluded by!**/*.jar,!**/*.jarGinger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/GingerAgentStarter.jaris excluded by!**/*.jar,!**/*.jarGinger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/build.xmlis excluded by!**/*.xmlGinger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/jsoup.jaris excluded by!**/*.jar,!**/*.jarGinger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/xsoup.jaris excluded by!**/*.jar,!**/*.jar
📒 Files selected for processing (32)
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml(2 hunks)Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml.cs(1 hunks)Ginger/Ginger/Ginger.csproj(1 hunks)Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs(6 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/.classpath(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/.externalToolBuilders/CreateJavaAgentJar.launch(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/.project(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/.settings/org.eclipse.jdt.core.prefs(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/GingerAgentJar.jardesc(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/GingerjarPreparation.bat(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/MANIFEST.MF(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/ASCFPack/ASCFHelper.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/BrowserHelper.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/EditorHelper.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/FormsDumpInfo.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/GingerAgent.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/GingerAgentFrame.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/GingerAgentStarterFrame.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/GingerJavaSocketServer.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/IXPath.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/PayLoad.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/Recorder.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/SwingHelper.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/Utils.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/WaitForIdle.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/WindowMonitor.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/XPathHelper.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/test/com/amdocs/ginger/BasicSwingApp.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/test/com/amdocs/ginger/BasicSwingComponents.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/test/com/amdocs/ginger/GingerAgentTestApp.java(0 hunks)Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/test/com/amdocs/ginger/JEditorPaneExample.java(0 hunks)Ginger/GingerCore/GingerCore.csproj(0 hunks)
💤 Files with no reviewable changes (28)
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/test/com/amdocs/ginger/JEditorPaneExample.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/.project
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/GingerAgentStarterFrame.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/.settings/org.eclipse.jdt.core.prefs
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/Utils.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/WindowMonitor.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/GingerjarPreparation.bat
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/XPathHelper.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/test/com/amdocs/ginger/GingerAgentTestApp.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/WaitForIdle.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/GingerAgentFrame.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/test/com/amdocs/ginger/BasicSwingApp.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/.classpath
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/GingerAgentJar.jardesc
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/IXPath.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/GingerJavaSocketServer.java
- Ginger/GingerCore/GingerCore.csproj
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/.externalToolBuilders/CreateJavaAgentJar.launch
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/PayLoad.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/FormsDumpInfo.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/BrowserHelper.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/test/com/amdocs/ginger/BasicSwingComponents.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/Recorder.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/ASCFPack/ASCFHelper.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/EditorHelper.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/SwingHelper.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/agent/com/amdocs/ginger/GingerAgent.java
- Ginger/GingerCore/Drivers/JavaDriverLib/GingerJavaAgent/MANIFEST.MF
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-06-16T10:37:13.073Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4232
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:19-22
Timestamp: 2025-06-16T10:37:13.073Z
Learning: In Ginger codebase, both `using amdocs.ginger.GingerCoreNET;` and `using Amdocs.Ginger.CoreNET;` are valid and serve different purposes. The first (lowercase) contains the WorkSpace class and related workspace functionality, while the second (proper case) contains drivers like GenericAppiumDriver and other core functionality. Both may be required in files that use types from both namespaces.
Applied to files:
Ginger/Ginger/Ginger.csproj
📚 Learning: 2025-07-16T14:42:32.229Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4254
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs:70-73
Timestamp: 2025-07-16T14:42:32.229Z
Learning: In Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs, prashelke prefers to keep the current `!WorkSpace.Instance.BetaFeatures.ShowPOMForAI` logic for binding and showing AI POM controls, rather than changing it to positive logic.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml.cs
📚 Learning: 2025-08-26T07:40:08.345Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4285
File: Ginger/Ginger/Actions/ActionEditPages/ActSecurityTestingEditPage.xaml:23-23
Timestamp: 2025-08-26T07:40:08.345Z
Learning: In the Ginger codebase ActSecurityTestingEditPage.xaml file, the large bottom margin of 530 on the "Acceptable Alerts" label (Margin="10,10,0,530") is intentional design choice, not an error.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml
📚 Learning: 2025-08-28T09:27:18.847Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4286
File: Ginger/Ginger/Drivers/DriversConfigsEditPages/WebAgentConfigEditPage.xaml.cs:556-571
Timestamp: 2025-08-28T09:27:18.847Z
Learning: In Ginger's WebAgentConfigEditPage.xaml.cs ZAP configuration, when ZAP Security Testing is enabled, the manual proxy UI should remain enabled/accessible even though ZAP will override the proxy settings in the background. This allows users to see and modify their proxy configurations for transparency and future use when ZAP is disabled. Confirmed by AmanPrasad43 in PR #4286.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 3429
File: Ginger/Ginger/AutomatePageLib/AddActionMenu/WindowExplorer/Common/WindowExplorerPage.xaml.cs:1123-1126
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has removed an unnecessary empty line at 1123 in `WindowExplorerPage.xaml.cs` for better code compactness.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml
🧬 Code graph analysis (1)
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml.cs (1)
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs (2)
ActLaunchJavaWSApplication(40-1202)Fields(82-106)
🔇 Additional comments (1)
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml.cs (1)
56-84: Binding forxTextBoxJavaAgentJDKVersionlooks consistentThe new Init call follows the existing
UCValueExpressionbinding pattern (Context + action +Fields.JavaAgentJDKVersion) and should correctly wire the new property into the edit page.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs (1)
581-585: RedundantFile.Exists("java.exe")check in Java path validationThe condition:
if (Directory.Exists(mJavaWSEXEPath_Calc) == false || File.Exists(Path.Combine(mJavaWSEXEPath_Calc, "java.exe")) == false || File.Exists(Path.Combine(mJavaWSEXEPath_Calc, "java.exe")) == false)checks for
java.exetwice, making the third operand redundant.You can simplify (and clarify intent) as:
- if (Directory.Exists(mJavaWSEXEPath_Calc) == false || File.Exists(Path.Combine(mJavaWSEXEPath_Calc, "java.exe")) == false || File.Exists(Path.Combine(mJavaWSEXEPath_Calc, "java.exe")) == false) + if (!Directory.Exists(mJavaWSEXEPath_Calc) || + !File.Exists(Path.Combine(mJavaWSEXEPath_Calc, "java.exe")))or, if you meant to also validate
javaws.exe, introduce that explicitly instead of duplicating the same check.
♻️ Duplicate comments (2)
Ginger/Ginger/Ginger.csproj (1)
2671-2673: Fix indentation inconsistency: use spaces instead of tabs.Lines 2671-2673 use tabs for indentation while all surrounding
<None Update>entries (lines 2668–2670, 2674–2676) use spaces. Replace tabs with spaces to match the project's formatting conventions.<None Update="sikulirestapi-1.0.jar"> <CopyToOutputDirectory>Always</CopyToOutputDirectory> </None> - <None Update="StaticDrivers\JavaAgent_V25.jar"> - <CopyToOutputDirectory>Always</CopyToOutputDirectory> - </None> + <None Update="StaticDrivers\JavaAgent_V25.jar"> + <CopyToOutputDirectory>Always</CopyToOutputDirectory> + </None> <None Update="StaticDrivers\GingerAgentStarter.jar"> <CopyToOutputDirectory>Always</CopyToOutputDirectory> </None>Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs (1)
619-621: Agent folder validation may be stricter than necessary vs dynamic jar selectionValidation currently requires both jars to exist:
if (Directory.Exists(mJavaAgentPath_Calc) == false || File.Exists(Path.Combine(mJavaAgentPath_Calc, "GingerAgent.jar")) == false || File.Exists(Path.Combine(mJavaAgentPath_Calc, "JavaAgent_V25.jar")) == false)while
GetJavaAgentJarat runtime chooses one jar based on the detected Java major version:if (majorVersion <= 8) return Path.Combine(agentPath, "GingerAgent.jar"); else return Path.Combine(agentPath, "JavaAgent_V25.jar");If deployments are allowed to ship only the jar they actually need (e.g., Java 17+ only with just
JavaAgent_V25.jar), this validation will block otherwise valid configurations.Consider either:
- Intentionally documenting and enforcing “both jars must be present”, or
- Relaxing validation to check only the jar that will actually be used (e.g., derive the expected jar via
GetJavaAgentJaror a cheaper equivalent and validate that path).Also applies to: 898-913
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
Ginger/Ginger/StaticDrivers/GingerAgent.jaris excluded by!**/*.jar,!**/*.jarGinger/Ginger/StaticDrivers/JavaAgent_V25.jaris excluded by!**/*.jar,!**/*.jar
📒 Files selected for processing (2)
Ginger/Ginger/Ginger.csproj(1 hunks)Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs(7 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: Maheshkale447
Repo: Ginger-Automation/Ginger PR: 3637
File: Ginger/GingerCoreNET/Application Models/Execution/POM/POMExecutionUtils.cs:226-234
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The `LastUpdatedTime` in the `POMExecutionUtils.cs` file of the Ginger project is always in the correct DateTime format, as confirmed by the user. This information is crucial for avoiding unnecessary error handling suggestions in future reviews.
Applied to files:
Ginger/Ginger/Ginger.csproj
📚 Learning: 2025-01-21T11:43:12.379Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4067
File: Ginger/GingerCoreCommon/Repository/SolutionRepository.cs:55-55
Timestamp: 2025-01-21T11:43:12.379Z
Learning: In Ginger's SolutionRepository, the path @"ExecutionResults\GingerExecutionResults.db" needs to be explicitly excluded in mSolutionPathsToAvoid list, even though its parent directory @"ExecutionResults\" is also excluded. This specific file exclusion serves a distinct purpose and should not be considered redundant.
Applied to files:
Ginger/Ginger/Ginger.csproj
📚 Learning: 2025-08-26T07:40:08.345Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4285
File: Ginger/Ginger/Actions/ActionEditPages/ActSecurityTestingEditPage.xaml:23-23
Timestamp: 2025-08-26T07:40:08.345Z
Learning: In the Ginger codebase ActSecurityTestingEditPage.xaml file, the large bottom margin of 530 on the "Acceptable Alerts" label (Margin="10,10,0,530") is intentional design choice, not an error.
Applied to files:
Ginger/Ginger/Ginger.csprojGinger/GingerCore/Actions/ActLaunchJavaWSApplication.cs
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs (1)
1206-1226:IsInstrumentationModuleLoadedonly detectsGingerAgent.jar, notJavaAgent_V25.jar.With the new dynamic jar selection,
PerformAttachGingerAgentmay attachJavaAgent_V25.jarfor Java 9+ versions (line 816), butIsInstrumentationModuleLoadedstill hardcodesGingerAgent.jarin both:
- The
handle.exearguments (line 1218)- The output detection check (line 1223)
This means for Java 9+ processes using
JavaAgent_V25.jar:
- The method always returns
false- The 30-second post-attach polling loop (lines 452-458) never observes the agent as loaded
- Re-attach protection based on this method fails
Update the detection to handle both jar names:
-var processHandle = Process.Start(new ProcessStartInfo() { FileName = handleExePath, Arguments = String.Format(" -accepteula -p {0} GingerAgent.jar", id), UseShellExecute = false, RedirectStandardOutput = true }); +var processHandle = Process.Start(new ProcessStartInfo() { FileName = handleExePath, Arguments = String.Format(" -accepteula -p {0}", id), UseShellExecute = false, RedirectStandardOutput = true }); string cliOut = processHandle.StandardOutput.ReadToEnd(); processHandle.WaitForExit(); processHandle.Close(); -if (cliOut.Contains("GingerAgent.jar")) +if (cliOut.Contains("GingerAgent.jar") || cliOut.Contains("JavaAgent_V25.jar")) { return true; }Note: Removing the jar name from
handle.exearguments makes it search all handles for the process, then checking output for either jar name ensures detection works for both agents.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml.cs(1 hunks)Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs(6 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-16T14:42:32.229Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4254
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs:70-73
Timestamp: 2025-07-16T14:42:32.229Z
Learning: In Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs, prashelke prefers to keep the current `!WorkSpace.Instance.BetaFeatures.ShowPOMForAI` logic for binding and showing AI POM controls, rather than changing it to positive logic.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml.cs
📚 Learning: 2025-08-26T07:40:08.345Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4285
File: Ginger/Ginger/Actions/ActionEditPages/ActSecurityTestingEditPage.xaml:23-23
Timestamp: 2025-08-26T07:40:08.345Z
Learning: In the Ginger codebase ActSecurityTestingEditPage.xaml file, the large bottom margin of 530 on the "Acceptable Alerts" label (Margin="10,10,0,530") is intentional design choice, not an error.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml.csGinger/GingerCore/Actions/ActLaunchJavaWSApplication.cs
📚 Learning: 2025-06-16T10:36:01.993Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4228
File: Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs:83-84
Timestamp: 2025-06-16T10:36:01.993Z
Learning: For the `UnLockType` property in `Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs`, the current naming convention with capital 'L' in the middle should be maintained per project preferences.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml.cs
🧬 Code graph analysis (1)
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml.cs (1)
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs (2)
ActLaunchJavaWSApplication(40-1260)Fields(82-105)
🔇 Additional comments (2)
Ginger/Ginger/Actions/ActionEditPages/ActLaunchJavaWSApplicationEditPage.xaml.cs (1)
79-79: LGTM - Whitespace-only change.This change only affects trailing whitespace with no functional impact.
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs (1)
600-602: Dual-jar validation is appropriately updated.The validation now correctly checks for both
GingerAgent.jarandJavaAgent_V25.jar, and the error message accurately references both files. While this requires both jars to be present (stricter than runtime needs since only one is used per Java version), it ensures the deployment bundle is complete.
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)
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs (1)
1210-1230: FixIsInstrumentationModuleLoadedto detect both agent jar names.The method at lines 1222 and 1227 hardcodes
"GingerAgent.jar"but the codebase deploys two different agents based on Java version:
GingerAgent.jarfor Java 8 and lower (line 890)JavaAgent_V25.jarfor Java 9+ (line 894)For Java 9+ processes with
JavaAgent_V25.jarattached, the method will always returnfalsebecausehandle.exeis called with"GingerAgent.jar"and the output check looks for"GingerAgent.jar". This causes:
- The 30-second polling loop (lines 452-458) never breaks early, timing out instead
- Re-attach protection fails, potentially leading to duplicate attach attempts
- Incorrect retry/validation behavior for Java 9+ applications
Solution: Modify
IsInstrumentationModuleLoadedto check for both jar names in the output:// Line 1222: pass both jar names or search for either // Line 1227: check if output contains EITHER jar name if (cliOut.Contains("GingerAgent.jar") || cliOut.Contains("JavaAgent_V25.jar")) { return true; }Alternatively, query the process modules directly without relying on the hardcoded jar name.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-26T07:40:08.345Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4285
File: Ginger/Ginger/Actions/ActionEditPages/ActSecurityTestingEditPage.xaml:23-23
Timestamp: 2025-08-26T07:40:08.345Z
Learning: In the Ginger codebase ActSecurityTestingEditPage.xaml file, the large bottom margin of 530 on the "Acceptable Alerts" label (Margin="10,10,0,530") is intentional design choice, not an error.
Applied to files:
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 3429
File: Ginger/Ginger/AutomatePageLib/AddActionMenu/WindowExplorer/Common/WindowExplorerPage.xaml.cs:1139-1142
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has updated the code by removing or addressing the commented code as suggested in the review.
Applied to files:
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs
🔇 Additional comments (3)
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs (3)
816-827: Dynamic Java agent jar selection correctly implemented.The code properly invokes
GetJavaAgentJarto select the appropriate agent jar based on Java version detection and consistently uses the result in both command parameters and the-jarcommand. This enables support for both Java 8 (GingerAgent.jar) and Java 9+ (JavaAgent_V25.jar).
847-896: Java version detection and jar selection logic is well-implemented.The implementation correctly:
- Executes
java -versionand reads from StandardError (where Java outputs version info)- Handles the case where java.exe is not found with a clear exception
- Parses version strings and extracts the version number safely
- Selects the appropriate agent jar based on major version (≤8 uses GingerAgent.jar, >8 uses JavaAgent_V25.jar)
- Provides safe fallbacks (file not found → exception, parse failure → "Unknown" → major version 0 → GingerAgent.jar)
898-920: Version parsing logic correctly handles both legacy and modern Java version formats.The implementation properly:
- Guards against null/whitespace input (returns 0 as fallback)
- Handles legacy Java 1.x format (e.g., "1.8.0_171") by checking
parts[0] == "1" && parts.Length > 1- Handles modern Java 9+ format (e.g., "17.0.2") by parsing the first part
- Uses
int.TryParsefor safe parsing without exceptions- Has appropriate safeguards against
IndexOutOfRangeException(the length check at line 908 prevents accessingparts[1]when it doesn't exist)The fallback of
majorVersion = 0is sensible as it causesGetJavaAgentJarto select GingerAgent.jar, which is the safer choice for unparseable versions.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs (1)
856-881: Consider adding defensive null/empty checks for process output.While the current implementation handles most error cases by returning "Unknown" and falling back to version 0 (which selects
GingerAgent.jar), adding explicit checks for empty output would make the code more robust and easier to debug.public static string GetJavaVersion(string javaExePath) { if (!File.Exists(javaExePath)) { throw new FileNotFoundException("java.exe not found at the specified path.", javaExePath); } ProcessStartInfo psi = new ProcessStartInfo { FileName = javaExePath, Arguments = "-version", RedirectStandardError = true, UseShellExecute = false, CreateNoWindow = true }; using (Process process = Process.Start(psi)) { string output = process.StandardError.ReadToEnd(); process.WaitForExit(); + if (string.IsNullOrWhiteSpace(output)) + { + Reporter.ToLog(eLogLevel.WARN, "java -version returned empty output"); + return "Unknown"; + } + // First line usually: java version "17.0.2" - string versionLine = output.Split('\n')[0]; + string[] lines = output.Split('\n'); + string versionLine = lines.Length > 0 ? lines[0] : string.Empty; return ExtractVersion(versionLine); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-26T07:40:08.345Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4285
File: Ginger/Ginger/Actions/ActionEditPages/ActSecurityTestingEditPage.xaml:23-23
Timestamp: 2025-08-26T07:40:08.345Z
Learning: In the Ginger codebase ActSecurityTestingEditPage.xaml file, the large bottom margin of 530 on the "Acceptable Alerts" label (Margin="10,10,0,530") is intentional design choice, not an error.
Applied to files:
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 3429
File: Ginger/Ginger/AutomatePageLib/AddActionMenu/WindowExplorer/Common/WindowExplorerPage.xaml.cs:1139-1142
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has updated the code by removing or addressing the commented code as suggested in the review.
Applied to files:
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs
🔇 Additional comments (2)
Ginger/GingerCore/Actions/ActLaunchJavaWSApplication.cs (2)
600-624: Verify JAR naming convention and version support scope.The code selects
JavaAgent_V25.jarfor all Java versions greater than 8 (line 610), but the jar name suggests it's specifically for JDK 25. If this jar is intended to support JDK 9 through 25+, the current logic is correct. However, if separate jars are needed for different version ranges (e.g., JDK 9-16, JDK 17-24, JDK 25+), this logic may need adjustment.Please confirm:
- Is
JavaAgent_V25.jarintended to support all JDK versions from 9 onwards?- Are there plans to support additional version-specific jars in the future?
If version-specific jars are needed, consider using a more flexible selection mechanism:
string requiredJar = majorVersion switch { <= 8 => "GingerAgent.jar", >= 9 and <= 16 => "JavaAgent_V16.jar", // If needed >= 17 and <= 24 => "JavaAgent_V24.jar", // If needed >= 25 => "JavaAgent_V25.jar", _ => "GingerAgent.jar" // fallback };
829-836: Dynamic jar selection correctly integrated into attach flow.The code properly uses
JarFilePathfor both the agent jar path parameter and the-jarcommand, ensuring the dynamically selected jar is consistently used throughout the attach operation.Note: This implementation depends on fixing the thread-safety issue with the static
JarFilePathfield (see comment on lines 636-637).
2 participants
Description
Type of Change
Checklist
[IsSerializedForLocalRepository]where neededReporter.ToLog()patternSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.