-
Notifications
You must be signed in to change notification settings - Fork 12
Fix cli build issues #312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix cli build issues #312
Conversation
🦋 Changeset detectedLatest commit: b56a72f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Coverage Report for ./apps/cli
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
bf1617c to
04176a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the image handling logic in the build function to only pull images when they don't exist and enhances the entrypoint determination and error messaging in the machine bootstrap.
- In
builder/docker.ts, the code now inspects an existing image first, pulls it only on inspection failure, and retrieves image information immediately after. - In
machine.ts, entrypoint assignment is refactored into explicit if/else blocks with a more descriptive error when no entrypoint is found.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/cli/src/builder/docker.ts | Wrapped getImageInfo in try/catch to pull missing images, moved subsequent getImageInfo calls into both branches. |
| apps/cli/src/machine.ts | Expanded entrypoint logic from a ?? expression to explicit if/else, and updated the error message for clarity. |
Comments suppressed due to low confidence (1)
apps/cli/src/builder/docker.ts:102
- Add tests to cover the flow where getImageInfo initially fails and triggers a pull followed by a successful inspect, to ensure this retry logic behaves correctly.
imageInfo = await getImageInfo(image);
| await execa("docker", ["image", "pull", image]); | ||
| imageInfo = await getImageInfo(image); |
Copilot
AI
Jul 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catching all errors from getImageInfo may hide failures other than a missing image (for example, an architecture mismatch). Consider narrowing the catch block to only handle "image not found" errors before attempting to pull the image.
| await execa("docker", ["image", "pull", image]); | |
| imageInfo = await getImageInfo(image); | |
| if (error instanceof Error && error.message.includes("No such image")) { | |
| await execa("docker", ["image", "pull", image]); | |
| imageInfo = await getImageInfo(image); | |
| } else { | |
| throw error; | |
| } |
tuler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include a changeset
|
@tuler I'll consider you changeset enough for the entrypoint issue and addes a fixup with a changest for the pull image issue |
c778723 to
e8380d0
Compare
e8380d0 to
b56a72f
Compare
This pull request refines the functionality of Docker image handling and machine booting processes in the CLI application. The changes improve error handling, provide more detailed error messages, and ensure better handling of Docker image information and entrypoints.
Docker Image Handling Improvements:
apps/cli/src/builder/docker.ts: Enhanced the logic for obtaining Docker image information by introducing atry-catchblock to handle errors when fetching image info after pulling or building an image. This ensures robust handling of scenarios where the image info retrieval might fail initially.Machine Booting Enhancements:
apps/cli/src/machine.ts: Refactored the logic for determining the machine's entrypoint, adding checks for cases whereentrypointorcmdare empty arrays. Improved the error message to guide users on how to resolve undefined entrypoints by updating their configuration or Dockerfile.