Skip to content

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.

Sign up for GitHub

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

Closed
jpopesculian wants to merge 1 commit into from
Closed

ci: push pr image #175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/cd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ jobs:
files: |
./docker-bake.hcl
cwd://${{ steps.meta-kubimo.outputs.bake-file }}
push: ${{ github.event_name != 'pull_request' }}
# TODO: revert back
# push: ${{ github.event_name != 'pull_request' }}
push: true
Comment on lines +163 to +165
Copy link

@coderabbitai coderabbitai bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.


npm:
name: Build and publish npm package
Expand Down
2 changes: 1 addition & 1 deletion docker/Dockerfile.kubimo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Copy link

@coderabbitai coderabbitai bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.


FROM ${KUBIMO_MARIMO_IMAGE} AS build

Expand Down
2 changes: 1 addition & 1 deletion docker/kubimo/marimo/marimo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ std_stream_max_bytes = 1000000
reactive_tests = true
watcher_on_save = "lazy"
output_max_bytes = 8000000
default_auto_download = ["html"]
default_auto_download = ["html", "ipynb", "md"]
on_cell_change = "autorun"
auto_instantiate = false
default_sql_output = "auto"
Expand Down
2 changes: 1 addition & 1 deletion template/src/dataset_marimo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::registry::REGISTRY;
use crate::utils::{assert_semver, assert_slug, assert_username, OptionExt};

const DEFAULT_PYTHON_VERSION: &str = "3.10";
const DEFAULT_MARIMO_VERSION: &str = "0.19.2";
const DEFAULT_MARIMO_VERSION: &str = "0.19.9";
const DEFAULT_CLI_VERSION_STR: &str = env!("CARGO_PKG_VERSION");

#[derive(Builder, Serialize, Debug)]
Expand Down
Loading