Upgrade codeserver to v4.112.0 on rhoai-3.4 (full hermetic)#2545
Conversation
Keep the existing hermetic Konflux model and 3.4 base/index while bumping code-server, patches, extensions, and RPM locks so multi-arch builds stay offline and aligned with main. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@ysok — This PR is from a fork. Recommended: Push your branch to the main repo for full CI: Then open a new PR from that branch. No push access? A maintainer will cherry-pick and test your changes. See CONTRIBUTING.md for details. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| "node_modules/shell-quote": { | ||
| "version": "1.8.3", | ||
| "resolved": "https://registry.npmjs.org/shell-quote/-/shell-quote-1.8.3.tgz", | ||
| "integrity": "sha512-ObmnIF4hXNg1BqhnHmgbDETF8dLPCggZWBjkQfhZpbszZnYur5DUljTcCHii5LC3J5E0yeO/1LIMyH+UvHQgyw==", | ||
| "license": "MIT", | ||
| "engines": { | ||
| "node": ">= 0.4" | ||
| }, | ||
| "funding": { | ||
| "url": "https://github.com/sponsors/ljharb" | ||
| } | ||
| }, |
| "node_modules/lodash-es": { | ||
| "version": "4.17.23", | ||
| "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.23.tgz", | ||
| "integrity": "sha512-kVI48u3PZr38HdYz98UmfPnXl2DXrpdctLrFLCd3kOx1xUkOmpFPx7gCWWM5MPkL/fD8zb+Ph0QzjGFs4+hHWg==", | ||
| "license": "MIT" | ||
| }, |
| "node_modules/lodash-es": { | ||
| "version": "4.17.23", | ||
| "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.23.tgz", | ||
| "integrity": "sha512-kVI48u3PZr38HdYz98UmfPnXl2DXrpdctLrFLCd3kOx1xUkOmpFPx7gCWWM5MPkL/fD8zb+Ph0QzjGFs4+hHWg==", | ||
| "license": "MIT" | ||
| }, |
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR upgrades the UBI9 Python 3.12 code-server image to v4.112.0, updates hermetic npm, pip, and RPM inputs, adds offline build patches and VSIX validation, revises NGINX and CI container handling, and changes the Playwright test image workflow. ChangesCodeserver v4.112 build and hermetic inputs
Estimated code review effort: 5 (Critical) | ~100 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/build-codeserver |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
codeserver/ubi9-python-3.12/Dockerfile.cpu (1)
160-169: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRestore
uvbefore thecodeserverinstall step
cpu-baseno longer installsuv, butcodeserverstill runsuv pip install --no-index ...later in this Dockerfile. Since the base image doesn’t provideuv, the build will fail withuv: command not foundunless it’s installed earlier.🤖 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 `@codeserver/ubi9-python-3.12/Dockerfile.cpu` around lines 160 - 169, Install or restore uv before the codeserver installation step that invokes uv pip install --no-index. Update the Dockerfile’s setup flow near the visible architecture-specific dependency block, ensuring uv is available in the image before the codeserver install command runs.codeserver/ubi9-python-3.12/README.md (1)
171-179: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd missing build arguments and clarify architecture paths.
The manual
podman buildcommand is missing the newly introducedLABEL_REGISTRY_PREFIX,LABEL_COMPONENT, andRELEASEbuild arguments. Without these, the resulting image will have broken or empty labels (e.g.,name="/").Additionally, clarify that the
x86_64path andlinux/amd64platform must match the architecture used during theprefetch-all.shstep, as the preceding prefetch example usesarm64.📝 Proposed fix to update the build example
podman build \ -f codeserver/ubi9-python-3.12/Dockerfile.konflux.cpu \ --platform linux/amd64 \ -t codeserver-test \ --build-arg BASE_IMAGE=quay.io/opendatahub/odh-base-image-cpu-py312-c9s:latest \ --build-arg PYLOCK_FLAVOR=cpu \ + --build-arg LABEL_REGISTRY_PREFIX=opendatahub \ + --build-arg LABEL_COMPONENT=odh-workbench-codeserver-datascience-cpu-py312-ubi9 \ + --build-arg RELEASE=3.4 \ -v "$(realpath ./cachi2/output)":/cachi2/output:z \ -v "$(realpath ./cachi2/output/deps/rpm/x86_64/repos.d)":/etc/yum.repos.d/:z \ . +# Note: If you prefetched for arm64, change x86_64 to aarch64 and linux/amd64 to linux/arm64🤖 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 `@codeserver/ubi9-python-3.12/README.md` around lines 171 - 179, Update the manual podman build example in the README to include the LABEL_REGISTRY_PREFIX, LABEL_COMPONENT, and RELEASE build arguments with appropriate example values. Clarify that the linux/amd64 platform and x86_64 repository path must match the architecture selected during the prefetch-all.sh step, and reconcile the example with the preceding arm64 prefetch example.
🧹 Nitpick comments (2)
codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/ripgrep/postinstall.js (1)
124-131: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
getTarget()call;targetlocal is unused.Line 124 computes
targetbut it's never read —opts.targetre-invokesgetTarget()on line 128 instead of reusing the local variable.♻️ Proposed fix
const target = await getTarget(); const opts = { version: VERSION, token: process.env['GITHUB_TOKEN'], - target: await getTarget(), + target: target, destDir: BIN_PATH, force: forceInstall };🤖 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 `@codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/ripgrep/postinstall.js` around lines 124 - 131, Remove the unused local target declaration and reuse a single getTarget() result in the opts object by assigning target from that local variable. Update the target field in opts while leaving the other options unchanged.codeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.sh (1)
113-115: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valuePredictable
/tmppath for the jq-rewritten lockfile.
/tmp/lock-agent-browser.jsonis a fixed, predictable path; since this script runs as root (per header comment), a local process could pre-create/symlink it. This mirrors the same pattern already used elsewhere in the file (ripgrep, vsce-sign blocks), so it's not a new risk class, but worth hardening withmktempwhile touching this code.🔒 Proposed fix
+ lock_tmp="$(mktemp)" jq 'del(.packages["node_modules/agent-browser"].integrity)' \ - lib/vscode/package-lock.json > /tmp/lock-agent-browser.json \ - && mv /tmp/lock-agent-browser.json lib/vscode/package-lock.json + lib/vscode/package-lock.json > "$lock_tmp" \ + && mv "$lock_tmp" lib/vscode/package-lock.json🤖 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 `@codeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.sh` around lines 113 - 115, Replace the fixed /tmp/lock-agent-browser.json path in the jq lockfile rewrite with a securely created temporary file via mktemp, then move the generated file into lib/vscode/package-lock.json while preserving the existing jq transformation and failure handling.Source: Linters/SAST tools
🤖 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 @.tekton/odh-workbench-codeserver-datascience-cpu-py312-v3-4-push.yaml:
- Around line 78-79: Revert the change to the `.tekton/` PipelineRun
configuration, including the removed
`codeserver/ubi9-python-3.12/prefetch-input/code-server/lib/vscode/build/vite`
entry. Do not make direct edits in `.tekton/`; apply the corresponding change in
the upstream `konflux-central` source instead.
---
Outside diff comments:
In `@codeserver/ubi9-python-3.12/Dockerfile.cpu`:
- Around line 160-169: Install or restore uv before the codeserver installation
step that invokes uv pip install --no-index. Update the Dockerfile’s setup flow
near the visible architecture-specific dependency block, ensuring uv is
available in the image before the codeserver install command runs.
In `@codeserver/ubi9-python-3.12/README.md`:
- Around line 171-179: Update the manual podman build example in the README to
include the LABEL_REGISTRY_PREFIX, LABEL_COMPONENT, and RELEASE build arguments
with appropriate example values. Clarify that the linux/amd64 platform and
x86_64 repository path must match the architecture selected during the
prefetch-all.sh step, and reconcile the example with the preceding arm64
prefetch example.
---
Nitpick comments:
In `@codeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.sh`:
- Around line 113-115: Replace the fixed /tmp/lock-agent-browser.json path in
the jq lockfile rewrite with a securely created temporary file via mktemp, then
move the generated file into lib/vscode/package-lock.json while preserving the
existing jq transformation and failure handling.
In
`@codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/ripgrep/postinstall.js`:
- Around line 124-131: Remove the unused local target declaration and reuse a
single getTarget() result in the opts object by assigning target from that local
variable. Update the target field in opts while leaving the other options
unchanged.
🪄 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: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 02bfda34-4045-42f0-815d-0ec8d537a3d1
⛔ Files ignored due to path filters (9)
codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.106.3/lib/vscode/extensions/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/custom-packages/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/vite/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/emmet/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/microsoft-authentication/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/remote/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/test/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (58)
.tekton/odh-workbench-codeserver-datascience-cpu-py312-pull-request.yaml.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-odh-main-pull-request.yaml.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-odh-main-push.yaml.tekton/odh-workbench-codeserver-datascience-cpu-py312-v3-4-push.yamlcodeserver/Extensions.mdcodeserver/ubi9-python-3.12/Dockerfile.cpucodeserver/ubi9-python-3.12/Dockerfile.konflux.cpucodeserver/ubi9-python-3.12/README.mdcodeserver/ubi9-python-3.12/build-args/cpu.confcodeserver/ubi9-python-3.12/build-args/konflux.cpu.confcodeserver/ubi9-python-3.12/devel_env_setup.shcodeserver/ubi9-python-3.12/nginx/httpconf/http.confcodeserver/ubi9-python-3.12/nginx/root/opt/app-root/nginxconf.sedcodeserver/ubi9-python-3.12/nginx/serverconf/proxy.conf.templatecodeserver/ubi9-python-3.12/nginx/serverconf/proxy.conf.template_nbprefixcodeserver/ubi9-python-3.12/prefetch-input/code-servercodeserver/ubi9-python-3.12/prefetch-input/odh/artifacts.in.yamlcodeserver/ubi9-python-3.12/prefetch-input/odh/rpms.in.yamlcodeserver/ubi9-python-3.12/prefetch-input/odh/rpms.lock.yamlcodeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.shcodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.106.3/README.mdcodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.106.3/lib/vscode/remote/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/agent-browser/postinstall.jscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/ci/build/build-vscode.shcodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/ci/dev/postinstall.shcodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/custom-packages/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/gulpfile.reh.tscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/lib/extensions.tscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/lib/tsgo.tscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/npm/preinstall.tscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/vite/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/emmet/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/microsoft-authentication/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/remote/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/ripgrep/postinstall.jscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/test/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/codeserver-offline-env.shcodeserver/ubi9-python-3.12/prefetch-input/patches/setup-offline-binaries.shcodeserver/ubi9-python-3.12/prefetch-input/patches/tweak-gha.shcodeserver/ubi9-python-3.12/prefetch-input/repos/centos.repocodeserver/ubi9-python-3.12/prefetch-input/repos/epel.repocodeserver/ubi9-python-3.12/prefetch-input/repos/openshift-clients.repocodeserver/ubi9-python-3.12/prefetch-input/repos/rhsm-pulp.repocodeserver/ubi9-python-3.12/prefetch-input/repos/ubi.repocodeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.lock.yamlcodeserver/ubi9-python-3.12/run-code-server.shcodeserver/ubi9-python-3.12/run-nginx.shcodeserver/ubi9-python-3.12/s390x.patchcodeserver/ubi9-python-3.12/utils/ms-python.python-2026.0.0.vsixcodeserver/ubi9-python-3.12/utils/ms-python.python-2026.4.0.vsixcodeserver/ubi9-python-3.12/utils/ms-vscode.js-debug.1.105.0.vsixcodeserver/ubi9-python-3.12/utils/ms-vscode.js-debug.1.112.0.vsixcodeserver/ubi9-python-3.12/utils/process.shmanifests/odh/base/code-server-notebook-imagestream.yamlscripts/buildinputs/buildinputs_test.goscripts/lockfile-generators/prefetch-all.sh
💤 Files with no reviewable changes (5)
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.106.3/lib/vscode/remote/package.json
- codeserver/ubi9-python-3.12/utils/ms-vscode.js-debug.1.105.0.vsix
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.106.3/README.md
- codeserver/ubi9-python-3.12/utils/ms-python.python-2026.0.0.vsix
- codeserver/ubi9-python-3.12/devel_env_setup.sh
| - path: codeserver/ubi9-python-3.12/prefetch-input/code-server/lib/vscode/build/vite | ||
| type: npm |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win
Do not modify .tekton/ files directly in this repository.
As per path instructions, do not modify .tekton/ in red-hat-data-services/notebooks directly; PipelineRuns are synced from konflux-central. Please revert the changes in this directory and make them in konflux-central instead to prevent them from being overwritten or causing sync failures.
🤖 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 @.tekton/odh-workbench-codeserver-datascience-cpu-py312-v3-4-push.yaml around
lines 78 - 79, Revert the change to the `.tekton/` PipelineRun configuration,
including the removed
`codeserver/ubi9-python-3.12/prefetch-input/code-server/lib/vscode/build/vite`
entry. Do not make direct edits in `.tekton/`; apply the corresponding change in
the upstream `konflux-central` source instead.
Source: Path instructions
pull_request_target builds resolve the AIPCC template from main, so the PR checkout needs the native Podman configure action, capture-kernel-logs, and matching containers/podman configs instead of the Homebrew TMPDIR path. Co-authored-by: Cursor <cursoragent@cursor.com>
Lock and install glibc (+ langpacks) with redhat-rpm-config so dnf can upgrade the base .24 stack to .25 when nodejs-packaging pulls rpm-config. Co-authored-by: Cursor <cursoragent@cursor.com>
The native Podman configure action expects 26.04; on 24.04 the AppArmor pasta reload fails because usr.bin.pasta is missing. Also skip the reload when that profile file is absent. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/build-codeserver |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/actions/install-podman-action/action.yml (1)
4-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider removing the unused
platforminput.The
platforminput is declared and required but no longer used in the scripts of this action. If this action is only used internally, consider removing this input here and from all callers to keep the configuration clean.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/actions/install-podman-action/action.yml around lines 4 - 7, Remove the unused required platform input from the action definition and update all callers of the install-podman action to stop passing platform. Preserve the action’s remaining inputs and behavior.
🤖 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.
Nitpick comments:
In @.github/actions/install-podman-action/action.yml:
- Around line 4-7: Remove the unused required platform input from the action
definition and update all callers of the install-podman action to stop passing
platform. Preserve the action’s remaining inputs and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8c8fe350-764f-4461-be5c-8f241af72aa9
📒 Files selected for processing (10)
.github/actions/capture-kernel-logs/action.yml.github/actions/install-podman-action/action.ymlci/cached-builds/containers.confci/cached-builds/podman.serviceci/cached-builds/podman.socketci/cached-builds/storage.confcodeserver/ubi9-python-3.12/Dockerfile.cpucodeserver/ubi9-python-3.12/Dockerfile.konflux.cpucodeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.in.yamlcodeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.lock.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- codeserver/ubi9-python-3.12/Dockerfile.konflux.cpu
The native Podman action and /mnt storage.conf require Podman 5.x; ubuntu-24.04 (4.9.3) fails system reset with "runroot must be set". Co-authored-by: Cursor <cursoragent@cursor.com>
Refresh CentOS Stream pins (fixes stale kernel-headers 404) and align codeserver ODH input with the glibc stack used by the Dockerfiles. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/build-codeserver |
GHA runners expose an ~8GiB tmpfs at /tmp; codeserver multi-arch RHDS prefetch overflowed it (ENOSPC) while / still had free space. Point TMPDIR and hermeto staging at /mnt/tmp on the root disk. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/build-codeserver |
Point cri-o at /mnt/containers/storage so OpenShift tests see locally built images, align Playwright with main's browser-image flow, waive code-server 4.112 seccomp static binaries, and pin mysql-connector to the 9.7.0 wheel available on the RHAI index. Co-authored-by: Cursor <cursoragent@cursor.com>
| "node_modules/shell-quote": { | ||
| "version": "1.8.3", | ||
| "resolved": "https://registry.npmjs.org/shell-quote/-/shell-quote-1.8.3.tgz", | ||
| "integrity": "sha512-ObmnIF4hXNg1BqhnHmgbDETF8dLPCggZWBjkQfhZpbszZnYur5DUljTcCHii5LC3J5E0yeO/1LIMyH+UvHQgyw==", | ||
| "license": "MIT", | ||
| "engines": { | ||
| "node": ">= 0.4" | ||
| }, | ||
| "funding": { | ||
| "url": "https://github.com/sponsors/ljharb" | ||
| } | ||
| }, |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/actions/playwright-test/action.yml (1)
126-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the redundant
|| ''fallback on a required input.
test-targetis declaredrequired: true, so the fallback never triggers.♻️ Proposed fix
- TEST_TARGET: ${{ inputs.test-target || '' }} + TEST_TARGET: ${{ inputs.test-target }}Based on learnings, "For composite GitHub Actions, if an input is declared with
required: true, callers must provide it, so you should not add a redundant fallback like|| ''... The|| ...fallback pattern should be reserved for workflow-level inputs... where the input may be unset."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/actions/playwright-test/action.yml at line 126, Remove the redundant empty-string fallback from the TEST_TARGET assignment in the action’s environment configuration, referencing inputs.test-target directly. Preserve the existing required-input contract and all other environment mappings.Source: Learnings
tests/browser/Dockerfile (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBootstrap pnpm version doesn't satisfy
devEngines.packageManagerrange.
pnpm@11.1.2is installed globally, buttests/browser/package.json5'sdevEngines.packageManagerrequires>=11.5.3 <12. WithonFail: 'download', pnpm should auto-fetch a compliant version at install time, so this likely still works, but the mismatch makes the pinned bootstrap version meaningless and worth keeping in sync to avoid an extra download and confusion for maintainers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/browser/Dockerfile` at line 15, Update the pnpm version in the Dockerfile’s global bootstrap install to satisfy the packageManager range declared by tests/browser/package.json5, using a version at least 11.5.3 and below 12 so the pinned bootstrap matches the project requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/browser/package.json5`:
- Around line 39-45: Update the CI script that extracts the pnpm version for
“Lint & Typecheck” to read tests/browser/package.json5’s
devEngines.packageManager.version (or reuse scripts/get_playwright_version.py)
instead of grepping for the removed legacy packageManager field; preserve the
existing version validation and downstream setup.
---
Nitpick comments:
In @.github/actions/playwright-test/action.yml:
- Line 126: Remove the redundant empty-string fallback from the TEST_TARGET
assignment in the action’s environment configuration, referencing
inputs.test-target directly. Preserve the existing required-input contract and
all other environment mappings.
In `@tests/browser/Dockerfile`:
- Line 15: Update the pnpm version in the Dockerfile’s global bootstrap install
to satisfy the packageManager range declared by tests/browser/package.json5,
using a version at least 11.5.3 and below 12 so the pinned bootstrap matches the
project requirement.
🪄 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: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 5fd351cb-c788-4756-a9af-2a3f6e75e474
⛔ Files ignored due to path filters (1)
tests/browser/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.github/actions/playwright-test/action.ymlci/cached-builds/crio.confscripts/check-payload/config.tomlscripts/get_playwright_version.pytests/browser/Dockerfiletests/browser/README.mdtests/browser/package.json5tests/browser/pnpm-workspace.yamltests/browser/scripts/README.mdtests/browser/tests/openshift_console.spec.tstests/browser/tests/testcontainers.tstests/containers/workbenches/jupyterlab/jupyterlab_datascience_test.py
| devEngines: { | ||
| packageManager: { | ||
| name: 'pnpm', | ||
| version: '>=11.5.3 <12', | ||
| onFail: 'download', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
CI is currently failing because of this field removal.
Removing the legacy "packageManager": "pnpm@10.33.0" field breaks a script elsewhere in the pipeline that still greps for it. Per the pipeline failure logs, "Test Playwright Action / Lint & Typecheck" fails with Failed to extract pnpm version from package.json5 (ver is empty) from a command shaped like grep -oP '"packageManager":\s*"pnpm@\K[0-9.]+' tests/browser/package.json5. That script needs to be updated to read the version from devEngines.packageManager.version (or reuse scripts/get_playwright_version.py's approach) instead of the removed field.
#!/bin/bash
rg -n 'packageManager' --type=yaml --type=sh -g '!node_modules' .
rg -n 'pnpm@\\K' .🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/browser/package.json5` around lines 39 - 45, Update the CI script that
extracts the pnpm version for “Lint & Typecheck” to read
tests/browser/package.json5’s devEngines.packageManager.version (or reuse
scripts/get_playwright_version.py) instead of grepping for the removed legacy
packageManager field; preserve the existing version validation and downstream
setup.
Source: Pipeline failures
Main's Dockerfile requires PLAYWRIGHT_VERSION and the playwright-test action needs a prebuilt image; update build/test workflows and add get-pnpm-version.sh for the new package.json5 layout. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/build-codeserver |
1 similar comment
|
/build-codeserver |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: daniellutz The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Summary
hermetic: true, cachi2--no-indexpip/RPM/npm).4.112), and regenerate ODH + RHDSrpms.lock.yaml.Test plan
gmake test/gmake test-unitBUILD_ARCH=linux/arm64 PUSH_IMAGES=no gmake codeserver-ubi9-python-3.12test_startup.py(code-server v4.112.0)Made with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Documentation