-
Notifications
You must be signed in to change notification settings - Fork 2
ci: push pr image #175
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
base: main
Are you sure you want to change the base?
ci: push pr image #175
Conversation
📝 WalkthroughWalkthroughThis PR updates Docker build configurations and Marimo version constants. The CD workflow now unconditionally pushes Docker images, the Kubimo Marimo base image tag is updated to a pre-release version, and the default Marimo version constant is bumped. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/cd.yaml:
- Around line 163-165: Temporary unconditional push is enabled (push: true) in
the CD workflow and must be reverted: replace the blanket push with a targeted
condition (restore the previous condition that prevented pushes on pull_request
events or use a conditional that only allows pushes for the same-repo PRs or
specific branches like main/release), update the commented hint (push: ${{
github.event_name != 'pull_request' }}) into an actual safe condition, and
create a follow-up issue to track reverting any temporary overrides so images
are not pushed from untrusted forks or non-protected branches.
In `@docker/Dockerfile.kubimo`:
- Line 1: The Dockerfile currently pins the build base to an ephemeral PR image
via ARG KUBIMO_MARIMO_IMAGE=ghcr.io/aqora-io/kubimo-marimo:pr-6; change this to
the stable/release image tag before merging (replace :pr-6 with the appropriate
release tag or a stable channel) and add a TODO comment above the ARG noting
that the PR tag was used temporarily and must be reverted to a release tag prior
to release (referencing the CD revert practice). Ensure you only modify the ARG
KUBIMO_MARIMO_IMAGE declaration and add the one-line TODO comment nearby so the
temporary PR tag won’t be shipped.
| # TODO: revert back | ||
| # push: ${{ github.event_name != 'pull_request' }} | ||
| push: true |
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.
Temporary unconditional push — ensure this gets reverted.
The TODO acknowledges this, but pushing Docker images from every PR (including from forks) is a risk if left in place: it pollutes the registry and could allow untrusted code to be published as an image. Consider adding a more targeted condition (e.g., only for PRs from the same repo, or specific branches) rather than a blanket push: true, or at minimum track the revert with a follow-up issue.
🤖 Prompt for AI Agents
In @.github/workflows/cd.yaml around lines 163 - 165, Temporary unconditional
push is enabled (push: true) in the CD workflow and must be reverted: replace
the blanket push with a targeted condition (restore the previous condition that
prevented pushes on pull_request events or use a conditional that only allows
pushes for the same-repo PRs or specific branches like main/release), update the
commented hint (push: ${{ github.event_name != 'pull_request' }}) into an actual
safe condition, and create a follow-up issue to track reverting any temporary
overrides so images are not pushed from untrusted forks or non-protected
branches.
| @@ -1,4 +1,4 @@ | |||
| ARG KUBIMO_MARIMO_IMAGE=ghcr.io/aqora-io/kubimo-marimo:0.1.9 | |||
| ARG KUBIMO_MARIMO_IMAGE=ghcr.io/aqora-io/kubimo-marimo:pr-6 | |||
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.
Base image pinned to a PR tag — must be reverted before release.
pr-6 is an ephemeral, non-release image tag. If this lands on main and the upstream PR image is later deleted or overwritten, builds will break. Like the cd.yaml TODO, this should be tracked for revert. Consider adding a TODO comment here as well so it doesn't get missed.
🤖 Prompt for AI Agents
In `@docker/Dockerfile.kubimo` at line 1, The Dockerfile currently pins the build
base to an ephemeral PR image via ARG
KUBIMO_MARIMO_IMAGE=ghcr.io/aqora-io/kubimo-marimo:pr-6; change this to the
stable/release image tag before merging (replace :pr-6 with the appropriate
release tag or a stable channel) and add a TODO comment above the ARG noting
that the PR tag was used temporarily and must be reverted to a release tag prior
to release (referencing the CD revert practice). Ensure you only modify the ARG
KUBIMO_MARIMO_IMAGE declaration and add the one-line TODO comment nearby so the
temporary PR tag won’t be shipped.
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 065e6fd5e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| push: ${{ github.event_name != 'pull_request' }} | ||
| # TODO: revert back | ||
| # push: ${{ github.event_name != 'pull_request' }} | ||
| push: true |
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.
Gate Docker image pushes for pull_request events
Setting push: true in .github/workflows/cd.yaml removes the prior PR guard and forces docker/bake-action to publish on every pull_request run; for PRs from forks, GITHUB_TOKEN cannot write packages, so this step fails with registry permission errors and the docker job goes red even when the build itself is fine. Restoring an event/fork check avoids breaking CI for external contributors.
Useful? React with 👍 / 👎.
Summary by CodeRabbit
Updates
Chores