You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fixes macOS/Windows builds without the SAP SDK by using the distribution profile and local installs. Adds macOS SAP GUI for Java and cached landscape support, broader JCo/SAP Crypto lookup paths, external landscape logging, and a macOS Secure Login API v2 fallback.
Bug Fixes
Switched compile to install with -Pdistribution -Dopenadt.distribution=true -DskipTests in openadt-bootstrap, openadt-cli, openadt-config, openadt-sap-adt.
Tests run under the distribution profile; added post-test install where needed.
Non-recursive install -N before openadt-config to register the parent POM.
Removed -am to avoid modules that require the SAP SDK.
package targets use the distribution profile and skip tests.
New Features
macOS: detect SAP GUI for Java landscape at ~/Library/Preferences/SAP/SAPGUILandscape.xml and cached landscapes at ~/Library/Preferences/SAP/Cache/SAPUILandscape/*.xml; extract message server details; log info when <Include url="..."> points to external landscapes.
macOS: expanded JCo jar/native search paths (Eclipse.app/BAS bundles, OSGi cache, user .p2 pool, system lib dirs) and SAP Crypto candidates.
Secure Login: try hub API v3 first; fall back to API v2 on macOS.
Docs: updated specs/setup.md with macOS paths and external landscape behavior.
Written for commit 0c69767. Summary will update on new commits.
Summary by CodeRabbit
Chores
Build/test/package workflows updated to use distribution-profiled Maven runs, module-scoped installs, and to remove multi-module "also-make" behavior.
New Features
Improved SAP GUI setup discovery with additional macOS/Linux search locations.
Detector now logs when external landscape includes are detected.
Secure Login Hub now falls back to an older API for status/login to improve resilience.
Documentation
Setup spec updated to document macOS lookup paths and external-include handling.
CodeAnt-AI Description
Fix cross-platform builds and add macOS SAP setup support
What Changed
Builds now work without the SAP SDK on macOS and Windows by using the distribution setup for compile, test, and package steps.
On macOS, the app can now find SAP GUI for Java landscapes in Preferences and cache folders, including cached systems with message server details.
When a landscape points to an external file, the app now warns that some system details may be missing and suggests opening SAP GUI or entering the message server manually.
Secure Login now tries the Windows API first and falls back to the macOS API if needed, so login checks work on both platforms.
The app now looks in more macOS locations for JCo and SAP Crypto files, which helps it detect local SAP prerequisites more reliably.
Impact
✅ Fewer build failures on macOS and Windows ✅ Easier first-time setup on macOS ✅ Fewer missing SAP landscape details ✅ More reliable Secure Login on macOS
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
@codeant-ai ask: Your question here
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
@codeant-ai ask: Can you suggest a safer alternative to storing this secret?
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
@codeant-ai: Your feedback here
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
@codeant-ai: Do not flag unused imports.
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
@codeant-ai: review
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Nx Maven target commands are standardized across four apps: -am is removed and distribution flags are added; some modules use reactor-level installs or two-step test/install sequences. Bootstrap Java code adds macOS discovery paths and detects external <Include url="..."> entries in SAP GUI landscape XML; docs updated.
Changes
Nx Maven Build Configuration Alignment
Layer / File(s)
Summary
Remove -am dependency flag from openadt-cli apps/openadt-cli/project.json
compile, test, and package targets drop -am to run module-scoped Maven invocations for apps/openadt-cli, preserving distribution flags and the dev-runtime-classpath post-step.
Add distribution profile and multi-step sequences to openadt-bootstrap and openadt-sap-adt apps/openadt-bootstrap/project.json, apps/openadt-sap-adt/project.json
Both modules add -Pdistribution -Dopenadt.distribution=true; compile now invokes install, test runs test then install -DskipTests, and package runs distribution-profiled package with -DskipTests.
Add distribution profile with full-project reactor coordination in openadt-config apps/openadt-config/project.json
Targets run a reactor-level install with -N -Pdistribution -Dopenadt.distribution=true -DskipTests and then execute module-scoped operations under the distribution profile.
Bootstrap discovery and landscape parsing
Layer / File(s)
Summary
Detect external Include URLs in landscape parsing apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SapGuiLandscapeDetector.java
parseLandscapeFile now inspects parsed XML for <Include url="..."> entries and emits informational messages when an external landscape URL is detected.
Add macOS-specific discovery paths for JCo and sapcrypto apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SetupPathLocator.java
Adds macOS preference SAPGUILandscape.xml path, p2 plugin pool and Eclipse.app plugins locations for JCo jars, macOS native library and OSGi cache search roots, and macOS libsapcrypto.dylib candidate locations.
Update setup spec for macOS and external-include behavior specs/setup.md
Docs now include the macOS SAPGUILandscape.xml lookup, reorder WSL entries, and document <Include url="..."> implications for message-server hostname discovery.
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
abapify/openadt#28: Related changes to SetupPathLocator-area discovery for JCo runtime locations and p2/dist paths.
Poem
🐰
I hopped through Maven lines today,
Tweaked flags so builds would find their way,
On macOS paths I left my print,
I sniffed includes and gave a hint,
Builds and landscapes — nibble, hop, hooray!
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
⚠️ Warning
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.
Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name
Status
Explanation
Title check
✅ Passed
The title clearly and concisely describes the main purpose of the PR: fixing cross-platform builds on macOS/Windows without the SAP SDK.
Linked Issues check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Description check
✅ Passed
The PR description comprehensively covers changes (compile/test/package, distribution profile, macOS landscape/JCo/Crypto discovery, Secure Login v2 fallback, spec updates) and indicates verification steps were considered.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches🧪 Generate unit tests (beta)
Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request updates the Nx project configuration files across multiple apps to use Maven install commands with distribution profiles. The review feedback correctly identifies that chaining commands with the && operator is not cross-platform compatible, particularly on Windows PowerShell 5.1. To resolve this, the reviewer suggests utilizing the commands array with parallel set to false in the nx:run-commands executor, which is a safer and more robust approach.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
The reason will be displayed to describe this comment to others. Learn more.
Using && to chain commands in a single command string is not fully cross-platform because Windows PowerShell (specifically Windows PowerShell 5.1, which is the default on Windows) does not support the && operator and will throw a syntax error. Since this PR aims to fix macOS/Windows cross-platform builds, we should use the commands array with "parallel": false in the nx:run-commands executor, which is fully cross-platform and safe. Additionally, the second command was missing the -q flag.
The reason will be displayed to describe this comment to others. Learn more.
Using && to chain commands in a single command string is not fully cross-platform because Windows PowerShell 5.1 (the default on Windows) does not support the && operator. To ensure reliable cross-platform execution on Windows, use the commands array with "parallel": false instead.
The reason will be displayed to describe this comment to others. Learn more.
Using && to chain commands in a single command string is not fully cross-platform because Windows PowerShell 5.1 (the default on Windows) does not support the && operator. To ensure reliable cross-platform execution on Windows, use the commands array with "parallel": false instead.
The reason will be displayed to describe this comment to others. Learn more.
Using && to chain commands in a single command string is not fully cross-platform because Windows PowerShell 5.1 (the default on Windows) does not support the && operator. To ensure reliable cross-platform execution on Windows, use the commands array with "parallel": false instead.
The reason will be displayed to describe this comment to others. Learn more.
Using && to chain commands in a single command string is not fully cross-platform because Windows PowerShell 5.1 (the default on Windows) does not support the && operator. To ensure reliable cross-platform execution on Windows, use the commands array with "parallel": false instead. Additionally, the second command was missing the -q flag.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The PR successfully addresses cross-platform build issues for macOS and Windows by transitioning Maven targets from 'compile' to 'install' and removing recursive build flags that pull in SDK-dependent components. Codacy analysis indicates the changes are up to standards with no new issues introduced.
While the implementation fulfills all acceptance criteria, a systemic redundancy was identified in the build configuration for library modules (bootstrap, config, and sap-adt). The 'test' target currently invokes an 'install' command that repeats work already completed by its dependency, the 'compile' target. Removing these redundant calls will improve build performance without impacting the availability of artifacts in the local repository.
About this PR
There is a systemic redundancy across library modules where 'mvn install' is executed twice. Since the 'test' target depends on 'compile', and 'compile' has been updated to perform an 'install', the subsequent installation after tests provides no additional benefit and increases build duration due to Maven startup costs.
Test suggestions
Module 'openadt-bootstrap' build uses 'install' with distribution profile and no '-am' flag
Module 'openadt-cli' build uses 'install' with distribution profile and no '-am' flag
Module 'openadt-config' performs non-recursive parent POM install before module install
Module 'openadt-sap-adt' build uses 'install' with distribution profile and no '-am' flag
Library modules (bootstrap, config, sap-adt) update local repository after successful tests
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Suggestion: The second Maven invocation is redundant. The artifact is already installed in the local repository during the 'compile' phase. Removing this call speeds up the test execution.
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Suggestion: This second Maven invocation is redundant. The 'compile' target already populates the local repository with the required artifacts. Removing this step reduces build overhead.
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Suggestion: The second Maven invocation (install) is redundant because the 'compile' target already ensures the artifact is installed in the local repository. Removing it optimizes the build cycle.
The reason will be displayed to describe this comment to others. Learn more.
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 `@apps/openadt-cli/project.json`:
- Around line 27-29: The test target (targets.test) for apps/openadt-cli
currently depends only on "package" and runs the mvnw-runner.mjs test command
without ensuring the project graph is compiled/installed; update targets.test to
depend on "compile" (in addition to or instead of "package") so
openadt-config/openadt-sap-adt/openadt-bootstrap are built first, and modify the
mvnw-runner.mjs invocation used by targets.test to append an install step
(install -DskipTests) after the test phase (matching the pattern used by
openadt-bootstrap and openadt-sap-adt) so required Maven artifacts are available
when running nx test openadt-cli in isolation.
🪄 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
Run ID: 3432862f-c647-4eac-b3ed-2f6dda3425a7
📥 Commits
Reviewing files that changed from the base of the PR and between 184f4bc and 51dbe04.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Description: Check if any Maven POMs declare openadt-cli as a dependency
rg -t xml --iglob '**/pom.xml' -C3 '<artifactId>openadt-cli</artifactId>'
Repository: abapify/openadt
Length of output: 41
🏁 Script executed:
#!/bin/bashset -euo pipefail
# 1) Read the relevant parts of openadt-cli/project.jsonecho"---- apps/openadt-cli/project.json (lines 1-120) ----"
nl -ba apps/openadt-cli/project.json | sed -n '1,120p'# 2) Find comparable project.json files for bootstrap and sap-adtecho"---- Find project.json under apps/openadt-* ----"
find apps -maxdepth 3 -name project.json | sed -n '1,200p'# 3) Read relevant sections from bootstrap/sap-adt project.json if presentforfin$(find apps -maxdepth 4 -name project.json | sort);doifecho"$f"| rg -q 'openadt-(bootstrap|sap-adt)';thenecho"---- $f (lines 1-160) ----"
nl -ba "$f"| sed -n '1,160p'fidone# 4) Double-check if openadt-cli is referenced in any non-pom config (quick search)echo"---- Search for openadt-cli references across repo ----"
rg -n --hidden --no-ignore -S "openadt-cli" -g'*.{xml,yml,yaml,json,properties,gradle,kts,sh,mjs,js,pom}'| head -n 200
#!/bin/bashset -euo pipefail
echo"---- apps/openadt-cli/pom.xml: artifactId/groupId ----"
rg -n --no-heading "<(groupId|artifactId)>" apps/openadt-cli/pom.xml | head -n 80
artifactId="$(rg -n --no-heading "<artifactId>" apps/openadt-cli/pom.xml | head -n 1 | sed -E 's/.*<artifactId>([^<]+)<\/artifactId>.*/\1/'|| true)"echo"Detected artifactId: ${artifactId:-<none>}"echo"---- Other pom.xml files depending on that artifactId (excluding openadt-cli/pom.xml) ----"if [ -n"${artifactId}" ];then
rg -t xml --iglob '**/pom.xml' --glob '!apps/openadt-cli/pom.xml' -n --no-heading "<artifactId>${artifactId}</artifactId>" -C2 ||truefiecho"---- Check whether any pom references openadt-cli module directory directly ----"
rg -n --no-heading "<module>apps/openadt-cli</module>" -S pom.xml **/pom.xml ||true
Repository: abapify/openadt
Length of output: 3750
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"---- apps/openadt-cli/pom.xml (top + dependencies section excerpt) ----"
cat -n apps/openadt-cli/pom.xml | sed -n '1,220p'echo"---- apps/openadt-cli/pom.xml: dependency coordinates (groupId/artifactId pairs) ----"
rg -n --no-heading "<dependency>" -n apps/openadt-cli/pom.xml >/dev/null ||true
python3 - <<'PY'import re, pathlibp=pathlib.Path("apps/openadt-cli/pom.xml").read_text()# crude parse: capture groupId/artifactId inside <dependency> blocksdeps=[]for m in re.finditer(r"<dependency>\s*(.*?)\s*</dependency>", p, flags=re.S): block=m.group(1) gid=re.search(r"<groupId>\s*([^<]+)\s*</groupId>", block) aid=re.search(r"<artifactId>\s*([^<]+)\s*</artifactId>", block) scope=re.search(r"<scope>\s*([^<]+)\s*</scope>", block) if aid: deps.append((gid.group(1).strip() if gid else None, aid.group(1).strip(), scope.group(1).strip() if scope else None))print("\n".join([f"{g or ''}:{a} (scope={s or 'none'})" for g,a,s in deps]))PYecho"---- Search other module poms for dependency on org.openadt:openadt ----"
rg -t xml --iglob '**/pom.xml' --glob '!apps/openadt-cli/pom.xml' -n "<groupId>org.openadt</groupId>|<artifactId>openadt</artifactId>" -C1
echo"---- Search other module poms for artifactId openadt (quick) ----"
rg -t xml --iglob '**/pom.xml' --glob '!apps/openadt-cli/pom.xml' -n --no-heading "<artifactId>openadt</artifactId>" -C2 ||trueecho"---- apps/openadt-cli/pom.xml: test-related plugins ----"
rg -n --no-heading "(maven-surefire-plugin|maven-failsafe-plugin|surefire|failsafe|jacoco|skipTests|testFailure|maven-surefire)" apps/openadt-cli/pom.xml -S ||trueecho"---- apps/openadt-cli/pom.xml: shade/assembly configuration ----"
rg -n --no-heading "(maven-shade-plugin|shade|assembly|classifier|repackage|manifest|mainClass|distribution)" apps/openadt-cli/pom.xml -S ||true
Repository: abapify/openadt
Length of output: 14446
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"---- Find dependencies on org.openadt:openadt in module poms ----"
rg -t xml --iglob 'apps/*/pom.xml' --glob '!apps/openadt-cli/pom.xml' -n \
"<groupId>org\.openadt</groupId>[\s\S]*?<artifactId>openadt</artifactId>" \
-C2 ||trueecho"---- Find any pom dependencies with <artifactId>openadt</artifactId> (show groupId around) ----"
rg -t xml --iglob 'apps/*/pom.xml' -n "<artifactId>openadt</artifactId>" -C3 ||trueecho"---- Check if any pom depends on the openadt-cli module artifactId openadt ----"
rg -t xml --iglob '**/pom.xml' --glob '!**/target/**' -n "<artifactId>openadt</artifactId>" -C1 ||trueecho"---- Search for openadt-cli as a dependency (module name) ----"
rg -t xml --iglob '**/pom.xml' -n "openadt-cli" -C2 ||true
Repository: abapify/openadt
Length of output: 1084
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"---- openadt-cli test sources (files) ----"
find apps/openadt-cli/src/test -type f | head -n 200
echo"---- Search for references to shaded/distribution jar or target paths in openadt-cli tests ----"
rg -n --no-heading "target/|shade|shaded|distribution|openadt.*\\.jar|\\.jar\\b|maven-shade|OpenAdtCommand" apps/openadt-cli/src/test ||trueecho"---- openadt-cli surefire/failsafe config in pom.xml ----"
rg -n --no-heading "(maven-surefire-plugin|maven-failsafe-plugin|surefire|failsafe|skipTests|skipITs)" apps/openadt-cli/pom.xml ||trueecho"---- Check whether any other module depends on the openadt artifactId openadt (global scan) ----"
rg -t xml --iglob 'apps/*/pom.xml' --glob '!apps/openadt-cli/pom.xml' -n "<artifactId>openadt</artifactId>" -C2 ||trueecho"---- Check whether any module depends on org.openadt groupId and artifactId=openadt (global scan) ----"
rg -t xml --iglob 'apps/*/pom.xml' --glob '!apps/openadt-cli/pom.xml' -n "<groupId>org.openadt</groupId>" -C1 | head -n 200
Repository: abapify/openadt
Length of output: 4147
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Locate and show mvnw-runner.mjs behavior (args handling)
ls -la scripts/mvnw-runner.mjs ||true
sed -n '1,220p' scripts/mvnw-runner.mjs ||true# Also check if any nx target overrides args or wraps -am internally
rg -n "mvnw-runner\.mjs" -S .| head -n 50
# Confirm whether openadt-cli:package has dependsOn and whether openadt-cli:test needs deps
cat -n apps/openadt-cli/project.json | sed -n '1,120p'
Repository: abapify/openadt
Length of output: 6399
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"---- package.json scripts ----"
cat -n package.json | sed -n '1,120p'echo"---- Search CI/workflows for nx test openadt-cli ----"
rg -n "nx\s+test\s+openadt-cli|openadt:test" .github package.json .| head -n 200
Repository: abapify/openadt
Length of output: 2270
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"---- Search GitHub workflows for openadt:test or nx test openadt-cli ----"
rg -n "openadt:test|nx\s+test\s+openadt-cli|nx\s+run\s+openadt-cli:test" .github/workflows -S ||trueecho"---- Search workflows for openadt-cli compile/package/install targets ----"
rg -n "nx\s+(run\s+)?openadt-cli:(compile|package|test|ensure-dev-jar)|nx\s+(compile|package|test)\s+openadt-cli" .github/workflows -S ||trueecho"---- Search workflows for any prior installs of openadt-config/openadt-bootstrap/openadt-sap-adt ----"
rg -n "openadt-(config|bootstrap|sap-adt):" .github/workflows -S ||trueecho"---- List workflows ----"
ls -la .github/workflows
#!/bin/bashset -euo pipefail
echo"---- Scan all pom.xml for artifactId openadt (org.openadt module) ----"
rg -t xml --iglob '**/pom.xml' -n "<artifactId>openadt</artifactId>" -C1 ||trueecho"---- Scan all pom.xml for groupId org.openadt + artifactId openadt together ----"
rg -t xml --iglob '**/pom.xml' -n "<groupId>org\.openadt</groupId>" -C1 | head -n 200
Repository: abapify/openadt
Length of output: 3016
Fix apps/openadt-cli/project.json Nx test target dependency chain
targets.test currently depends on ["package"] and runs mvnw-runner.mjs ... -q test -pl apps/openadt-cli ... without first compiling/install-ing openadt-config, openadt-sap-adt, or openadt-bootstrap (those are only ensured by the compile target). In contrast, openadt-bootstrap and openadt-sap-adttest targets depend on compile (and also append an install -DskipTests step).
The missing post-test install for the openadt-cli artifact itself isn’t the main problem (no other module depends on org.openadt:openadt), but the current test chain can leave required Maven dependencies unavailable when nx test openadt-cli is run in isolation.
🤖 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 `@apps/openadt-cli/project.json` around lines 27 - 29, The test target
(targets.test) for apps/openadt-cli currently depends only on "package" and runs
the mvnw-runner.mjs test command without ensuring the project graph is
compiled/installed; update targets.test to depend on "compile" (in addition to
or instead of "package") so openadt-config/openadt-sap-adt/openadt-bootstrap are
built first, and modify the mvnw-runner.mjs invocation used by targets.test to
append an install step (install -DskipTests) after the test phase (matching the
pattern used by openadt-bootstrap and openadt-sap-adt) so required Maven
artifacts are available when running nx test openadt-cli in isolation.
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/openadt-cli/project.json">
<violation number="1" location="apps/openadt-cli/project.json:39">
P1: `package` target dropped Maven `-am` without adding Nx `dependsOn` for upstream modules, breaking isolated builds</violation>
</file>
Reply with feedback, questions, or to request a fix.
Add support for SAP GUI for Java landscape files on macOS:
- Detect ~/Library/Preferences/SAP/SAPGUILandscape.xml
- Detect external landscape URLs from <Include> tags
- Warn users when message server details may be in external landscape
- Update spec to document macOS paths and external landscape behavior
Also includes previous Eclipse.app bundle support for JCo detection.
Use sourcePath in the emitted Include warning context (or remove it).
sourcePath is currently unused, and the log output loses which landscape file triggered the message. Include the file path in the message to keep diagnostics actionable.
Suggested patch
private void detectExternalLandscapeUrls(Document document, Path sourcePath) {
@@
if (url != null) {
- System.err.println("INFO: External SAP landscape URL detected: " + url);+ System.err.println("INFO: External SAP landscape URL detected in " + sourcePath + ": " + url);
System.err.println("INFO: Some system details (message server hostnames) may be in the external landscape.");
System.err.println("INFO: Open SAP GUI once to cache the full landscape, or provide message server manually.");
}
}
}
🤖 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
`@apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SapGuiLandscapeDetector.java`
around lines 62 - 73, The method detectExternalLandscapeUrls(Document document,
Path sourcePath) currently ignores the sourcePath parameter which makes the
emitted Include warnings non-actionable; update the log lines in
detectExternalLandscapeUrls to include the sourcePath (e.g., append
sourcePath.toString() or sourcePath.getFileName()) in the warning messages so
each "External SAP landscape URL detected" and related lines identify which file
triggered them, and ensure sourcePath is null-checked (or if you intentionally
don't need it, remove the sourcePath parameter from the method signature and all
callers).
🤖 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
`@apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SetupPathLocator.java`:
- Around line 113-123: The repeated macOS path literal fragments in
SetupPathLocator (e.g., the Path.of(...) calls that use "Eclipse.app",
"Contents", "Eclipse", "plugins", and "libsapcrypto.dylib") should be extracted
into private static final String constants (for example ECLIPSE_APP,
CONTENTS_DIR, ECLIPSE_DIR, PLUGINS_DIR, LIB_SAP_CRYPTO) and then reused when
constructing paths in the methods that call paths.add(Path.of(...)) (including
the blocks shown and the similar code at the other flagged locations around the
other Path.of calls). Replace the repeated string literals with the new
constants to remove duplication and ensure consistent reuse across
SetupPathLocator.
- Around line 139-147: The macOS path-building in SetupPathLocator currently
wraps both absolute and home-dependent candidates behind a check for non-blank
user.home; update the logic in the method that builds mac paths (the block that
checks os.contains("mac")) to always add the absolute, system-level paths (e.g.,
Path.of("/usr","local","lib") and Path.of("/Library","Java","Extensions"), and
the Eclipse.app OSGi cache at Path.of("/Applications","Eclipse.app",...))
regardless of home.isBlank(), while only adding the user-specific candidates
(e.g., Path.of(home,"lib"), Path.of(home,".p2"),
Path.of(home,"Applications","Eclipse.app",...)) when home is non-blank; apply
the same separation to the analogous macOS block later in the class (the second
occurrence around the other mac block).
---
Nitpick comments:
In
`@apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SapGuiLandscapeDetector.java`:
- Around line 62-73: The method detectExternalLandscapeUrls(Document document,
Path sourcePath) currently ignores the sourcePath parameter which makes the
emitted Include warnings non-actionable; update the log lines in
detectExternalLandscapeUrls to include the sourcePath (e.g., append
sourcePath.toString() or sourcePath.getFileName()) in the warning messages so
each "External SAP landscape URL detected" and related lines identify which file
triggered them, and ensure sourcePath is null-checked (or if you intentionally
don't need it, remove the sourcePath parameter from the method signature and all
callers).
🪄 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
Run ID: 5ba0800b-bd60-4bcb-8367-694c2687cf45
📥 Commits
Reviewing files that changed from the base of the PR and between 51dbe04 and a029df2.
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Deduplicate repeated path literals to satisfy static analysis and reduce path drift risk.
The new macOS path additions repeat literals ("Eclipse.app", "Contents", "Eclipse", "plugins", "libsapcrypto.dylib") that are already flagged by Sonar failures. Promote these to constants and reuse them across the new path builders.
Also applies to: 145-147, 185-188
🧰 Tools🪛 GitHub Check: SonarCloud Code Analysis
[failure] 118-118: Define a constant instead of duplicating this literal "Eclipse" 5 times.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SetupPathLocator.java`
around lines 113 - 123, The repeated macOS path literal fragments in
SetupPathLocator (e.g., the Path.of(...) calls that use "Eclipse.app",
"Contents", "Eclipse", "plugins", and "libsapcrypto.dylib") should be extracted
into private static final String constants (for example ECLIPSE_APP,
CONTENTS_DIR, ECLIPSE_DIR, PLUGINS_DIR, LIB_SAP_CRYPTO) and then reused when
constructing paths in the methods that call paths.add(Path.of(...)) (including
the blocks shown and the similar code at the other flagged locations around the
other Path.of calls). Replace the repeated string literals with the new
constants to remove duplication and ensure consistent reuse across
SetupPathLocator.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
Don’t gate absolute macOS search paths on user.home.
Both macOS blocks skip /usr/local/lib and /Library/... when user.home is blank, even though those paths are independent of home. Split home-dependent and absolute candidates so discovery remains robust in constrained runtimes.
Also applies to: 184-188
🤖 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
`@apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SetupPathLocator.java`
around lines 139 - 147, The macOS path-building in SetupPathLocator currently
wraps both absolute and home-dependent candidates behind a check for non-blank
user.home; update the logic in the method that builds mac paths (the block that
checks os.contains("mac")) to always add the absolute, system-level paths (e.g.,
Path.of("/usr","local","lib") and Path.of("/Library","Java","Extensions"), and
the Eclipse.app OSGi cache at Path.of("/Applications","Eclipse.app",...))
regardless of home.isBlank(), while only adding the user-specific candidates
(e.g., Path.of(home,"lib"), Path.of(home,".p2"),
Path.of(home,"Applications","Eclipse.app",...)) when home is non-blank; apply
the same separation to the analogous macOS block later in the class (the second
occurrence around the other mac block).
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SetupPathLocator.java">
<violation number="1" location="apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SetupPathLocator.java:139">
P2: Absolute system paths (`/usr/local/lib`, `/Library/Java/Extensions`, `/Applications/Eclipse.app/...`) are gated behind `!home.isBlank()`, but they don't depend on the user's home directory. If `user.home` is blank (e.g., in constrained runtimes), these system-wide paths won't be searched. Split the condition so home-dependent paths (like `home/lib`, `home/.p2`) are guarded by the blank check, while absolute paths are only guarded by the OS check.</violation>
</file>
Reply with feedback, questions, or to request a fix.
The reason will be displayed to describe this comment to others. Learn more.
P2: Absolute system paths (/usr/local/lib, /Library/Java/Extensions, /Applications/Eclipse.app/...) are gated behind !home.isBlank(), but they don't depend on the user's home directory. If user.home is blank (e.g., in constrained runtimes), these system-wide paths won't be searched. Split the condition so home-dependent paths (like home/lib, home/.p2) are guarded by the blank check, while absolute paths are only guarded by the OS check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SetupPathLocator.java, line 139:
<comment>Absolute system paths (`/usr/local/lib`, `/Library/Java/Extensions`, `/Applications/Eclipse.app/...`) are gated behind `!home.isBlank()`, but they don't depend on the user's home directory. If `user.home` is blank (e.g., in constrained runtimes), these system-wide paths won't be searched. Split the condition so home-dependent paths (like `home/lib`, `home/.p2`) are guarded by the blank check, while absolute paths are only guarded by the OS check.</comment>
<file context>
@@ -113,6 +132,20 @@ static List<Path> jcoJarRoots() {
+ String home = System.getProperty(USER_HOME_PROPERTY, "");
+
+ // macOS-specific paths for JCo native libraries
+ if (os.contains("mac") && !home.isBlank()) {
+ paths.add(Path.of(home, "lib"));
+ paths.add(Path.of(home, ".p2"));
</file context>
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The code prints the full external landscape url value directly to stderr, which can leak embedded credentials or internal endpoints into CI logs and terminal history. Redact sensitive URL parts (user-info/query/fragment) or avoid printing the raw URL value. [security]
Severity Level: Critical 🚨
- ❌ External landscape URLs with credentials exposed in stderr logs.
- ⚠️ SetupAnalyzer CLI leaks internal SAP endpoints to operators.
Steps of Reproduction ✅
1. Create an SAP GUI landscape file containing an external include, for example `<Includeurl="https://user:secret@example.com/SAPUILandscape.xml"/>`, and place it at a location
returned by `SetupPathLocator.sapGuiLandscapeFiles()` (e.g.
`~/Library/Preferences/SAP/SAPGUILandscape.xml` for macOS as configured in
SetupPathLocator.java:37-42).
2. Run the setup analysis flow, which constructs a default `SetupAnalyzer`
(SetupAnalyzer.java:25-36); its constructor wires in a `SapGuiLandscapeDetector` instance
at line 28 and later calls `analyze()` (SetupAnalyzer.java:45-51), causing each
`SystemDetector` to execute `detect()`.
3. Inside `SapGuiLandscapeDetector.detect()` (SapGuiLandscapeDetector.java:35-47), the
detector iterates over paths from `SetupPathLocator.sapGuiLandscapeFiles()`
(SapGuiLandscapeDetector.java:25-27) and, for each existing file, invokes
`parseLandscapeFile(path)` (SapGuiLandscapeDetector.java:49-56).
4.`parseLandscapeFile()` parses the XML into a `Document`
(SapGuiLandscapeDetector.java:51) and calls `detectExternalLandscapeUrls(document, path)`
(line 55); for each `<Include>` element with a non-blank `url` attribute
(SapGuiLandscapeDetector.java:62-67), `detectExternalLandscapeUrls()` executes
`System.err.println("INFO: External SAP landscape URL detected: " + url);` at line 68,
emitting the full external URL—including any embedded credentials, tokens, or internal
hostnames—directly to stderr and into CLI/CI logs.
This is a comment left during a code review.
**Path:** apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SapGuiLandscapeDetector.java
**Line:** 68:68
**Comment:***Security: The code prints the full external landscape `url` value directly to stderr, which can leak embedded credentials or internal endpoints into CI logs and terminal history. Redact sensitive URL parts (user-info/query/fragment) or avoid printing the raw URL value.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Cached landscape files are filtered with a case-sensitive .endsWith(".xml"), so valid files like .XML are silently skipped and never parsed. Use a case-insensitive extension check (as done elsewhere in this class) to avoid missing landscape entries. [logic error]
Severity Level: Major ⚠️
- ⚠️ Cached SAP GUI landscapes with .XML extensions ignored.
- ⚠️ Message server hostnames missing from detected system profiles.
Steps of Reproduction ✅
1. On macOS, ensure SAP GUI for Java has cached SAP landscape entries in
`~/Library/Preferences/SAP/Cache/SAPUILandscape` (the cache directory constructed in
`sapGuiLandscapeFiles()` at SetupPathLocator.java:41-43), and rename or create a cache
file with an uppercase extension such as `DEV.XML`.
2. Run `new SapGuiLandscapeDetector().detect()` so that its default constructor uses
`SetupPathLocator.sapGuiLandscapeFiles()` (SapGuiLandscapeDetector.java:24-27) to gather
both main landscape and cache files, and `detect()` (SapGuiLandscapeDetector.java:35-47)
iterates over those paths.
3. Inside `sapGuiLandscapeFiles()`, once `Files.exists(cacheDir)` returns true
(SetupPathLocator.java:43), the code lists the cache directory via `Files.list(cacheDir)`
and applies `stream.filter(f -> f.toString().endsWith(".xml"))` at line 45 before adding
entries to the `paths` set at line 46.
4. Because `.endsWith(".xml")` is case-sensitive and operates on the raw path string, a
cache file named `DEV.XML` does not match the filter and is excluded, unlike other XML
detection paths in this class that normalize to lowercase (see `listXmlFiles()` at
SetupPathLocator.java:31-39); as a result, `SapGuiLandscapeDetector.parseLandscapeFile()`
(SapGuiLandscapeDetector.java:49-56) never parses that cached landscape, and any message
server details present only in the cache remain absent from detected `SystemProfile`
entries.
This is a comment left during a code review.
**Path:** apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SetupPathLocator.java
**Line:** 45:46
**Comment:***Logic Error: Cached landscape files are filtered with a case-sensitive `.endsWith(".xml")`, so valid files like `.XML` are silently skipped and never parsed. Use a case-insensitive extension check (as done elsewhere in this class) to avoid missing landscape entries.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
- Modify SecureLoginHubClient to try API v3 (Windows) first
- Fall back to API v2 (macOS) if v3 fails
- Note: macOS Secure Login Client uses incompatible API
- Requires further investigation for full macOS SNC support
Related: macOS Secure Login hub returns 'Only requests in SLC namespace supported'
The reason will be displayed to describe this comment to others. Learn more.
❌ New issue: Code Duplication
The module contains 4 functions with similar structure: loginWebAdapterV2,loginWebAdapterV3,webAdapterStatusV2,webAdapterStatusV3
Consider extracting shared v2/v3 logic to reduce duplication.
The v2 and v3 status methods (lines 99-111 vs 113-125) differ only in the endpoint path (/slc3/api/status vs /slc2/api/status) and error message. The same duplication exists for the login methods (lines 140-163 vs 165-188).
♻️ Proposed refactor to extract shared logic
+private String webAdapterStatusForApi(String apiVersion, String encodedProfileId) throws IOException, InterruptedException {+ URI uri = URI.create(hubBaseUrl + "/slc" + apiVersion + "/api/status?profileid=" + encodedProfileId);+ HttpResponse<String> response = httpClient.send(+ hubRequest(uri).GET().build(),+ HttpResponse.BodyHandlers.ofString()+ );+ if (response.statusCode() != 200) {+ String versionLabel = "3".equals(apiVersion) ? "" : " v" + apiVersion;+ throw new IOException("Secure Login hub" + versionLabel + " status failed with HTTP " + response.statusCode());+ }+ JsonNode root = objectMapper.readTree(response.body());+ JsonNode status = root.path("status");+ return status.isTextual() ? status.asText() : "UNKNOWN";+}+
private String webAdapterStatusV3(String encodedProfileId) throws IOException, InterruptedException {
- URI uri = URI.create(hubBaseUrl + "/slc3/api/status?profileid=" + encodedProfileId);- HttpResponse<String> response = httpClient.send(- hubRequest(uri).GET().build(),- HttpResponse.BodyHandlers.ofString()- );- if (response.statusCode() != 200) {- throw new IOException("Secure Login hub status failed with HTTP " + response.statusCode());- }- JsonNode root = objectMapper.readTree(response.body());- JsonNode status = root.path("status");- return status.isTextual() ? status.asText() : "UNKNOWN";+ return webAdapterStatusForApi("3", encodedProfileId);
}
private String webAdapterStatusV2(String encodedProfileId) throws IOException, InterruptedException {
- URI uri = URI.create(hubBaseUrl + "/slc2/api/status?profileid=" + encodedProfileId);- HttpResponse<String> response = httpClient.send(- hubRequest(uri).GET().build(),- HttpResponse.BodyHandlers.ofString()- );- if (response.statusCode() != 200) {- throw new IOException("Secure Login hub v2 status failed with HTTP " + response.statusCode());- }- JsonNode root = objectMapper.readTree(response.body());- JsonNode status = root.path("status");- return status.isTextual() ? status.asText() : "UNKNOWN";+ return webAdapterStatusForApi("2", encodedProfileId);
}
Apply the same pattern for the login methods using a shared loginWebAdapterForApi(String apiVersion, String profileId, boolean browserMonitor) helper.
Also applies to: 165-188
🤖 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
`@apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java`
around lines 113 - 125, The webAdapterStatusV2 logic is duplicated with the v3
variant; extract a shared helper like webAdapterStatus(String apiVersion, String
encodedProfileId) that builds the URI using apiVersion (e.g., "slc2" or "slc3"),
sends the request via hubRequest(...), checks the status code and throws the
same IOException with a consistent message, then parses and returns the "status"
text; similarly refactor the two login methods into a single
loginWebAdapterForApi(String apiVersion, String profileId, boolean
browserMonitor) that selects the endpoint path based on apiVersion and contains
the shared request/response handling so callers of
webAdapterStatusV2/webAdapterStatusV3 and the two login methods can delegate to
these helpers.
🤖 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
`@apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java`:
- Around line 89-97: In webAdapterStatus(String profileId) and similarly in
loginWebAdapter, preserve the original v3 IOException when falling back to v2 by
catching the v3 exception, logging an explicit message that v3 failed and v2 is
being attempted (include the v3 exception message), then call the v2 method and
if v2 also throws, add the saved v3 IOException as a suppressed exception on the
v2 exception before rethrowing; also consider narrowing the catch to only the
specific exceptions that should trigger a fallback (rather than broad
IOException) so transient/network errors can be handled appropriately.
- Around line 89-97: Tests only cover the v3 path for webAdapterStatus; add unit
tests in SecureLoginHubClientTest to exercise the v3→v2 fallback and both-fail
cases by stubbing/mocking webAdapterStatusV3 and webAdapterStatusV2 behavior:
create tests where webAdapterStatusV3(encodedProfileId) throws different
IOExceptions (e.g., simulate 404, timeout, parse error) and assert that
webAdapterStatus then calls webAdapterStatusV2(encodedProfileId) and returns its
success result, and add tests where both webAdapterStatusV3 and
webAdapterStatusV2 throw to assert the propagated exception/behavior; reference
the methods webAdapterStatus, webAdapterStatusV3, webAdapterStatusV2 to locate
and mock the behavior in the test.
---
Nitpick comments:
In
`@apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java`:
- Around line 113-125: The webAdapterStatusV2 logic is duplicated with the v3
variant; extract a shared helper like webAdapterStatus(String apiVersion, String
encodedProfileId) that builds the URI using apiVersion (e.g., "slc2" or "slc3"),
sends the request via hubRequest(...), checks the status code and throws the
same IOException with a consistent message, then parses and returns the "status"
text; similarly refactor the two login methods into a single
loginWebAdapterForApi(String apiVersion, String profileId, boolean
browserMonitor) that selects the endpoint path based on apiVersion and contains
the shared request/response handling so callers of
webAdapterStatusV2/webAdapterStatusV3 and the two login methods can delegate to
these helpers.
🪄 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
Run ID: 0c7aaa6a-eab3-4f4a-b72d-c6ef2296f540
📥 Commits
Reviewing files that changed from the base of the PR and between 9d14808 and 0c69767.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
Preserve v3 error context when falling back to v2.
When v3 fails with IOException and the fallback to v2 also fails, only the v2 error is propagated—the original v3 failure is lost. This makes debugging harder because users won't know that v3 was attempted first or why it failed.
Additionally, the broad IOException catch means any failure (network timeout, parse error, HTTP 404) triggers the v2 fallback. If v3 fails due to a network issue, v2 will likely fail the same way, and the dual attempt adds latency without helping the user understand the root cause.
🔍 Suggested improvement: Add suppressed exception and logging
public String webAdapterStatus(String profileId) throws IOException, InterruptedException {
String encodedProfileId = URLEncoder.encode(profileId, StandardCharsets.UTF_8);
// Try API v3 first (Windows), fall back to v2 (macOS)
try {
return webAdapterStatusV3(encodedProfileId);
} catch (IOException e) {
+ CliLog.info("Secure Login hub API v3 status failed (" + e.getMessage() + "), trying v2 fallback");+ try {
return webAdapterStatusV2(encodedProfileId);
+ } catch (IOException v2Error) {+ v2Error.addSuppressed(e);+ throw v2Error;+ }
}
}
This preserves the v3 error as a suppressed exception and logs the fallback attempt. Apply the same pattern to loginWebAdapter at lines 131-138.
Note: The same issue and fix apply to the loginWebAdapter fallback at lines 131-138.
🤖 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
`@apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java`
around lines 89 - 97, In webAdapterStatus(String profileId) and similarly in
loginWebAdapter, preserve the original v3 IOException when falling back to v2 by
catching the v3 exception, logging an explicit message that v3 failed and v2 is
being attempted (include the v3 exception message), then call the v2 method and
if v2 also throws, add the saved v3 IOException as a suppressed exception on the
v2 exception before rethrowing; also consider narrowing the catch to only the
specific exceptions that should trigger a fallback (rather than broad
IOException) so transient/network errors can be handled appropriately.
⚠️ Potential issue | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Description: Search for tests covering the v2 fallback behavior# Search for test methods that might cover v2 fallback
rg -n -C5 'slc2/api/(status|login)' --type=java --glob='*Test.java'# Search for test assertions that check fallback behavior
rg -n -C3 '(webAdapterStatus|loginWebAdapter).*fallback|v2.*fallback|IOException.*v2' --type=java --glob='*Test.java'
Repository: abapify/openadt
Length of output: 41
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Locate the main test class for SecureLoginHubClient
fd -i 'SecureLoginHubClientTest.java' -a .||true
fd -i 'SecureLoginHubClient*Test.java' -a .||true# Find direct references/usages of v2 fallback helpers in tests
rg -n --glob='*Test.java''webAdapterStatusV2|loginWebAdapterV2|webAdapterStatusV3|loginWebAdapterV3'.||true# Search for any stubbing/mocking of slc2/api endpoints for v2 specifically
rg -n --glob='*Test.java''slc2/api'.||true
rg -n --glob='*Test.java''slc2/api/.*(v2|_v2|version.?2)'.||true# Search for fallback-related assertions/flow in tests
rg -n --glob='*Test.java''(fallback|v3 fails|v2 succeeds|catch \(IOException|IOException.*v2)'.||true# Narrow search to the secure login hub client test files (if present)
TESTS="$(fd -i 'SecureLoginHubClient*Test.java'. -a || true)"if [ -n"$TESTS" ];then
rg -n --glob='*Test.java''webAdapterStatus|loginWebAdapter'$TESTS||truefi
Repository: abapify/openadt
Length of output: 2652
🏁 Script executed:
#!/bin/bashset -euo pipefail
TEST_FILE="apps/openadt-sap-adt/src/test/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClientTest.java"# Show key parts around the existing assertions
nl -ba "$TEST_FILE"| sed -n '1,220p'# Targeted searches inside the test file for v2/v3/fallback hintsecho"---- rg v2/v3/fallback in SecureLoginHubClientTest.java ----"
rg -n --glob='*.java''(webAdapterStatusV[23]|loginWebAdapterV[23]|fallback|v3|v2|slc2/api|/v2|/v3|statusV[23])'"$TEST_FILE"||trueecho"---- Search within the test file for IOException-driven fallback patterns ----"
rg -n --glob='*.java''(catch\s*\(\s*IOException|throws\s*IOException|IOException)'"$TEST_FILE"||true# Also find any helper test client setup that might indicate endpoint versions
rg -n --glob='*.java''(mock|stub|wireMock|server|endpoint|url|baseUrl|host|scheme)'"$TEST_FILE"||true
Repository: abapify/openadt
Length of output: 102
🏁 Script executed:
#!/bin/bashset -euo pipefail
TEST_FILE="apps/openadt-sap-adt/src/test/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClientTest.java"# Show key parts around the existing assertions using cat -n
sed -n '1,220p'"$TEST_FILE"| cat -n
echo"---- rg v2/v3/fallback hints in SecureLoginHubClientTest.java ----"
rg -n --glob='*.java''(webAdapterStatusV[23]|loginWebAdapterV[23]|fallback|v3|v2|slc2/api|/v2|/v3|statusV[23])'"$TEST_FILE"||trueecho"---- Search within the test file for IOException-driven fallback patterns ----"
rg -n --glob='*.java''(catch\s*\(\s*IOException|throws\s*IOException|IOException)'"$TEST_FILE"||trueecho"---- Search within the test file for mocking/endpoint/version hints ----"
rg -n --glob='*.java''(mock|stub|wireMock|server|endpoint|url|baseUrl|host|scheme)'"$TEST_FILE"||true
Repository: abapify/openadt
Length of output: 9940
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Search all test sources for v2 (slc2) endpoint usage and fallback mentions
rg -n --glob='*Test.java''slc2/api/(status|login)'.||true
rg -n --glob='*Test.java''(webAdapterStatusV2|loginWebAdapterV2|webAdapterStatusV3|loginWebAdapterV3)'.||true
rg -n --glob='*Test.java''(webAdapterStatus|loginWebAdapter).*catch\s*\(IOException|fallback|v2|slc2)'.||true
Repository: abapify/openadt
Length of output: 260
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Search all test sources for v2 (slc2) endpoint usage
rg -n --glob='*Test.java''slc2/api/(status|login)'.||true# Search all test sources for explicit v2/v3 helper method names
rg -n --glob='*Test.java''(webAdapterStatusV[23]|loginWebAdapterV[23])'.||true# Search for fallback wording and IOException patterns without problematic regex
rg -n --glob='*Test.java''fallback'.||true
rg -n --glob='*Test.java''IOException'.||true
rg -n --glob='*Test.java''v2'.||true
rg -n --glob='*Test.java''v3'.||true
Repository: abapify/openadt
Length of output: 10077
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Search for v2 endpoint paths (secure login hub v2)
rg -n --glob='*Test.java''/slc2/api/(status|login)'.||true# Search for v3 endpoint paths (already seen /slc3/api/status in SecureLoginHubClientTest)
rg -n --glob='*Test.java''/slc3/api/(status|login)'.||true# Search for direct references to the v2 helper method names
rg -n --glob='*Test.java''webAdapterStatusV2|loginWebAdapterV2'.||true# Search for try/catch patterns mentioning v3->v2 fallback in tests
rg -n --glob='*Test.java''webAdapterStatusV3.*catch|loginWebAdapterV3.*catch|fallback to v2|fall back to v2|v2 \\(macOS\\)'.||true
Repository: abapify/openadt
Length of output: 211
Add test coverage for SecureLoginHubClient’s v3→v2 fallback paths
SecureLoginHubClientTest only validates the v3 status endpoint (assertEquals("/slc3/api/status", lastPath)) and doesn’t exercise any /slc2/api/... behavior or the “v3 throws → v2 succeeds/fails” fallback scenarios. Extend tests to cover:
v3 failing with different IOException causes (e.g., 404, timeout, response/parse issues) and v2 succeeding afterward
both v3 and v2 failing (assert the resulting exception/behavior)
Also applies to: 131-138
🤖 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
`@apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java`
around lines 89 - 97, Tests only cover the v3 path for webAdapterStatus; add
unit tests in SecureLoginHubClientTest to exercise the v3→v2 fallback and
both-fail cases by stubbing/mocking webAdapterStatusV3 and webAdapterStatusV2
behavior: create tests where webAdapterStatusV3(encodedProfileId) throws
different IOExceptions (e.g., simulate 404, timeout, parse error) and assert
that webAdapterStatus then calls webAdapterStatusV2(encodedProfileId) and
returns its success result, and add tests where both webAdapterStatusV3 and
webAdapterStatusV2 throw to assert the propagated exception/behavior; reference
the methods webAdapterStatus, webAdapterStatusV3, webAdapterStatusV2 to locate
and mock the behavior in the test.
The reason will be displayed to describe this comment to others. Learn more.
3 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java">
<violation number="1" location="apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java:94">
P2: Fallback from v3 to v2 catches all IOExceptions, masking genuine v3 errors (auth failures, server errors) by making a second request to v2</violation>
<violation number="2" location="apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java:95">
P2: Preserve the original v3 IOException when falling back to v2. If v2 also fails, this currently drops the initial v3 failure context, making root-cause diagnosis harder.</violation>
<violation number="3" location="apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java:136">
P2: Retain the original v3 IOException in the login fallback path. When the v2 call also throws, the initial v3 failure should be attached (for example as a suppressed exception) so error reporting keeps full context.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
The reason will be displayed to describe this comment to others. Learn more.
P2: Fallback from v3 to v2 catches all IOExceptions, masking genuine v3 errors (auth failures, server errors) by making a second request to v2
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java, line 94:
<comment>Fallback from v3 to v2 catches all IOExceptions, masking genuine v3 errors (auth failures, server errors) by making a second request to v2</comment>
<file context>
@@ -88,6 +88,15 @@ public boolean isReachable() {
+ // Try API v3 first (Windows), fall back to v2 (macOS)
+ try {
+ return webAdapterStatusV3(encodedProfileId);
+ } catch (IOException e) {
+ return webAdapterStatusV2(encodedProfileId);
+ }
</file context>
The reason will be displayed to describe this comment to others. Learn more.
P2: Retain the original v3 IOException in the login fallback path. When the v2 call also throws, the initial v3 failure should be attached (for example as a suppressed exception) so error reporting keeps full context.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java, line 136:
<comment>Retain the original v3 IOException in the login fallback path. When the v2 call also throws, the initial v3 failure should be attached (for example as a suppressed exception) so error reporting keeps full context.</comment>
<file context>
@@ -101,11 +110,34 @@ public String webAdapterStatus(String profileId) throws IOException, Interrupted
+ try {
+ loginWebAdapterV3(profileId, browserMonitor);
+ } catch (IOException e) {
+ loginWebAdapterV2(profileId, browserMonitor);
+ }
+ }
</file context>
The reason will be displayed to describe this comment to others. Learn more.
P2: Preserve the original v3 IOException when falling back to v2. If v2 also fails, this currently drops the initial v3 failure context, making root-cause diagnosis harder.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/openadt-sap-adt/src/main/java/org/openadt/sap/adt/bootstrap/SecureLoginHubClient.java, line 95:
<comment>Preserve the original v3 IOException when falling back to v2. If v2 also fails, this currently drops the initial v3 failure context, making root-cause diagnosis harder.</comment>
<file context>
@@ -88,6 +88,15 @@ public boolean isReachable() {
+ try {
+ return webAdapterStatusV3(encodedProfileId);
+ } catch (IOException e) {
+ return webAdapterStatusV2(encodedProfileId);
+ }
+ }
</file context>
Suggested change
returnwebAdapterStatusV2(encodedProfileId);
try {
returnwebAdapterStatusV2(encodedProfileId);
} catch (IOExceptionv2Error) {
v2Error.addSuppressed(e);
throwv2Error;
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
codeant-ai
Bot
added
the
size:M
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
User description
Summary by cubic
Fixes macOS/Windows builds without the SAP SDK by using the
distributionprofile and localinstalls. Adds macOS SAP GUI for Java and cached landscape support, broader JCo/SAP Crypto lookup paths, external landscape logging, and a macOS Secure Login API v2 fallback.Bug Fixes
compiletoinstallwith-Pdistribution -Dopenadt.distribution=true -DskipTestsinopenadt-bootstrap,openadt-cli,openadt-config,openadt-sap-adt.distributionprofile; added post-testinstallwhere needed.install -Nbeforeopenadt-configto register the parent POM.-amto avoid modules that require the SAP SDK.packagetargets use thedistributionprofile and skip tests.New Features
~/Library/Preferences/SAP/SAPGUILandscape.xmland cached landscapes at~/Library/Preferences/SAP/Cache/SAPUILandscape/*.xml; extract message server details; log info when<Include url="...">points to external landscapes..p2pool, system lib dirs) and SAP Crypto candidates.specs/setup.mdwith macOS paths and external landscape behavior.Written for commit 0c69767. Summary will update on new commits.
Summary by CodeRabbit
Chores
New Features
Documentation
CodeAnt-AI Description
Fix cross-platform builds and add macOS SAP setup support
What Changed
Impact
✅ Fewer build failures on macOS and Windows✅ Easier first-time setup on macOS✅ Fewer missing SAP landscape details✅ More reliable Secure Login on macOS💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.