Harden production promotion image copy and readiness#356
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Control Plane org normalization; verifies production env-var parity; replaces the prior copy flow with a Docker Buildx imagetools-based promotion step that computes and exports the next production image tag; and updates health/rollback checks, docs, and specs. ChangesProduction Image Copy Promotion Workflow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48f8adecde
ℹ️ 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".
| staging_commit="${STAGING_IMAGE##*_}" | ||
| if [[ "${staging_commit}" == "${STAGING_IMAGE}" || -z "${staging_commit}" ]]; then | ||
| echo "::error::Staging image '${STAGING_IMAGE}' does not include the expected '_<commit>' suffix." | ||
| exit 1 |
There was a problem hiding this comment.
When the staging workload points at a valid cpflow image that was built without --commit (for example my-app:7), this new guard aborts the entire production promotion even though cpflow still supports such tags: latest_image_next only appends _<commit> when a commit is present (lib/core/controlplane.rb:53-62), and the previous copy-image-from-upstream path passed a nil commit through to create the next plain production tag (lib/command/copy_image_from_upstream.rb:80-90). This regresses promotions for existing/manual staging images that do not have the optional commit suffix; generate the production name without a suffix in that case instead of exiting.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR replaces
Confidence Score: 4/5Safe to merge — the new copy path is well-structured with proper credential isolation, cleanup traps, and retry logic. The core logic is sound: sequential image numbering, temporary profile creation with CPLN_TOKEN_STAGING, Docker credential setup, and a trap-based cleanup all hang together correctly. The two findings are cosmetic: CPLN_PROFILE on docker manifest inspect has no effect, and the retry warning does not distinguish manifest-inspect failures from copy failures — neither affects correctness. No files require special attention beyond the style notes on the docker manifest inspect line in both workflow files. Important Files Changed
Sequence DiagramsequenceDiagram
participant WF as Workflow
participant CplnProd as cpln (production default profile)
participant CplnStage as cpln (upstream-staging profile)
participant DockerStore as Docker credential store
participant StageReg as Staging Registry
participant ProdReg as Production Registry
WF->>CplnProd: cpln image query --org PRODUCTION (get latest image number)
CplnProd-->>WF: latest_number
WF->>WF: "production_image = APP:latest_number+1_commit"
WF->>CplnStage: "CPLN_TOKEN=STAGING cpln profile create upstream-profile"
CplnStage-->>WF: profile created
WF->>CplnStage: "CPLN_PROFILE=upstream cpln image docker-login --org STAGING"
CplnStage->>DockerStore: write staging registry credentials
DockerStore-->>WF: credentials stored
loop up to copy_image_attempts
WF->>StageReg: docker manifest inspect STAGING_IMAGE
StageReg-->>WF: manifest OK
WF->>CplnStage: cpln image copy STAGING_IMAGE --profile upstream --to-profile default --to-org PRODUCTION --to-name production_image
CplnStage->>ProdReg: copy image
ProdReg-->>WF: copy complete
end
WF->>WF: output production_image
WF->>CplnProd: cpflow deploy-image -a PRODUCTION_APP
WF->>WF: trap cleanup: cpln profile delete upstream-profile
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| copy_status=1 | ||
| for attempt in $(seq 1 "${copy_image_attempts}"); do | ||
| if cpflow copy-image-from-upstream -a "${PRODUCTION_APP_NAME}" --org "${CPLN_ORG_PRODUCTION}" --image "${STAGING_IMAGE}"; then | ||
| if CPLN_PROFILE="${upstream_profile}" docker manifest inspect "${source_image_ref}" >/dev/null && |
There was a problem hiding this comment.
CPLN_PROFILE variable prefix on docker manifest inspect is a no-op — Docker ignores it entirely. Credentials for the staging registry were already written to Docker's credential store by the cpln image docker-login call above. Leaving it here implies Docker respects CPLN_PROFILE, which could mislead future maintainers into thinking removing it would break authentication.
| if CPLN_PROFILE="${upstream_profile}" docker manifest inspect "${source_image_ref}" >/dev/null && | |
| if docker manifest inspect "${source_image_ref}" >/dev/null && |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| copy_status=1 | ||
| for attempt in $(seq 1 "${copy_image_attempts}"); do | ||
| if cpflow copy-image-from-upstream -a "${PRODUCTION_APP_NAME}" --org "${CPLN_ORG_PRODUCTION}" --image "${STAGING_IMAGE}"; then | ||
| if CPLN_PROFILE="${upstream_profile}" docker manifest inspect "${source_image_ref}" >/dev/null && |
There was a problem hiding this comment.
CPLN_PROFILE prefix on docker manifest inspect as in the reusable workflow — Docker doesn't consume this variable. The credential was already written to Docker's store by cpln image docker-login.
| if CPLN_PROFILE="${upstream_profile}" docker manifest inspect "${source_image_ref}" >/dev/null && | |
| if docker manifest inspect "${source_image_ref}" >/dev/null && |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \ | ||
| '[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0' | ||
| )" | ||
| production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}" |
There was a problem hiding this comment.
Race condition on sequential promotions: latest_number is computed from a live API query, so two promotions running close together (e.g. a re-run while the first is still copying) can both read the same max number and produce the same production_image tag. The second cpln image copy would overwrite the first silently.
If the Control Plane API supports it, passing --to-name without the numeric prefix and letting the registry assign it, or using a commit-hash-only tag like ${PRODUCTION_APP_NAME}:${staging_commit}, would be race-free. If the incrementing number is intentional for ordering, consider adding a uniqueness guard (e.g. abort if the computed tag already exists in the production registry before copying).
| copy_status=1 | ||
| for attempt in $(seq 1 "${copy_image_attempts}"); do | ||
| if cpflow copy-image-from-upstream -a "${PRODUCTION_APP_NAME}" --org "${CPLN_ORG_PRODUCTION}" --image "${STAGING_IMAGE}"; then | ||
| if CPLN_PROFILE="${upstream_profile}" docker manifest inspect "${source_image_ref}" >/dev/null && |
There was a problem hiding this comment.
CPLN_PROFILE has no effect on docker manifest inspect — Docker reads credentials from its own config (set up by cpln image docker-login above), not from this env var. The prefix is harmless but misleading; it could imply the manifest check uses a different identity than it does.
| if CPLN_PROFILE="${upstream_profile}" docker manifest inspect "${source_image_ref}" >/dev/null && | |
| if docker manifest inspect "${source_image_ref}" >/dev/null && |
Code ReviewOverviewThis PR replaces The change is well-motivated: the previous workflow surfaced opaque failures when staging registry credentials weren't reachable, and Strengths
Issues / SuggestionsMedium —
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81c3f99b36
ℹ️ 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".
| exit 1 | ||
| fi | ||
|
|
||
| staging_commit="${STAGING_IMAGE##*_}" |
There was a problem hiding this comment.
When the staging workload was deployed with use_digest_image_ref, cpflow deploy-image writes a tagged digest reference (see lib/command/deploy_image.rb:72-75), so this workflow captures a STAGING_IMAGE like app:7_deadbeef@sha256:<digest>. This extraction keeps the digest in staging_commit, and line 381 then builds a --to-name such as prod:8_deadbeef@sha256:<digest>, which is not a valid destination image name/tag for cpln image copy; promotions from digest-pinned staging workloads will fail before deployment. Strip any @... digest from STAGING_IMAGE before extracting the commit and constructing production_image.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| latest_number="$( | ||
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json | | ||
| jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \ | ||
| '[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0' | ||
| )" | ||
| production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}" |
There was a problem hiding this comment.
Silent-failure risk in latest_number subshell assignment
In bash, var="$(cmd)" always exits 0 — even with set -o pipefail — so if cpln image query fails (network hiccup, API error), the pipeline produces no output, jq exits 0 on empty stdin, and latest_number is silently set to "". Bash then evaluates $(("" + 1)) as 1, meaning production_image would be tagged :1_<commit> and potentially overwrite an existing image.
Suggested fix: validate the result before using it in arithmetic:
| latest_number="$( | |
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json | | |
| jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \ | |
| '[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0' | |
| )" | |
| production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}" | |
| latest_number="$( | |
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json | | |
| jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \ | |
| '[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0' | |
| )" | |
| if ! [[ "${latest_number}" =~ ^[0-9]+$ ]]; then | |
| echo "::error::Failed to determine the latest production image number for '${PRODUCTION_APP_NAME}'; cpln image query may have failed." | |
| exit 1 | |
| fi | |
| production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}" |
| latest_number="$( | ||
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json | | ||
| jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \ | ||
| '[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0' | ||
| )" | ||
| production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}" |
There was a problem hiding this comment.
Same silent-failure risk as the companion comment in the reusable workflow: latest_number="$(subshell)" masks pipeline failures in bash, so a cpln image query error silently produces production_image="…:1_<commit>".
Apply the same guard here:
| latest_number="$( | |
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json | | |
| jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \ | |
| '[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0' | |
| )" | |
| production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}" | |
| latest_number="$( | |
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json | | |
| jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \ | |
| '[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0' | |
| )" | |
| if ! [[ "${latest_number}" =~ ^[0-9]+$ ]]; then | |
| echo "::error::Failed to determine the latest production image number for '${PRODUCTION_APP_NAME}'; cpln image query may have failed." | |
| exit 1 | |
| fi | |
| production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}" |
Code ReviewOverviewReplaces What works well
One issue to fix (both files)Silent failure in In bash, The inline comments include a suggested fix: add a Minor observations (non-blocking)
SummaryThe direction is right and the implementation is clean. The one concrete issue worth fixing before merge is the silent |
b8952a2 to
59e1dfc
Compare
Review: Use native Control Plane image copy in promotion workflowWhat this doesReplaces Strengths
Issues🔴 Race condition in production image numbering (both workflow files)
Possible mitigations:
🟡
|
| jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \ | ||
| '[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0' | ||
| )" | ||
| if ! [[ "${latest_number}" =~ ^[0-9]+$ ]]; then |
There was a problem hiding this comment.
Race condition: the cpln image query → increment → push sequence is not atomic. Two concurrent promotion runs can both read latest_number=N and both push :N+1_<sha>, with the second push silently overwriting the first. Both deploy-image steps then succeed against the same tag with no error.
The simplest fix is to drop the numeric counter entirely and use the commit SHA as the sole differentiator, which makes the push idempotent:
| if ! [[ "${latest_number}" =~ ^[0-9]+$ ]]; then | |
| production_image="${PRODUCTION_APP_NAME}:${staging_commit}" |
If the counter is required for ordering, add a uniqueness guard after the push (e.g., verify the pushed digest matches the remote digest) or serialize promotions via a GitHub environment concurrency limit.
| jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \ | ||
| '[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0' | ||
| )" | ||
| if ! [[ "${latest_number}" =~ ^[0-9]+$ ]]; then |
There was a problem hiding this comment.
Race condition (same issue as the deployed workflow): the cpln image query → increment → push sequence is not atomic. Two concurrent promotion runs can both read latest_number=N and both push :N+1_<sha>, with the second silently overwriting the first.
| if ! [[ "${latest_number}" =~ ^[0-9]+$ ]]; then | |
| production_image="${PRODUCTION_APP_NAME}:${staging_commit}" |
Using the commit SHA alone is idempotent and eliminates the race. If the numeric counter is required, serialize via GitHub environment concurrency limits or add a post-push digest check.
| copy_status=1 | ||
| for attempt in $(seq 1 "${copy_image_attempts}"); do | ||
| if cpflow copy-image-from-upstream -a "${PRODUCTION_APP_NAME}" --org "${CPLN_ORG_PRODUCTION}" --image "${STAGING_IMAGE}"; then | ||
| if printf '%s' "${CPLN_TOKEN_STAGING}" | |
There was a problem hiding this comment.
Silent multi-arch downgrade: docker pull without --platform fetches only the manifest for the runner's native architecture (linux/amd64 on GitHub-hosted runners). If the staging image is a multi-arch manifest list, the production registry ends up with a single-arch image on every promotion — arm64 and other variants are silently dropped.
If single-arch is intentional, document it explicitly:
| if printf '%s' "${CPLN_TOKEN_STAGING}" | | |
| docker pull --platform linux/amd64 "${source_image_ref}" && |
If multi-arch needs to be preserved, use docker buildx imagetools create --tag "${production_image_ref}" "${source_image_ref}" instead of the pull/tag/push sequence.
| copy_status=1 | ||
| for attempt in $(seq 1 "${copy_image_attempts}"); do | ||
| if cpflow copy-image-from-upstream -a "${PRODUCTION_APP_NAME}" --org "${CPLN_ORG_PRODUCTION}" --image "${STAGING_IMAGE}"; then | ||
| if printf '%s' "${CPLN_TOKEN_STAGING}" | |
There was a problem hiding this comment.
Silent multi-arch downgrade (same issue as the deployed workflow): docker pull without --platform fetches only the runner's native architecture. If the staging image is a multi-arch manifest list, only that one arch is pushed to production.
| if printf '%s' "${CPLN_TOKEN_STAGING}" | | |
| docker pull --platform linux/amd64 "${source_image_ref}" && |
For multi-arch preservation, prefer docker buildx imagetools create --tag "${production_image_ref}" "${source_image_ref}" which copies the full manifest list without pulling layers to the runner.
|
Code Review posted - see inline comments below for specific findings. |
Code Review: Use native Control Plane image copy in promotion workflowOverviewThis PR replaces Strengths
Issues1. Missing stderr redirect in
2. Docker logins run on every retry iteration Inside the retry loop, each attempt re-runs Suggested fix: perform both logins once before the loop, and only retry from 3. Image number race window (depends on the concurrency group) There is a TOCTOU gap between the 4. Full image pulled to runner disk on every promotion Switching from Minor / Non-blocking
Test CoverageThe spec additions are thorough -- positive assertions for all new shell fragments and negative assertions confirming complete removal of the old approach. No gaps identified. |
| if ! [[ "${value}" =~ ^[a-z0-9]([a-z0-9-]*[a-z0-9])?$ ]]; then | ||
| local display_value | ||
| display_value="$(printf '%q' "${value}")" | ||
| echo "::error::${label} (${display_value}) must be a valid Control Plane org name; use lowercase alphanumeric characters and hyphens only, with no leading or trailing hyphen." |
There was a problem hiding this comment.
Missing >&2 redirect — sanitize_control_plane_name uses >&2 for its error, but this one goes to stdout. GitHub Actions processes ::error:: annotations from either stream, so the annotation itself is fine, but for consistency and so that error output is on stderr when this script is run locally or piped, add >&2 here.
| echo "::error::${label} (${display_value}) must be a valid Control Plane org name; use lowercase alphanumeric characters and hyphens only, with no leading or trailing hyphen." | |
| echo "::error::${label} (${display_value}) must be a valid Control Plane org name; use lowercase alphanumeric characters and hyphens only, with no leading or trailing hyphen." >&2 |
| if ! [[ "${value}" =~ ^[a-z0-9]([a-z0-9-]*[a-z0-9])?$ ]]; then | ||
| local display_value | ||
| display_value="$(printf '%q' "${value}")" | ||
| echo "::error::${label} (${display_value}) must be a valid Control Plane org name; use lowercase alphanumeric characters and hyphens only, with no leading or trailing hyphen." |
There was a problem hiding this comment.
Same missing >&2 as the deployed workflow — error annotation goes to stdout instead of stderr. Should be kept in sync with the fix in the other file.
| echo "::error::${label} (${display_value}) must be a valid Control Plane org name; use lowercase alphanumeric characters and hyphens only, with no leading or trailing hyphen." | |
| echo "::error::${label} (${display_value}) must be a valid Control Plane org name; use lowercase alphanumeric characters and hyphens only, with no leading or trailing hyphen." >&2 |
| exit 1 | ||
| fi | ||
|
|
||
| latest_number="$( |
There was a problem hiding this comment.
There is a TOCTOU window between this query and the docker push below: if two promotion runs somehow bypassed the concurrency group (or the group was misconfigured), both would read the same latest_number and push to the same production tag, silently overwriting each other. The concurrency group closes this in practice, but the comment explaining the dependency is currently on the Deploy image step. Worth adding a note here so the invariant is visible at the point where it actually matters:
| latest_number="$( | |
| # The workflow-level concurrency group serializes access here: without it, | |
| # two concurrent runs could derive the same latest_number and overwrite each other. | |
| latest_number="$( | |
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json | | |
| jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \ | |
| '[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0' | |
| )" |
| copy_status=1 | ||
| for attempt in $(seq 1 "${copy_image_attempts}"); do | ||
| if cpflow copy-image-from-upstream -a "${PRODUCTION_APP_NAME}" --org "${CPLN_ORG_PRODUCTION}" --image "${STAGING_IMAGE}"; then | ||
| if printf '%s' "${CPLN_TOKEN_STAGING}" | |
There was a problem hiding this comment.
Both docker login calls run on every retry iteration. If the root cause of a failure is a transient docker pull blip, re-logging in is harmless but wasteful. If docker login itself is the flaky step (e.g. registry auth endpoint rate-limit), including it in the retry body means it becomes an extra failure surface on every attempt.
Consider moving both docker login calls to just before the loop so the loop only retries the actual image operations (manifest inspect → pull → tag → push). The DOCKER_CONFIG temp dir is already set up before the loop, so credentials will persist across iterations.
…roduction-image-copy-template * origin/main: Bump version to 5.1.0 Update CHANGELOG.md for 5.1.0 (#357)
| # The workflow-level concurrency group serializes this sequence so two | ||
| # production promotions cannot derive and publish the same next tag. | ||
| latest_number="$( | ||
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" --max 0 -o json | |
There was a problem hiding this comment.
Potential bug: --max 0 may return zero results.
Many CLIs and APIs treat a page-size of 0 as "return nothing" rather than "return everything". If cpln image query follows that convention, .items will always be empty, max // 0 will always produce 0, and every promotion will attempt to create tag :1. The pre-copy collision guard would then permanently block the second promotion.
Please verify via cpln image query --help (or the Control Plane docs) that --max 0 means unlimited. If it does not, replace with an explicit large ceiling:
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" --max 0 -o json | | |
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" --max 9999 -o json | |
and add a comment explaining the intent. This same concern applies to the parallel block in .github/workflows/cpflow-promote-staging-to-production.yml.
| missing_vars="$(comm -23 <(printf '%s\n' "${staging_vars}") <(printf '%s\n' "${production_vars}"))" | ||
| production_only_vars="$(comm -13 <(printf '%s\n' "${staging_vars}") <(printf '%s\n' "${production_vars}"))" |
There was a problem hiding this comment.
Minor bug: printf '%s\n' "" emits a blank line, not empty stdin.
When production_vars is an empty string (production has no env vars), printf '%s\n' "${production_vars}" outputs one blank line. comm -13 then sees that blank line as a production-only variable and emits a misleading warning — "Production GVC has environment variables not present in staging:" followed by an empty line — even though production is correctly empty.
The staging side has an early-return guard ([[ -z "${staging_vars}" ]]), but the production side has none. Suggested fix: strip blank lines from both inputs before feeding comm:
| missing_vars="$(comm -23 <(printf '%s\n' "${staging_vars}") <(printf '%s\n' "${production_vars}"))" | |
| production_only_vars="$(comm -13 <(printf '%s\n' "${staging_vars}") <(printf '%s\n' "${production_vars}"))" | |
| missing_vars="$(comm -23 <(printf '%s\n' "${staging_vars}") <(printf '%s\n' "${production_vars}" | grep -v '^$' || true))" | |
| production_only_vars="$(comm -13 <(printf '%s\n' "${staging_vars}") <(printf '%s\n' "${production_vars}" | grep -v '^$' || true))" |
The same change is needed in .github/workflows/cpflow-promote-staging-to-production.yml.
justin808
changed the title
|
|
||
| workload_ready="$(echo "${workload_json}" | jq -r '.status.ready // false')" | ||
| latest_ready="$(echo "${workload_json}" | jq -r '.status.readyLatest // false')" | ||
| readiness_status="$(echo "${workload_json}" | jq -r '.health.readiness // "unknown"')" |
There was a problem hiding this comment.
Likely wrong JSON path — will always show "unknown"
The Control Plane workload object returned by cpln workload get -o json does not have a top-level .health key. Health/readiness state lives under .status (e.g. .status.ready, .status.readyLatest). The .health.readiness path will resolve to null on every poll, so the // "unknown" fallback always fires.
This only affects the log line (not the readiness gate), but it makes the "waiting for latest deployment" message permanently say readiness=unknown and removes a useful diagnostic signal. Consider dropping this field until the correct path is confirmed from a real cpln workload response, or verify the path against the API and correct it.
| readiness_status="$(echo "${workload_json}" | jq -r '.health.readiness // "unknown"')" | |
| readiness_status="$(echo "${workload_json}" | jq -r '.status.readyLatest // "unknown"')" |
| # The workflow-level concurrency group serializes this sequence so two | ||
| # production promotions cannot derive and publish the same next tag. | ||
| latest_number="$( | ||
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" --max 0 -o json | |
There was a problem hiding this comment.
Verify --max 0 means "no limit", not "return 0 results"
The tag-numbering logic relies on this query returning all production images so it can find the highest numeric tag. If cpln image query --max 0 means "return up to 0 items" (rather than "unlimited"), then .items would always be empty, max // 0 would always return 0, and every promotion would derive tag :1. The second promotion would then hit the collision guard and abort.
The same pattern exists in the template file. If --max 0 means "no limit", this is correct — please add a short comment to that effect so the intent is clear:
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" --max 0 -o json | | |
| cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" --max 0 -o json | # --max 0 = no limit; returns all matching images |
| staging_workload_vars="$(list_workload_env_names "${CPLN_TOKEN_STAGING}" "${CPLN_ORG_STAGING}" "${STAGING_APP_NAME}" "${workload_name}")" | ||
| production_workload_vars="$(list_workload_env_names "${CPLN_TOKEN_PRODUCTION}" "${CPLN_ORG_PRODUCTION}" "${PRODUCTION_APP_NAME}" "${workload_name}")" |
There was a problem hiding this comment.
Unclear failure when a production workload doesn't exist
If workload_name exists in staging but the production GVC hasn't been bootstrapped yet, cpln workload get for production will fail with a raw cpln error (e.g. "workload not found") and set -euo pipefail will abort the step immediately — bypassing check_required_vars and env_check_failed=1. The operator sees a cpln CLI error rather than the actionable message like "Production workload 'rails' is missing environment variables...".
Consider wrapping the production fetch with an explicit not-found check:
production_workload_vars="$(list_workload_env_names ... 2>/dev/null)" || {
echo "::error::Production workload '${workload_name}' was not found in GVC '${PRODUCTION_APP_NAME}'; bootstrap it before promotion."
env_check_failed=1
continue
}Same issue in the template file.
|
|
||
| staging_registry="${CPLN_ORG_STAGING}.registry.cpln.io" | ||
| production_registry="${CPLN_ORG_PRODUCTION}.registry.cpln.io" | ||
| source_image_ref="${staging_registry}/${STAGING_IMAGE}" |
There was a problem hiding this comment.
source_image_ref intentionally uses the original (possibly digest-suffixed) STAGING_IMAGE
This is correct for digest-pinned copies — docker buildx imagetools create will use registry/app:tag@sha256:abc as the source, pulling by digest instead of by tag. Just noting this is a deliberate asymmetry vs staging_image (which strips @sha256:... for the cpln image get validation). A brief comment here would help future readers:
| source_image_ref="${staging_registry}/${STAGING_IMAGE}" | |
| source_image_ref="${staging_registry}/${STAGING_IMAGE}" # use original ref (may include @sha256 digest) for digest-preserving copy |
| if docker buildx imagetools inspect "${source_image_ref}" >/dev/null && | ||
| docker buildx imagetools create --prefer-index=false --tag "${production_image_ref}" "${source_image_ref}"; then |
There was a problem hiding this comment.
Source re-inspect on every retry is redundant (and can mask transient create failures)
Each retry re-runs imagetools inspect "${source_image_ref}" before imagetools create. The source image doesn't change between retries, so repeated inspection adds latency without value. More importantly: if inspect passes but create fails transiently, the retry will show a combined failure exit code from the && compound — which is correct, but it's easier to diagnose if the two commands are separated:
| if docker buildx imagetools inspect "${source_image_ref}" >/dev/null && | |
| docker buildx imagetools create --prefer-index=false --tag "${production_image_ref}" "${source_image_ref}"; then | |
| inspect_ok=0 | |
| if ! docker buildx imagetools inspect "${source_image_ref}" >/dev/null; then | |
| inspect_ok=1 | |
| fi | |
| if [[ "${inspect_ok}" -eq 0 ]] && docker buildx imagetools create --prefer-index=false --tag "${production_image_ref}" "${source_image_ref}"; then |
Alternatively, move the imagetools inspect "${source_image_ref}" above the retry loop (it only needs to run once) and let the loop contain only imagetools create.
PR Review: Harden production promotion image copy and readinessOverviewThis PR addresses real, production-confirmed failure modes — particularly stale-ready replica masking a broken latest deployment, CR/LF-corrupted org names, and missing env vars not caught before image copy. The overall approach is sound and the security practices (isolated Docker config dir, credential cleanup via Potential Bug
The entire tag-numbering sequence — querying all production images to find the highest Code Quality
SecurityThe credential isolation ( Pinning Tests / Docs
SummaryStrong PR overall — catches real promotion failures and the image-copy approach is more robust than |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/github_flow_templates/.github/workflows/cpflow-promote-staging-to-production.yml (1)
470-485:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDigest-only staging refs still fail before the Buildx copy runs.
If the workload is pinned as
app@sha256:...,${STAGING_IMAGE%%@*}becomes justapp, socpln image get "${staging_image}"aborts the promotion even thoughdocker buildx imagetoolscan copy that digest ref. This breaks the digest-pinned path the new workflow is trying to support.Suggested fix
- staging_image="${STAGING_IMAGE%%@*}" - if [[ -z "${staging_image}" ]]; then + staging_image="${STAGING_IMAGE%%@*}" + if [[ -z "${staging_image}" ]]; then echo "::error::Staging image '${STAGING_IMAGE}' did not contain a usable image tag." exit 1 fi - if ! CPLN_TOKEN="${CPLN_TOKEN_STAGING}" cpln image get "${staging_image}" --org "${CPLN_ORG_STAGING}" -o json >/dev/null; then + has_tag_ref=false + if [[ "${staging_image}" == *:* ]]; then + has_tag_ref=true + fi + + if [[ "${has_tag_ref}" == "true" ]] && + ! CPLN_TOKEN="${CPLN_TOKEN_STAGING}" cpln image get "${staging_image}" --org "${CPLN_ORG_STAGING}" -o json >/dev/null; then echo "::error::Staging image '${STAGING_IMAGE}' was not found in org '${CPLN_ORG_STAGING}'; aborting promotion." exit 1 fi - staging_tag="${staging_image##*:}" + staging_tag="" + if [[ "${has_tag_ref}" == "true" ]]; then + staging_tag="${staging_image##*:}" + fi🤖 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 `@lib/github_flow_templates/.github/workflows/cpflow-promote-staging-to-production.yml` around lines 470 - 485, The script currently strips the entire "@..." suffix from STAGING_IMAGE into staging_image which turns a digest ref like "app@sha256:..." into just "app" and causes the cpln image get check to fail; change the logic so staging_image preserves digest refs (i.e., if STAGING_IMAGE contains '@' set staging_image="${STAGING_IMAGE}" else keep the existing stripping behavior), and update staging_tag extraction to handle both tag and digest forms (if staging_image contains '@' set staging_tag to the substring after '@', elseif it contains ':' set staging_tag to the substring after ':'; otherwise leave empty); keep using the same symbols staging_image, STAGING_IMAGE, staging_tag and the cpln image get check so the correct ref is validated.
🤖 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.
Outside diff comments:
In
`@lib/github_flow_templates/.github/workflows/cpflow-promote-staging-to-production.yml`:
- Around line 470-485: The script currently strips the entire "@..." suffix from
STAGING_IMAGE into staging_image which turns a digest ref like "app@sha256:..."
into just "app" and causes the cpln image get check to fail; change the logic so
staging_image preserves digest refs (i.e., if STAGING_IMAGE contains '@' set
staging_image="${STAGING_IMAGE}" else keep the existing stripping behavior), and
update staging_tag extraction to handle both tag and digest forms (if
staging_image contains '@' set staging_tag to the substring after '@', elseif it
contains ':' set staging_tag to the substring after ':'; otherwise leave empty);
keep using the same symbols staging_image, STAGING_IMAGE, staging_tag and the
cpln image get check so the correct ref is validated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8abbc9f4-694c-404a-bab9-4e08a1aa7c52
📒 Files selected for processing (3)
.github/workflows/cpflow-promote-staging-to-production.ymllib/github_flow_templates/.github/workflows/cpflow-promote-staging-to-production.ymlspec/command/generate_github_actions_spec.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/cpflow-promote-staging-to-production.yml
- spec/command/generate_github_actions_spec.rb
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues.
Reviewed by Cursor Bugbot for commit 6cd8225. Configure here.
| staging_commit="" | ||
| if [[ "${staging_tag}" == *_* ]]; then | ||
| staging_commit="${staging_tag##*_}" | ||
| fi |
There was a problem hiding this comment.
Digest pin drops commit suffix
Medium Severity
When the staging image reference includes a digest (@sha256:…), commit suffix extraction uses the digest segment instead of the tag before @, so _<commit> is not copied onto the new production tag despite documented behavior.
Reviewed by Cursor Bugbot for commit 6cd8225. Configure here.
| echo | ||
| echo "Previous image: \`${PREVIOUS_IMAGE}\`" | ||
| echo "Previous version: ${PREVIOUS_VERSION}" | ||
| echo "Deployed image: \`${deployed_image}\`" |
There was a problem hiding this comment.
Summary mislabels failed promotion image
Medium Severity
On a failed health check, the promotion summary always prints Deployed image as the pre-promotion reference, even when rollback did not run or failed and production may still be running the newly copied image.
Reviewed by Cursor Bugbot for commit 6cd8225. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cd8225efa
ℹ️ 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".
| if [[ "${STAGING_IMAGE}" == *@* ]]; then | ||
| staging_image="${STAGING_IMAGE}" | ||
| else | ||
| staging_image="${STAGING_IMAGE%%@*}" | ||
| fi |
There was a problem hiding this comment.
When the staging workload is digest-pinned, STAGING_IMAGE is shaped like app:7_deadbeef@sha256:...; this branch keeps the @... suffix instead of removing it, so cpln image get is asked for a tagged-digest string rather than the Control Plane image name and the later tag parsing reads sha256:... instead of 7_deadbeef. Fresh evidence in this revision is that the new normalization branch preserves @ references on lines 451-455, so digest-pinned staging promotions still abort or lose the commit suffix instead of promoting the tag portion documented for this workflow.
Useful? React with 👍 / 👎.
Code Review — PR #356: Harden production promotion image copy and readinessOverviewThis PR addresses real production failure modes discovered in a downstream tutorial: stale-replica health masking, registry auth not crossing workflow boundaries, CR/LF-corrupted org names, and the old Bug: Commit suffix not preserved for digest-pinned imagesFor a digest-pinned staging image like Suggested replacement: staging_tag=""
if [[ "${staging_image}" == *:*@* ]]; then
staging_tag="${staging_image##*:}" # e.g. 123_abc@sha256:...
staging_tag="${staging_tag%%@*}" # strip digest -> 123_abc
elif [[ "${staging_image}" == *@* ]]; then
staging_tag="" # digest-only ref, no tag to extract
elif [[ "${staging_image}" == *:* ]]; then
staging_tag="${staging_image##*:}"
fiSame fix is needed in both the reusable workflow and the template. Minor / Code QualityDead The
Validation uses No version annotation on the pinned Pin-by-hash is the right security call, but without a version comment reviewers cannot tell which release this hash corresponds to. A trailing Operational note:
|
| staging_tag="" | ||
| if [[ "${staging_image}" == *@* ]]; then | ||
| staging_tag="${staging_image##*@}" | ||
| elif [[ "${staging_image}" == *:* ]]; then | ||
| staging_tag="${staging_image##*:}" | ||
| fi | ||
| staging_commit="" | ||
| if [[ "${staging_tag}" == *_* ]]; then | ||
| staging_commit="${staging_tag##*_}" | ||
| fi |
There was a problem hiding this comment.
Bug: commit suffix is never extracted for digest-pinned images.
When staging_image is myapp:123_abc@sha256:deadbeef, the *@* branch sets staging_tag = "sha256:deadbeef" (the digest). Because a digest hash never contains _, staging_commit stays empty and the _abc suffix is silently dropped from the production tag — contradicting the documented behaviour.
| staging_tag="" | |
| if [[ "${staging_image}" == *@* ]]; then | |
| staging_tag="${staging_image##*@}" | |
| elif [[ "${staging_image}" == *:* ]]; then | |
| staging_tag="${staging_image##*:}" | |
| fi | |
| staging_commit="" | |
| if [[ "${staging_tag}" == *_* ]]; then | |
| staging_commit="${staging_tag##*_}" | |
| fi | |
| staging_tag="" | |
| if [[ "${staging_image}" == *:*@* ]]; then | |
| staging_tag="${staging_image##*:}" # e.g. 123_abc@sha256:... | |
| staging_tag="${staging_tag%%@*}" # strip digest -> 123_abc | |
| elif [[ "${staging_image}" == *@* ]]; then | |
| staging_tag="" # digest-only ref, no tag to extract | |
| elif [[ "${staging_image}" == *:* ]]; then | |
| staging_tag="${staging_image##*:}" | |
| fi | |
| staging_commit="" | |
| if [[ "${staging_tag}" == *_* ]]; then | |
| staging_commit="${staging_tag##*_}" | |
| fi |
| staging_tag="" | ||
| if [[ "${staging_image}" == *@* ]]; then | ||
| staging_tag="${staging_image##*@}" | ||
| elif [[ "${staging_image}" == *:* ]]; then | ||
| staging_tag="${staging_image##*:}" | ||
| fi | ||
| staging_commit="" | ||
| if [[ "${staging_tag}" == *_* ]]; then | ||
| staging_commit="${staging_tag##*_}" | ||
| fi |
There was a problem hiding this comment.
Bug: same commit suffix extraction issue as in the reusable workflow — for myapp:123_abc@sha256:deadbeef, staging_tag receives the digest (sha256:deadbeef), not the tag, so staging_commit is always empty for digest-pinned images.
| staging_tag="" | |
| if [[ "${staging_image}" == *@* ]]; then | |
| staging_tag="${staging_image##*@}" | |
| elif [[ "${staging_image}" == *:* ]]; then | |
| staging_tag="${staging_image##*:}" | |
| fi | |
| staging_commit="" | |
| if [[ "${staging_tag}" == *_* ]]; then | |
| staging_commit="${staging_tag##*_}" | |
| fi | |
| staging_tag="" | |
| if [[ "${staging_image}" == *:*@* ]]; then | |
| staging_tag="${staging_image##*:}" # e.g. 123_abc@sha256:... | |
| staging_tag="${staging_tag%%@*}" # strip digest -> 123_abc | |
| elif [[ "${staging_image}" == *@* ]]; then | |
| staging_tag="" # digest-only ref, no tag to extract | |
| elif [[ "${staging_image}" == *:* ]]; then | |
| staging_tag="${staging_image##*:}" | |
| fi | |
| staging_commit="" | |
| if [[ "${staging_tag}" == *_* ]]; then | |
| staging_commit="${staging_tag##*_}" | |
| fi |
| if [[ "${STAGING_IMAGE}" == *@* ]]; then | ||
| staging_image="${STAGING_IMAGE}" | ||
| else | ||
| staging_image="${STAGING_IMAGE%%@*}" | ||
| fi |
There was a problem hiding this comment.
Dead code: the else branch only runs when STAGING_IMAGE contains no @. In that case %%@* strips nothing and staging_image == STAGING_IMAGE. The whole block is equivalent to staging_image="${STAGING_IMAGE}".
| if [[ "${STAGING_IMAGE}" == *@* ]]; then | |
| staging_image="${STAGING_IMAGE}" | |
| else | |
| staging_image="${STAGING_IMAGE%%@*}" | |
| fi | |
| staging_image="${STAGING_IMAGE}" |
|
|
||
| if ! CPLN_TOKEN="${CPLN_TOKEN_STAGING}" cpln image get "${STAGING_IMAGE}" --org "${CPLN_ORG_STAGING}" -o json >/dev/null; then | ||
| if [[ "${STAGING_IMAGE}" == *@* ]]; then | ||
| staging_image="${STAGING_IMAGE}" | ||
| else | ||
| staging_image="${STAGING_IMAGE%%@*}" | ||
| fi |
There was a problem hiding this comment.
Same dead-code issue as in the reusable workflow — %%@* is a no-op when there is no @.
| if ! CPLN_TOKEN="${CPLN_TOKEN_STAGING}" cpln image get "${STAGING_IMAGE}" --org "${CPLN_ORG_STAGING}" -o json >/dev/null; then | |
| if [[ "${STAGING_IMAGE}" == *@* ]]; then | |
| staging_image="${STAGING_IMAGE}" | |
| else | |
| staging_image="${STAGING_IMAGE%%@*}" | |
| fi | |
| staging_image="${STAGING_IMAGE}" |
|
|
||
| staging_registry="${CPLN_ORG_STAGING}.registry.cpln.io" | ||
| production_registry="${CPLN_ORG_PRODUCTION}.registry.cpln.io" | ||
| source_image_ref="${staging_registry}/${STAGING_IMAGE}" |
There was a problem hiding this comment.
Minor: uses ${STAGING_IMAGE} (raw env var) while all validation above used ${staging_image} (normalized local). They are always equal today, but using the local variable here would be consistent and remove a maintenance trap.
| source_image_ref="${staging_registry}/${STAGING_IMAGE}" | |
| source_image_ref="${staging_registry}/${staging_image}" |
|
|
||
| staging_registry="${CPLN_ORG_STAGING}.registry.cpln.io" | ||
| production_registry="${CPLN_ORG_PRODUCTION}.registry.cpln.io" | ||
| source_image_ref="${staging_registry}/${STAGING_IMAGE}" |
There was a problem hiding this comment.
Same inconsistency as in the reusable workflow — prefer ${staging_image} for consistency with the validation above.
| source_image_ref="${staging_registry}/${STAGING_IMAGE}" | |
| source_image_ref="${staging_registry}/${staging_image}" |
| echo "image=${staging_image}" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 |
There was a problem hiding this comment.
Pin-by-hash is correct for security, but please add a version comment so reviewers can identify the release without having to look up the hash.
| uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 | |
| uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # vX.Y.Z |
| echo "image=${staging_image}" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 |
There was a problem hiding this comment.
Same as the reusable workflow — add a version comment alongside the pinned hash.
| uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 | |
| uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # vX.Y.Z |
1 participant
Summary
status.readyandstatus.readyLatestbefore endpoint health checks and rollback readiness pollingWhy
The downstream tutorial promotion exposed several production-only failure modes:
environment secrets do not cross reusable workflow boundaries, Control Plane
profile selection does not carry through Docker credential helpers, pasted
CR/LF values can break raw registry hosts, and a promotion can look healthy when
an older ready replica still answers while the latest workload version cannot
boot. This PR updates the reusable workflow/template to catch those failures
before production is promoted or before a stale endpoint can mask a failed
deployment.
Successful downstream promotion after the fixes:
https://github.com/shakacode/react-webpack-rails-tutorial/actions/runs/26854234803
Validation
git diff --checkbundle exec rake check_command_docsCPLN_ORG=dummy-test bundle exec rspec spec/command/generate_github_actions_spec.rbbundle exec rubocop spec/command/generate_github_actions_spec.rbNote
High Risk
Changes the production promotion path (registry auth, image tagging/copy, deploy/rollback gates) and production health semantics; mistakes could block or mis-handle live promotions.
Overview
Production promotion is hardened end-to-end: org names are trimmed and validated before any Control Plane or registry step; staging→production image copy no longer uses
cpflow copy-image-from-upstreamand instead logs into both registries with tokens, derives the next production tag (optionally keeping a_commitsuffix), and copies viadocker buildx imagetools create(including digest-pinned sources) with a collision check on the target tag.Pre-deploy gates now compare env var names from staging to production at both the GVC and each configured app workload container, failing the run before copy when production is missing names staging has.
Readiness for promote health checks and rollback polling requires
status.readyandstatus.readyLatestbefore HTTP endpoint checks, so an older ready replica cannot mask a failed latest revision.Docs and generator specs are updated to match the new workflow and wait-for-health behavior; the promotion summary reports the copied production image on success.
Reviewed by Cursor Bugbot for commit 6cd8225. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Release Notes
New Features
Documentation