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
Build devcontainer image with SAP ADT p2 plugins and \libsapjco3.so\ at /opt/openadt\ using @abapify/p2-cli\ in the Dockerfile (Codespaces-friendly, no host bootstrap required for SDK).
Simplify \postCreate\ / devcontainer bootstrap to prefer image SDK paths and optional --provision-p2-sdk\ for custom images.
Extend \SetupPathLocator\ with /opt/openadt\ and staged p2 plugin roots; fix Windows PowerShell path for runtime prepare.
Document devcontainer usage in \docs/usage.md; align release workflow and version tooling with parent \pom.xml.
Test plan
Rebuild devcontainer image and confirm /opt/openadt/dist/p2/plugins\ and \libsapjco3.so\ exist.
Run \�un run bootstrap:devcontainer -- --non-interactive --skip-if-missing --container-workspace /workspaces/openadt\ in container; verify
untime.openadt.toml\ points at image paths.
\mvnw -q test -pl apps/openadt-bootstrap -Dtest=SetupPathLocatorTest\
\�un scripts/verify-spec-sync.ts\ and \�un scripts/verify-package-docs.ts\ (if behavior docs touched)
Bake the SAP ADT SDK into the devcontainer image at /opt/openadt for zero host setup. Bootstrap prefers the image SDK, can optionally provision p2 for custom images, and improves PATH/LD_LIBRARY_PATH handling.
New Features
Image builds SAP ADT p2 and extracts libsapjco3.so with @abapify/p2-cli into /opt/openadt/dist/p2 and /opt/openadt/dist/jco; exports OPENADT_SDK_ROOT and preserves container LD_LIBRARY_PATH while prepending image JCo.
Bootstrap detects the image SDK, writes adt_plugins_dir, supports --provision-p2-sdk/--skip-p2-sdk, and extracts Linux JCo native from the fragment; image SDK is “ready” only when required bundles and a resolvable JCo jar exist.
Locator searches /opt/openadt and staged .devcontainer/dist/p2/plugins and jco; devcontainer.json points envs at the image SDK; docs updated for the image-first flow.
Bug Fixes
Windows: use System32\WindowsPowerShell\v1.0\powershell.exe; prepend trusted dirs to PATH (don’t replace) so mvnw finds Java; robust SystemRoot resolution.
Bootstrap: deterministic JCo fragment selection and improved regex; LD_LIBRARY_PATH now preserves ${containerEnv:LD_LIBRARY_PATH}.
Safer OPENADT_SDK_ROOT parsing with fallback to /opt/openadt; minor CI/format fixes and Dockerfile adds wget.
Written for commit 45274cc. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Resolved PowerShell path resolution issues on Windows systems for improved launcher reliability.
Chores
Integrated SAP ADT SDK into development container for enhanced development environment.
Updated Maven build configuration and release tooling for consistency.
Documentation
Updated devcontainer setup and usage documentation with revised bootstrap procedures.
Codespaces and devcontainers get p2 plugins and libsapjco3.so at build time via @abapify/p2-cli; postCreate only merges config and optional SNC staging. Bootstrap and SetupPathLocator prefer image paths; release tooling reads parent pom version.
Co-authored-by: Cursor <cursoragent@cursor.com>
This PR introduces SAP ADT SDK installation to the devcontainer image and refactors version management to use the root POM. The image now bakes p2 plugins and JCo libraries into /opt/openadt. Bootstrap path discovery, Windows runtime preparation, and the devcontainer bootstrap tool are updated to locate and use these SDK artifacts. Release scripts are refactored to read version from the root openadt-parent artifact instead of the CLI submodule, with corresponding workflow updates.
Changes
SAP ADT SDK Devcontainer Integration
Layer / File(s)
Summary
Devcontainer image SDK installation and environment .devcontainer/Dockerfile, .devcontainer/devcontainer.json, package.json
Dockerfile adds wget and a build stage that downloads SAP ADT p2 and JCo artifacts into /opt/openadt/dist/ via @abapify/p2-cli. devcontainer.json defines OPENADT_SDK_ROOT env var and expands LD_LIBRARY_PATH to include image SDK and workspace library paths. @abapify/p2-cli is added to devDependencies.
Bootstrap path discovery for image and workspace SDK apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SetupPathLocator.java, apps/openadt-bootstrap/src/test/java/org/openadt/bootstrap/SetupPathLocatorTest.java
SetupPathLocator adds new openadtSdkRoot() helper that resolves /opt/openadt from OPENADT_SDK_ROOT env var or fallback, then includes dist/p2/plugins and dist/jco paths in jar and native search roots alongside staged workspace paths. Test verifies staged P2 directory inclusion.
Windows PowerShell and runtime preparation apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SetupRuntimePreparer.java, packaging/windows/launcher/OpenAdtLauncher.cs, packaging/windows/prepare-openadt-runtime.ps1
SetupRuntimePreparer adds System32 to PowerShell path and constructs Windows PATH from ordered prefixes prepended to inherited PATH. OpenAdtLauncher resolves powershell.exe to absolute path under System32. prepare-openadt-runtime.ps1 checks for mvnw.cmd at source root and runs Maven with explicit pom.xml and module restriction.
Added --provision-p2-sdk and --skip-p2-sdk CLI flags; extends types to track P2/JCo paths and sapcrypto. Implements image SDK readiness check, JCo version parsing, libsapjco3.so extraction, and @abapify/p2-cli download integration. Introduces ensureP2Sdk(...) to reuse or download SDK and finalizeDevcontainer(...) to centralize SDK setup, runtime TOML generation, and entrypoint updates. Refactors main() to invoke finalization after runtime detection or staging.
Devcontainer usage documentation docs/usage.md
Updated "Devcontainer Usage" section to describe image-time SDK installation, config merging in postCreate, and explicit bun run bootstrap:devcontainer commands for re-downloading p2 and staging sapcrypto from local archives.
Release Version Management Refactoring
Layer / File(s)
Summary
Root POM version sourcing in release scripts tools/release-version/src/main.ts, tools/package-release/src/main.ts
tools/release-version now reads root pom.xml for openadt-parent version and updates version in multiple POM files (root and apps/*) by locating artifactId marker and replacing the nearest version tag. tools/package-release extracts openadt-parent version from root pom.xml and resolves jar from CLI target directory with release/snapshot fallback.
Release workflow and staged file updates .github/workflows/release.yml
Version-bump commit now stages pom.xml and packaging/ directory instead of apps/openadt-cli/pom.xml, aligning with root POM refactoring.
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~75 minutes
Poem
🐰 A Rabbit's Ode to SDK Stacks
The devcontainer now bakes its p2,
While bootstrap paths find roots anew,
Release scripts trace the parent's way,
From root to apps in one display—
No more hop-skip through modules old,
Just unified versions, strong and bold! 🎉
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
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.
Description check
❓ Inconclusive
The description provides a comprehensive summary of changes across multiple areas (image building, bootstrap logic, path handling, Windows fixes, release tooling, and docs) and includes a detailed test plan, but omits critical template sections like spec/docs review checkboxes.
Add the required template checklist sections (Spec/docs, Verification, Package map) with explicit checkmarks or notes to meet repository standards.
✅ Passed checks (3 passed)
Check name
Status
Explanation
Title check
✅ Passed
The title 'feat(devcontainer): bake SAP ADT SDK into image at /opt/openadt' clearly and concisely summarizes the main change—building the SAP ADT SDK into the devcontainer image at a specific path.
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.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches📝 Generate docstrings
Create stacked PR
Commit on current branch
🧪 Generate unit tests (beta)
Create PR with unit tests
Commit unit tests in branch feat/devcontainer-image-sdk
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.
Bake SAP ADT SDK into devcontainer image and simplify bootstrap
✨ Enhancement
Walkthroughs
Description
• Bake SAP ADT SDK into devcontainer image at /opt/openadt using @abapify/p2-cli
• Simplify postCreate bootstrap to prefer image SDK paths with optional --provision-p2-sdk
• Extend SetupPathLocator with /opt/openadt paths and fix Windows PowerShell path resolution
• Align release tooling to read version from parent pom.xml instead of CLI module
• Update devcontainer documentation and add p2 plugin download capability
Diagram
flowchart LR
A["Dockerfile builds image"] -->|"@abapify/p2-cli download"| B["SAP ADT p2 plugins at /opt/openadt"]
B -->|"Extract libsapjco3.so"| C["JCo native library staged"]
D["postCreate bootstrap"] -->|"Check imageSdkReady()"| E{SDK in image?}
E -->|"Yes"| F["Use /opt/openadt paths"]
E -->|"No + --provision-p2-sdk"| G["Download p2 to workspace"]
F --> H["Write runtime.openadt.toml"]
G --> H
I["SetupPathLocator"] -->|"Search order"| J["Image SDK → Staged → User homes"]
1. WORKDIR points to deleted dir✓ Resolved🐞 Bug☼ Reliability
Description
The Dockerfile sets WORKDIR /tmp/openadt-image-sdk and then deletes /tmp/openadt-image-sdk,
leaving the final image configured with a non-existent working directory. This can make containers
fail to start with a CWD/chdir error depending on how the runtime launches the container.
The image’s WORKDIR is set to /tmp/openadt-image-sdk and that exact directory is deleted later in
the same Dockerfile, with no subsequent WORKDIR reset shown.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`.devcontainer/Dockerfile` sets `WORKDIR` to a temporary build directory and later deletes that directory, leaving the produced image with an invalid default working directory.
## Issue Context
A container’s configured working directory must exist at runtime; otherwise startup can fail before any entrypoint/command logic runs.
## Fix Focus Areas
- .devcontainer/Dockerfile[60-75]
## Suggested fix
- Avoid persisting the temp `WORKDIR` into the final image, e.g.:
- Replace `WORKDIR /tmp/openadt-image-sdk` with `RUN cd /tmp/openadt-image-sdk && ...`, or
- Add a final `WORKDIR /` (or another valid dir) after the cleanup step.
- If you keep `WORKDIR`, don’t `rm -rf` the working directory (or recreate it after cleanup).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. findLatestJcoPluginJar matches underscore jar 📘 Rule violation≡ Correctness
Description
The devcontainer bootstrap now selects com.sap.conn.jco_<version>.jar from the p2 plugins
directory (and can fall back to sapjco3.jar) for runtime.jco_jar. This violates the requirement
that the core JCo jar must use hyphen-version naming (com.sap.conn.jco-<version>.jar), which can
cause JCo to reject the jar at runtime.
PR Compliance ID 732535 requires referencing the core JCo jar using hyphen-version naming, not the
Eclipse p2 underscore format. The new findLatestJcoPluginJar() only matches
com.sap.conn.jco_<version>.jar, and resolveDevcontainerRuntime() persists that path as
jco_jar, which violates the rule.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The devcontainer bootstrap resolves `runtime.jco_jar` to an Eclipse p2-style jar name (`com.sap.conn.jco_<version>.jar`) and may also fall back to `sapjco3.jar`. Per compliance requirements, the core JCo jar must be referenced with hyphen-version naming (`com.sap.conn.jco-<version>.jar`).
## Issue Context
`findLatestJcoPluginJar()` explicitly matches underscore naming and `resolveDevcontainerRuntime()` writes the selected path into `runtime.openadt.toml`.
## Fix Focus Areas
- tools/devcontainer-bootstrap/src/main.ts[500-717]
- tools/devcontainer-bootstrap/src/main.ts[719-737]
- .devcontainer/Dockerfile[57-75]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
tools/devcontainer-bootstrap can write runtime.adt_plugins_dir to the workspace p2 path even
when the image SDK is not ready and --provision-p2-sdk was not used, which forces SDK transport
selection. When SDK transport initializes, it throws because the configured plugins directory does
not exist.
If the image SDK isn’t ready and --provision-p2-sdk isn’t set, ensureP2Sdk() returns without
creating plugins; resolveDevcontainerRuntime() still sets adtPluginsDir to the workspace p2 path
and writeDevcontainerRuntime() writes it. SDK transport selection checks only for non-blank
adt_plugins_dir, and SDK classloader initialization throws when that directory is missing.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`tools/devcontainer-bootstrap` writes `adt_plugins_dir` unconditionally in `runtime.openadt.toml` even when neither the image-baked SDK is present nor p2 provisioning has happened.
This makes OpenADT select SDK transport purely because the config value is non-empty, and SDK startup then fails fast because the plugins directory doesn’t exist.
## Issue Context
- SDK transport is selected when `runtime.adt_plugins_dir` is non-blank.
- SDK bootstrap requires that directory to exist.
- `ensureP2Sdk()` returns early when `--provision-p2-sdk` is not set.
## Fix Focus Areas
- tools/devcontainer-bootstrap/src/main.ts[612-736]
## Suggested fix
- Make `adtPluginsDir` optional in `DevcontainerRuntimePaths`.
- In `resolveDevcontainerRuntime(...)`:
- For image SDK: keep current behavior.
- For workspace SDK: set `adtPluginsDir` **only if** `hasRequiredAdtBundles(p2PluginsDir(root))` (and/or `existsSync(p2PluginsDir(root))`). Otherwise set it to `undefined`.
- In `writeDevcontainerRuntime(...)`:
- Only emit `adt_plugins_dir = ...` when `runtime.adtPluginsDir` is defined/non-empty.
This preserves the intended fallback behavior (rest-rfc/http) when SDK plugins aren’t actually available.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
4. Bun install bloats image 🐞 Bug➹ Performance
Description
The Dockerfile copies the repo root package.json and runs bun install, which installs all
devDependencies (including heavy ones like Playwright) just to run @abapify/p2-cli. This
unnecessarily increases devcontainer image build time and the chance of build failures unrelated to
the ADT SDK download.
The Dockerfile installs dependencies from the repo’s root package.json, and that package.json
includes multiple non-SDK devDependencies such as Playwright, Nx, and TypeScript that will be
installed into the image layer.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The image bake step runs `bun install` against the repository’s full devDependency set to obtain `@abapify/p2-cli`, which is overkill and can significantly slow down image builds.
## Issue Context
`package.json` includes additional devDependencies (e.g. `playwright`, `nx`) that aren’t needed to download p2 plugins.
## Fix Focus Areas
- .devcontainer/Dockerfile[60-67]
- package.json[17-29]
## Suggested fix options
1) **Minimal temporary package.json in Docker build** (recommended):
- Create a small `/tmp/openadt-image-sdk/package.json` containing only `@abapify/p2-cli` in `dependencies`.
- Run `bun install --production` there.
2) **Move p2-cli to `dependencies` + production install**:
- Move `@abapify/p2-cli` from `devDependencies` to `dependencies`.
- Use `bun install --production` in the Dockerfile.
3) Use a one-off runner if supported (e.g. `bunx @abapify/p2-cli ...`) to avoid installing the full repo toolchain.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The reason will be displayed to describe this comment to others. Learn more.
Summary
This PR introduces a devcontainer image with SAP ADT SDK baked into /opt/openadt, simplifying bootstrap for Codespaces. The overall approach is solid, but I've identified 4 critical defects that must be fixed before merge.
Critical Issues Found
Security: Download without integrity verification - Dockerfile downloads from external URL without checksum validation (CWE-494)
Logic: Shell glob pattern failure handling - Unquoted glob can cause silent failures in Dockerfile
Logic: Missing directory existence check - findLatestJcoPluginJar crashes if directory doesn't exist
Logic: Missing directory check before readdirSync - Bootstrap can crash if image SDK path is invalid
All issues have been commented inline with specific fixes.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request bakes the SAP ADT SDK into the devcontainer image, updates runtime path resolution, and refactors POM version management. The review feedback highlights several critical improvements: first, when updating the parent POM version, child POMs' parent blocks must also be updated to avoid Maven build failures; second, empty segments in the child process path resolution should be filtered out to prevent an untrusted search path vulnerability (CWE-426); and finally, the hardcoded /opt/openadt paths in SetupPathLocator.java should respect the OPENADT_SDK_ROOT environment variable.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The PR successfully implements the baking of the SAP ADT SDK into the devcontainer image and updates the bootstrap utility, aligning with the primary goals. While Codacy analysis indicates the changes are 'up to standards', the review highlights several implementation gaps and risks.
Several acceptance criteria related to logic for detecting image-baked SDKs, JCo fragment extraction, and host-to-container path mapping lack corresponding automated test scenarios. Furthermore, the versioning tools rely on fragile regular expressions that are sensitive to line endings and XML tag ordering. These issues should be addressed to ensure the robustness of the release pipeline and the bootstrap utility.
About this PR
The complex logic introduced in tools/devcontainer-bootstrap (specifically version comparison, jar extraction, and path mapping) lacks automated unit tests. Given the criticality of this utility for environment setup, consider adding coverage for these helper functions.
Test suggestions
Verify SetupPathLocator correctly identifies staged p2 plugins in the .devcontainer directory.
Verify the devcontainer-bootstrap logic for detecting a valid image-baked SDK (imageSdkReady).
Verify the extraction of libsapjco3.so from a JCo fragment jar (stageJcoNativeFromFragment).
Verify that the bootstrap utility correctly maps absolute host paths to container workspace paths.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify the devcontainer-bootstrap logic for detecting a valid image-baked SDK (imageSdkReady).
2. Verify the extraction of libsapjco3.so from a JCo fragment jar (stageJcoNativeFromFragment).
3. Verify that the bootstrap utility correctly maps absolute host paths to container workspace paths.
Low confidence findings
The Dockerfile build depends on tools.hana.ondemand.com. This external dependency makes the image build non-deterministic if the site is unavailable or changes its structure. Consider if these artifacts can be mirrored or cached.
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR makes the devcontainer (and release tooling) more “repo-root aware” and Codespaces-friendly by baking the SAP ADT SDK (Eclipse p2 plugins + Linux JCo native) into the devcontainer image under /opt/openadt, and updating bootstrap/runtime path discovery and release scripts accordingly.
Changes:
Bake SAP ADT SDK artifacts into the devcontainer image at /opt/openadt and prefer those paths during devcontainer bootstrap.
Update runtime path detection (including staged devcontainer p2 roots) and Windows runtime preparation path handling.
Align release/version tooling to use the reactor root pom.xml as the version source.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
File
Description
tools/release-version/src/main.ts
Switch version read/write to reactor root pom.xml (openadt-parent).
tools/package-release/src/main.ts
Read version from reactor root pom.xml; simplify jar path construction.
tools/devcontainer-bootstrap/src/main.ts
Add image-SDK detection, optional p2 provisioning, and write runtime.openadt.toml including adt_plugins_dir.
packaging/windows/prepare-openadt-runtime.ps1
Build CLI module from reactor root; fix paths for jar discovery/copy.
packaging/windows/launcher/OpenAdtLauncher.cs
Use an explicit System32\\WindowsPowerShell\\v1.0\\powershell.exe path for launcher execution.
package.json
Add @abapify/p2-cli dev dependency for p2 download in devcontainer workflows.
docs/usage.md
Document new devcontainer flow (image SDK vs workspace provisioning).
apps/openadt-cli/pom.xml
Bump maven-dependency-plugin version for CLI build.
The new P2-only bootstrap flow still hard-requires local archives.
With --provision-p2-sdk --skip-if-missing, this code only skips when the directory is absent. If sourceRoot exists but does not contain the JCo/Crypto/SAPCAR archives, it still throws here, which breaks the documented "re-download p2 into the workspace" workflow.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/devcontainer-bootstrap/src/main.ts` around lines 801 - 846, The code
currently requires matching archives even when the directory exists; change the
flow so missing archive files do not error out when the user passed
skipIfMissing (and is provisioning P2): call resolveInputFile for
jcoArchive/cryptoArchive/sapcar inside a try/catch and if resolveInputFile
throws a "not found" error, and args.skipIfMissing (and args.provisionP2Sdk if
applicable) is true, call finalizeDevcontainer(root, containerWorkspace, args)
and return instead of re-throwing; otherwise re-throw the error. This touches
resolveInputFile, jcoArchive/cryptoArchive/sapcar, effectiveSourceRoot,
args.skipIfMissing, args.provisionP2Sdk and finalizeDevcontainer.
🤖 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 @.devcontainer/devcontainer.json:
- Line 12: The LD_LIBRARY_PATH assignment in .devcontainer/devcontainer.json
currently replaces any existing value; update the remoteEnv entry for
"LD_LIBRARY_PATH" to preserve the inherited container image paths by prepending
the existing value (use the variable substitution token
${containerEnv:LD_LIBRARY_PATH}) and then append the explicit paths shown;
target the "LD_LIBRARY_PATH" key in remoteEnv so native libraries from the base
image remain on the search path.
In @.devcontainer/Dockerfile:
- Around line 68-72: The FRAG selection is architecture-agnostic and may pick
the wrong com.sap.conn.jco.linux.*.jar; update the logic that sets FRAG to pick
the fragment matching the container architecture (use uname -m or Docker build
ARG to derive an arch token and map it to the p2 filename suffix) so the ls
pattern filters by that token (instead of matching all linux fragments), then
continue extracting SO_PATH and writing libsapjco3.so into
${OPENADT_SDK_ROOT}/dist/jco as before (references: FRAG, SO_PATH,
${OPENADT_SDK_ROOT}/dist/p2/plugins, libsapjco3.so).
In @.github/workflows/release.yml:
- Line 61: The release workflow currently runs git add pom.xml packaging/ which
misses module POMs; update the git add invocation in
.github/workflows/release.yml (the step that creates the bump commit) to stage
all rewritten module POMs as well, e.g. include a recursive glob for module POMs
such as replacing the command with one that adds pom.xml, packaging/ and all
module POMs (e.g. add '**/pom.xml' or use git ls-files to collect and git add
all changed pom.xml files) so apps/openadt-*/pom.xml files are included in the
bump commit.
In
`@apps/openadt-bootstrap/src/main/java/org/openadt/bootstrap/SetupRuntimePreparer.java`:
- Around line 132-137: The code in SetupRuntimePreparer that calls
builder.environment().put("PATH", ...) replaces the entire inherited PATH, which
prevents mvnw.cmd (invoked by prepare-openadt-runtime.ps1) from finding
java.exe; change this to prepend the Windows PowerShell/System32 entries to the
existing PATH instead of overwriting it: read the current PATH from
builder.environment().get("PATH") (or System.getenv("PATH") if null), construct
a new value that adds systemRoot + "\\System32\\WindowsPowerShell\\v1.0;" +
systemRoot + "\\System32;" + systemRoot + ";" + existingPath, and put that back
into builder.environment().put("PATH", newPath) so mvnw.cmd/java.exe resolution
(or JAVA_HOME) is preserved.
In `@tools/devcontainer-bootstrap/src/main.ts`:
- Around line 475-478: The imageSdkReady() function currently only checks ADT
bundles and libsapjco3.so, so update it to also verify a resolvable JCo jar is
present before treating the SDK as ready: locate and confirm the presence of a
JCo JAR matching com.sap.conn.jco_*.jar (either in the ADT pluginsDir or the JCo
distribution path used by resolveDevcontainerRuntime/ imageSdkJcoDir()) or reuse
resolveDevcontainerRuntime()'s resolution logic to ensure the JCo jar path is
valid; return false if no resolvable JCo jar is found. Reference:
imageSdkReady() and resolveDevcontainerRuntime().
---
Outside diff comments:
In `@tools/devcontainer-bootstrap/src/main.ts`:
- Around line 801-846: The code currently requires matching archives even when
the directory exists; change the flow so missing archive files do not error out
when the user passed skipIfMissing (and is provisioning P2): call
resolveInputFile for jcoArchive/cryptoArchive/sapcar inside a try/catch and if
resolveInputFile throws a "not found" error, and args.skipIfMissing (and
args.provisionP2Sdk if applicable) is true, call finalizeDevcontainer(root,
containerWorkspace, args) and return instead of re-throwing; otherwise re-throw
the error. This touches resolveInputFile, jcoArchive/cryptoArchive/sapcar,
effectiveSourceRoot, args.skipIfMissing, args.provisionP2Sdk and
finalizeDevcontainer.
🪄 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: 45d9092e-67e8-4a9f-b1a2-c49ed03a3cb1
📥 Commits
Reviewing files that changed from the base of the PR and between 04415aa and adcad99.
- Fix nx format:check failure in devcontainer-bootstrap/main.ts
- Add deterministic sort() before JCo fragment jar selection to prevent nondeterministic picks when version regex doesn't match
- Add try/catch around OPENADT_SDK_ROOT Path.of() to fall back to /opt/openadt on InvalidPathException
- imageSdkReady(): also require resolvable JCo jar before treating SDK as ready
- devcontainer.json: preserve inherited LD_LIBRARY_PATH with ${containerEnv:LD_LIBRARY_PATH}
- SetupRuntimePreparer.applyTrustedPath(): prepend trusted dirs to existing PATH instead of replacing it entirely, so mvnw can find java.exe on Windows
Fixed — now prepends trusted Windows/System32 paths to the existing inherited PATH instead of replacing it. This ensures mvnw.cmd can still find java.exe.
Dockerfile:67 — Download without integrity check
The p2 CLI downloads from tools.hana.ondemand.com (official SAP-hosted Eclipse update site). For a devcontainer image build this is acceptable risk; the URL is pinned and the site is SAP-operated. Adding checksum verification would require maintaining a separate checksum list for every p2 artifact.
main.ts:513 — JCo underscore jar naming (Qodo)
The underscore naming (com.sap.conn.jco_<ver>.jar) is the standard Eclipse p2 bundle convention, not a compliance violation. The runtime references the jar by its actual filesystem path in runtime.openadt.toml; JCo itself does not validate the jar filename — only the native library and class contents matter.
Dockerfile:73 — WORKDIR points to deleted dir
Already fixed — the Dockerfile has cd / before rm -rf (line 74) and WORKDIR / after cleanup (line 77).
findLatestJcoPluginJar directory check
Already has existsSync(pluginsDir) guard at line 501-503.
pathForChildProcess empty PATH segments
Already filters empty segments via .filter((entry) => entry.trim().length > 0) at line 577.
writeDevcontainerRuntime empty adt_plugins_dir
Already conditional — only emits adt_plugins_dir when runtime.adtPluginsDir is truthy (lines 746-748).
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.devcontainer/Dockerfile around lines 68 - 70, The current JARS/FRAG
selection is architecture-agnostic and may pick the wrong Linux JCo fragment;
fix by selecting the fragment that matches the build image architecture.
Introduce an ARCH variable (e.g., from `uname -m` or `dpkg
--print-architecture`), map it to the p2 artifact name (e.g., x86_64 ->
linux.x86_64, aarch64 -> linux.aarch64), then change the JARS glob in the
Dockerfile to include that architecture segment (replace
com.sap.conn.jco.linux.*.jar with com.sap.conn.jco.linux.${ARCH_PATTERN}.*.jar)
so FRAG picks the correct file and the existing `test -f "${FRAG}"` remains
valid.
🤖 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.
Duplicate comments:
In @.devcontainer/Dockerfile:
- Around line 68-70: The current JARS/FRAG selection is architecture-agnostic
and may pick the wrong Linux JCo fragment; fix by selecting the fragment that
matches the build image architecture. Introduce an ARCH variable (e.g., from
`uname -m` or `dpkg --print-architecture`), map it to the p2 artifact name
(e.g., x86_64 -> linux.x86_64, aarch64 -> linux.aarch64), then change the JARS
glob in the Dockerfile to include that architecture segment (replace
com.sap.conn.jco.linux.*.jar with com.sap.conn.jco.linux.${ARCH_PATTERN}.*.jar)
so FRAG picks the correct file and the existing `test -f "${FRAG}"` remains
valid.
ℹ️ Review info⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed256357-9621-4d88-83db-98709b000fff
📥 Commits
Reviewing files that changed from the base of the PR and between adcad99 and 45274cc.
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=".devcontainer/devcontainer.json">
<violation number="1" location=".devcontainer/devcontainer.json:12">
P2: Appending `${containerEnv:LD_LIBRARY_PATH}` without a default can yield a trailing `:` when unset, which adds the current working directory to `LD_LIBRARY_PATH` lookup. Provide a non-empty default to avoid zero-length path entries.</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: Appending ${containerEnv:LD_LIBRARY_PATH} without a default can yield a trailing : when unset, which adds the current working directory to LD_LIBRARY_PATH lookup. Provide a non-empty default to avoid zero-length path entries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .devcontainer/devcontainer.json, line 12:
<comment>Appending `${containerEnv:LD_LIBRARY_PATH}` without a default can yield a trailing `:` when unset, which adds the current working directory to `LD_LIBRARY_PATH` lookup. Provide a non-empty default to avoid zero-length path entries.</comment>
<file context>
@@ -9,7 +9,7 @@
"OPENADT_SDK_ROOT": "/opt/openadt",
"SECUDIR": "/workspaces/openadt/.devcontainer/sec",
- "LD_LIBRARY_PATH": "/opt/openadt/dist/jco:/workspaces/openadt/.devcontainer/dist/jco:/workspaces/openadt/.devcontainer/dist/snc"
+ "LD_LIBRARY_PATH": "/opt/openadt/dist/jco:/workspaces/openadt/.devcontainer/dist/jco:/workspaces/openadt/.devcontainer/dist/snc:${containerEnv:LD_LIBRARY_PATH}"
},
"postCreateCommand": "bun install && bun run bootstrap:devcontainer -- --non-interactive --skip-if-missing --container-workspace /workspaces/openadt",
</file context>
- imageSdkReady(): also require resolvable JCo jar before treating SDK as ready
- devcontainer.json: preserve inherited LD_LIBRARY_PATH with ${containerEnv:LD_LIBRARY_PATH}
- SetupRuntimePreparer.applyTrustedPath(): prepend trusted dirs to existing PATH instead of replacing it entirely, so mvnw can find java.exe on Windows
feat(devcontainer): bake SAP ADT SDK into image at /opt/openadt
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.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?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}
Summary
Test plan
untime.openadt.toml\ points at image paths.
Made with Cursor
Summary by cubic
Bake the SAP ADT SDK into the devcontainer image at /opt/openadt for zero host setup. Bootstrap prefers the image SDK, can optionally provision p2 for custom images, and improves PATH/LD_LIBRARY_PATH handling.
New Features
libsapjco3.sowith@abapify/p2-cliinto/opt/openadt/dist/p2and/opt/openadt/dist/jco; exportsOPENADT_SDK_ROOTand preserves containerLD_LIBRARY_PATHwhile prepending image JCo.adt_plugins_dir, supports--provision-p2-sdk/--skip-p2-sdk, and extracts Linux JCo native from the fragment; image SDK is “ready” only when required bundles and a resolvable JCo jar exist./opt/openadtand staged.devcontainer/dist/p2/pluginsandjco;devcontainer.jsonpoints envs at the image SDK; docs updated for the image-first flow.Bug Fixes
System32\WindowsPowerShell\v1.0\powershell.exe; prepend trusted dirs toPATH(don’t replace) somvnwfinds Java; robustSystemRootresolution.LD_LIBRARY_PATHnow preserves${containerEnv:LD_LIBRARY_PATH}.OPENADT_SDK_ROOTparsing with fallback to/opt/openadt; minor CI/format fixes and Dockerfile addswget.Written for commit 45274cc. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Chores
Documentation