Fix Corepack with usr-bin-first shell paths - #11
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
🧙 Sourcery has finished reviewing your pull request! Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The Dockerfile loop that snapshots original Corepack binaries assumes each command exists in
/usr/binand is executable; consider making this more defensive (e.g., skipping missing commands or falling back to/usr/local/bin) to avoid build failures on images with different layouts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Dockerfile loop that snapshots original Corepack binaries assumes each command exists in `/usr/bin` and is executable; consider making this more defensive (e.g., skipping missing commands or falling back to `/usr/local/bin`) to avoid build failures on images with different layouts.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@codex review |
|
@sourcery-ai review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR updates the Layer 3 user image Corepack wrapper so that Corepack and its package-manager shims (pnpm/yarn) continue to work when shell PATH ordering prefers /usr/bin over /usr/local/bin, while ensuring Corepack uses a per-user writable cache.
Changes:
- Preserve original Corepack/package-manager entry points and add wrapper interception for both
/usr/binand/usr/local/bin. - Update the wrapper to execute preserved originals instead of calling back into
/usr/bin/$command. - Add a regression test for
/usr/bin-first PATH behavior and bump Layer 3 version to1.2.2.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| user-layer/Dockerfile | Adds preservation of “original” commands and symlinks wrappers into both /usr/bin and /usr/local/bin; bumps layer version. |
| user-layer/corepack-user-cache | Changes wrapper exec target to preserved originals under /usr/local/libexec/workbenches-corepack-original. |
| devBenches/devcontainer.test/test.sh | Adds a regression check for pnpm resolution with /usr/bin-first PATH. |
| devBenches/devcontainer.test/README.md | Documents the new PATH-ordering compatibility for the Layer 3 Corepack cache behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| RUN set -eux && \ | ||
| chmod 0755 /usr/local/libexec/workbenches-corepack-user-cache && \ | ||
| mkdir -p /usr/local/libexec/workbenches-corepack-original && \ | ||
| for command in corepack pnpm pnpx yarn yarnpkg; do \ | ||
| original_command="$(readlink -f "/usr/bin/$command")"; \ | ||
| test -x "$original_command"; \ | ||
| ln -sfn "$original_command" "/usr/local/libexec/workbenches-corepack-original/$command"; \ | ||
| ln -sfn /usr/local/libexec/workbenches-corepack-user-cache "/usr/local/bin/$command"; \ | ||
| ln -sfn /usr/local/libexec/workbenches-corepack-user-cache "/usr/bin/$command"; \ | ||
| done |
There was a problem hiding this comment.
Addressed in bfe1372: each original entry point is now renamed in place before either PATH location is rewired. This preserves relative symlinks and regular binaries without recursion. The rebuilt Rust Layer 3 image passes both the real Corepack test and a disposable regular-file preservation test.
|
@codex review |
|
@sourcery-ai review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
user-layer/Dockerfile:54
- The “preserve original Corepack entry points” logic currently records originals as symlinks to paths under /usr/bin or /usr/local/bin, but then immediately replaces those same paths with the wrapper. If the original is a regular file (common for /usr/bin/corepack) or if later shims resolve to a command already wrapped earlier in the loop, the preserved symlink will end up pointing back to the wrapper and can recurse.
A safer approach is to snapshot the original executables before any rewiring (e.g., copy them into /usr/local/libexec/workbenches-corepack-original/) and only then replace /usr/bin/* and /usr/local/bin/* with the wrapper.
RUN set -eux && \
chmod 0755 /usr/local/libexec/workbenches-corepack-user-cache && \
mkdir -p /usr/local/libexec/workbenches-corepack-original && \
for command in corepack pnpm pnpx yarn yarnpkg; do \
if [ -e "/usr/bin/$command" ] || [ -L "/usr/bin/$command" ]; then \
original_command="/usr/bin/$command"; \
elif [ -e "/usr/local/bin/$command" ] || [ -L "/usr/local/bin/$command" ]; then \
original_command="/usr/local/bin/$command"; \
else \
echo "Required Corepack command is unavailable: $command" >&2; \
exit 1; \
fi; \
preserved_command="${original_command}.workbenches-original"; \
mv "$original_command" "$preserved_command"; \
test -x "$preserved_command"; \
ln -sfn "$preserved_command" "/usr/local/libexec/workbenches-corepack-original/$command"; \
Summary
/usr/binand/usr/local/binpackage-manager shims/usr/binfirstValidation
/usr/bin/pnpmpnpm installpasses in/workspace/projects/profile-switcherafter the fixpnpm tauri --versionreports 2.11.4COREPACK_HOMEgit diff --checkpassSummary by Sourcery
Update the user layer Corepack wrapper to handle /usr/bin-first PATH configurations and ensure package manager shims route through a writable per-user cache.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests: