chore: remove dagger#4173
Merged
Merged
Conversation
Contributor
|
Caution Review failedPull request was closed or merged during review 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:
📝 WalkthroughWalkthroughThis PR removes Dagger-based tooling and configuration, migrates CI/release flows to Nix/Makefile/GitHub Actions, adds a local migration SQLC testdata generator and benthos-collector version ldflag support, and updates linter/IDE/FOSSA exclusions and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as "GitHub Actions"
participant Nix as "Nix / nix develop"
participant Builder as "Make / Helm / Go build"
participant Artifacts as "GitHub Artifacts / OCI registry"
participant Release as "GitHub Release / npm registry"
GH->>Nix: set up Nix, restore caches
Nix->>Builder: run build/package commands (helm package, cross-build, archive)
Builder->>Artifacts: upload artifacts (tgz, tar.gz)
GH->>Artifacts: fetch artifacts for release job
GH->>Release: create GitHub Release and attach assets
GH->>Release: call npm-release workflow to publish JS SDK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
AGENTS.md (1)
7-14: Tiny duplication heads-up 🙃Lines 7 and 14 both now say essentially the same thing about
Makefilebeing primary andjustfilebeing seldom used. Not a blocker at all, but if you feel like tidying, you could drop one mention and leave the other as the canonical note.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 7 - 14, Duplicate note about using the Makefile appears in AGENTS.md; remove one of the repeated sentences so the guidance is only stated once. Edit the AGENTS.md content block that mentions "Development commands are run via `Makefile`..." or the earlier line "Use the `Makefile` for all common tasks..." (whichever you prefer to keep) and delete the other redundant sentence, keeping a single canonical mention of `Makefile` and the optional note about `justfile`; ensure the remaining paragraph reads smoothly and no other contextual info is lost..github/workflows/release.yaml (2)
179-188: Consider marking pre-release tags as GitHub pre-releases.
gh release createdefaults to a non-prerelease entry, so av1.2.3-beta.1tag will show up as a regular release on the Releases page and in "latest release" API responses. A tiny conditional keeps the Releases UI honest:🏷️ Suggested tweak
run: | + prerelease_flag="" + case "${GITHUB_REF_NAME}" in + *-beta.*|*-dev.*) prerelease_flag="--prerelease" ;; + esac gh release create "${GITHUB_REF_NAME}" \ --title "${GITHUB_REF_NAME}" \ --generate-notes \ - --verify-tag \ + --verify-tag \ + $prerelease_flag \ dist/benthos-collector_*.tar.gz \ dist/checksums.txt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yaml around lines 179 - 188, The GitHub release step uses gh release create (invoked as gh release create "${GITHUB_REF_NAME}" ...) which always makes a full release; detect prerelease tags (e.g. GITHUB_REF_NAME containing a hyphen like v1.2.3-beta.1) and pass the --prerelease flag when true. Update the workflow to compute a boolean/variable (e.g. PRERELEASE) from GITHUB_REF_NAME and conditionally include --prerelease in the gh release create invocation so prerelease tags are marked correctly in Releases.
67-71: Minor: prefer${{ secrets.GITHUB_TOKEN }}over${{ github.token }}for consistency, and consider using the helm OCI--registry-configflow.Totally cosmetic — both
github.tokenandsecrets.GITHUB_TOKENresolve to the same value, but the rest of the file usessecrets.GITHUB_TOKEN(e.g. the Nix setup steps), so using one style everywhere makes it easier to grep for auth flows. Also, piping the token throughechointohelm registry loginworks fine; if you ever wanted to avoid token material on a pipeline,helm registry login --password-stdinfrom an env var (printenv GH_TOKEN | ...) is a common alternative. Not a blocker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yaml around lines 67 - 71, Replace the use of `${{ github.token }}` with `${{ secrets.GITHUB_TOKEN }}` in the "Login to GitHub Container Registry" step and switch the login to use an env-var + stdin or the helm OCI `--registry-config` flow; specifically, update the `helm registry login ghcr.io --username "${{ github.actor }}" --password-stdin` invocation in that step to consume `GITHUB_TOKEN` from the environment (e.g., `echo "${{ secrets.GITHUB_TOKEN }}" | nix develop ... -c ... --password-stdin`) or refactor to use `helm registry login --registry-config <path>` per your OCI config convention so the token usage matches the rest of the workflow.Makefile (1)
55-64: Nice guard, but consider resetting the tree if the check fails.
migrate-check-schemarunsgo generate -x ./openmeter/ent/...which can mutate the tree. On CI that's fine — the job fails and the runner is tossed. Locally though, a dev runningmake migrate-checkwill get a dirty worktree even when they intended only to validate. Totally optional, but a hint in the failure message telling them togit checkout -- openmeter/ent(or the schema-not-in-sync message already suggests regen + commit, which also works) keeps things friendly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 55 - 64, The failure of the migrate-check-schema target leaves generated changes in the working tree; update the migrate-check-schema Makefile recipe (the target named migrate-check-schema that runs go generate -x ./openmeter/ent/...) so that when the check detects differences it also resets the mutated files before exiting (e.g., run a git checkout -- openmeter/ent or git restore --staged/--worktree for openmeter/ent) and update the final echo to advise the user to either regenerate & commit or run the provided reset command to revert local changes.tools/migrate/generate-sqlc-testdata.sh (1)
17-56: Small robustness suggestion: validateVERSIONand clean up on failure.Two nits that'll make this script friendlier for folks hacking on it locally:
- Scratch DB leaks on failure.
set -euo pipefailmeans any failure between creating and dropping the scratch DB (e.g., a broken migration) will leavesqlc_gen_<VERSION>behind, and a re-run will thenDROP DATABASE IF EXISTSbefore recreating — which is mostly fine, but atrapcleanup is nicer and also covers interrupts.VERSIONgoes straight into SQL and a migrate CLI arg. It's expected to be a timestamp, but an early regex check would fail fast with a clear message instead of producing weird SQL errors.🛡️ Suggested tweak
if [[ -z "${VERSION:-}" ]]; then echo "ERROR: VERSION is required (e.g. VERSION=20240826120919)" >&2 exit 1 fi + +if [[ ! "${VERSION}" =~ ^[0-9]+$ ]]; then + echo "ERROR: VERSION must be a numeric migration timestamp (got: ${VERSION})" >&2 + exit 1 +fi @@ SCRATCH_DB="sqlc_gen_${VERSION}" + +cleanup() { + psql -h "${PG_HOST}" -p "${PG_PORT}" -U "${PG_USER}" -d postgres \ + -c "DROP DATABASE IF EXISTS ${SCRATCH_DB};" >/dev/null 2>&1 || true +} +trap cleanup EXITAnd then the explicit drop at the end can go away since the trap handles it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/migrate/generate-sqlc-testdata.sh` around lines 17 - 56, The script can leave the scratch DB (SCRATCH_DB) behind on failures and accepts any VERSION string; add a sanity check and a cleanup trap: validate VERSION with a strict regex (e.g., only digits of expected length) before using it, and register a trap that drops SCRATCH_DB (using the same psql DROP DATABASE IF EXISTS "${SCRATCH_DB}" command) on EXIT/INT/TERM so the DB is removed if migrate or other steps fail; update places that reference VERSION, SCRATCH_DB, psql and migrate to rely on the validated VERSION and remove the final explicit DROP if the trap covers cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yaml:
- Around line 220-227: The JS_SDK_RELEASE_TAG is hard-coded to "latest" which
will publish prereleases under the latest dist-tag; change the Publish NPM
package step to derive JS_SDK_RELEASE_TAG from github.ref_name instead of
hard-coding it (use a conditional expression or a small shell step) so that if
github.ref_name contains "beta" it becomes "beta", if it contains "dev" or
similar it becomes "dev"/"next", otherwise it remains "latest"; update the
environment variable JS_SDK_RELEASE_TAG used by the publish-javascript-sdk
target accordingly.
In `@Makefile`:
- Around line 110-124: Add a new file cmd/benthos-collector/version.go that
declares the package main, a package-level variable var version string, and an
init() that sets version = "unknown" when empty so the Makefile's -X
main.version linker flag can inject the build version; follow the same pattern
used by cmd/server/version.go (declare var version string and initialize to
"unknown" in init()).
---
Nitpick comments:
In @.github/workflows/release.yaml:
- Around line 179-188: The GitHub release step uses gh release create (invoked
as gh release create "${GITHUB_REF_NAME}" ...) which always makes a full
release; detect prerelease tags (e.g. GITHUB_REF_NAME containing a hyphen like
v1.2.3-beta.1) and pass the --prerelease flag when true. Update the workflow to
compute a boolean/variable (e.g. PRERELEASE) from GITHUB_REF_NAME and
conditionally include --prerelease in the gh release create invocation so
prerelease tags are marked correctly in Releases.
- Around line 67-71: Replace the use of `${{ github.token }}` with `${{
secrets.GITHUB_TOKEN }}` in the "Login to GitHub Container Registry" step and
switch the login to use an env-var + stdin or the helm OCI `--registry-config`
flow; specifically, update the `helm registry login ghcr.io --username "${{
github.actor }}" --password-stdin` invocation in that step to consume
`GITHUB_TOKEN` from the environment (e.g., `echo "${{ secrets.GITHUB_TOKEN }}" |
nix develop ... -c ... --password-stdin`) or refactor to use `helm registry
login --registry-config <path>` per your OCI config convention so the token
usage matches the rest of the workflow.
In `@AGENTS.md`:
- Around line 7-14: Duplicate note about using the Makefile appears in
AGENTS.md; remove one of the repeated sentences so the guidance is only stated
once. Edit the AGENTS.md content block that mentions "Development commands are
run via `Makefile`..." or the earlier line "Use the `Makefile` for all common
tasks..." (whichever you prefer to keep) and delete the other redundant
sentence, keeping a single canonical mention of `Makefile` and the optional note
about `justfile`; ensure the remaining paragraph reads smoothly and no other
contextual info is lost.
In `@Makefile`:
- Around line 55-64: The failure of the migrate-check-schema target leaves
generated changes in the working tree; update the migrate-check-schema Makefile
recipe (the target named migrate-check-schema that runs go generate -x
./openmeter/ent/...) so that when the check detects differences it also resets
the mutated files before exiting (e.g., run a git checkout -- openmeter/ent or
git restore --staged/--worktree for openmeter/ent) and update the final echo to
advise the user to either regenerate & commit or run the provided reset command
to revert local changes.
In `@tools/migrate/generate-sqlc-testdata.sh`:
- Around line 17-56: The script can leave the scratch DB (SCRATCH_DB) behind on
failures and accepts any VERSION string; add a sanity check and a cleanup trap:
validate VERSION with a strict regex (e.g., only digits of expected length)
before using it, and register a trap that drops SCRATCH_DB (using the same psql
DROP DATABASE IF EXISTS "${SCRATCH_DB}" command) on EXIT/INT/TERM so the DB is
removed if migrate or other steps fail; update places that reference VERSION,
SCRATCH_DB, psql and migrate to rely on the validated VERSION and remove the
final explicit DROP if the trap covers cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a2c92f65-d3de-4fad-81cb-2314e9a82ee7
⛔ Files ignored due to path filters (4)
.dagger/go.modis excluded by!**/*.mod.dagger/go.sumis excluded by!**/*.sum,!**/*.sumapi/client/javascript/Makefileis excluded by!api/client/**flake.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (23)
.dagger/.gitattributes.dagger/.gitignore.dagger/build.go.dagger/generate.go.dagger/main.go.dagger/migrate.go.dagger/release.go.dagger/utils.go.dagger/versions.go.dagger/versions_pinned.go.fossa.yaml.github/workflows/ci.yaml.github/workflows/release.yaml.golangci-fast.yaml.golangci.yaml.vscode/default.code-workspaceAGENTS.mdMakefiledagger.jsonflake.nixjustfileopenmeter/testutils/pg_driver.gotools/migrate/generate-sqlc-testdata.sh
💤 Files with no reviewable changes (16)
- .golangci.yaml
- .golangci-fast.yaml
- .dagger/.gitignore
- .vscode/default.code-workspace
- .dagger/.gitattributes
- .dagger/versions_pinned.go
- .dagger/main.go
- .dagger/versions.go
- .dagger/generate.go
- .dagger/utils.go
- dagger.json
- .fossa.yaml
- flake.nix
- .dagger/release.go
- .dagger/migrate.go
- .dagger/build.go
tothandras
force-pushed
the
chore/remove-dagger
branch
from
5b0b075 to
b177189
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/release.yaml (1)
236-248:⚠️ Potential issue | 🟠 MajorKeep prerelease tags off npm
latest, and make beta versions rerunnable.Because lines 5-8 include
-dev/-betatags, the tag branch here still sends prereleases tolatest. Also, non-tag publishes use only the short SHA, so a manual rerun for the same commit tries to republish the same immutable npm version.Suggested metadata tweak
- name: Determine version and npm dist-tag id: meta env: REF_TYPE: ${{ github.ref_type }} + RUN_NUMBER: ${{ github.run_number }} run: | if [[ "$REF_TYPE" == "tag" ]]; then - echo "version=${GITHUB_REF_NAME}" >> "$GITHUB_OUTPUT" - echo "dist-tag=latest" >> "$GITHUB_OUTPUT" + echo "version=${GITHUB_REF_NAME#v}" >> "$GITHUB_OUTPUT" + case "$GITHUB_REF_NAME" in + *-beta.*) echo "dist-tag=beta" >> "$GITHUB_OUTPUT" ;; + *-dev.*) echo "dist-tag=dev" >> "$GITHUB_OUTPUT" ;; + *) echo "dist-tag=latest" >> "$GITHUB_OUTPUT" ;; + esac else short_sha="${GITHUB_SHA:0:12}" - echo "version=1.0.0-beta-${short_sha}" >> "$GITHUB_OUTPUT" + echo "version=1.0.0-beta-${short_sha}.${RUN_NUMBER}" >> "$GITHUB_OUTPUT" echo "dist-tag=beta" >> "$GITHUB_OUTPUT" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yaml around lines 236 - 248, Update the "Determine version and npm dist-tag" step (id: meta) so prerelease tags don't get dist-tag=latest and non-tag beta versions are unique per workflow run: when REF_TYPE=="tag" check GITHUB_REF_NAME for a prerelease marker (e.g., contains '-') and only set dist-tag=latest for normal semver tags, otherwise set dist-tag=beta; when not a tag, append a run-unique value (use GITHUB_RUN_ID or GITHUB_RUN_NUMBER) to the generated beta version (instead of only the short SHA) so reruns produce a new npm version; keep references to GITHUB_REF_NAME and GITHUB_SHA usage in the same step.
🧹 Nitpick comments (2)
tools/migrate/generate-sqlc-testdata.sh (1)
48-93: Consider a cleanup trap so scratch DBs don't linger on failure.If
migrate goto,pg_dump, orsqlc generatefail, the script exits viaset -eand leavessqlc_gen_<VERSION>behind. It's self-healing (next run drops-if-exists), but anEXITtrap keeps things tidy and also protects against Ctrl-C mid-run.♻️ Suggested tweak
echo ">>> Creating scratch database ${SCRATCH_DB}" psql -h "${PG_HOST}" -p "${PG_PORT}" -U "${PG_USER}" -d postgres \ -c "DROP DATABASE IF EXISTS ${SCRATCH_DB};" \ -c "CREATE DATABASE ${SCRATCH_DB};" + +cleanup() { + psql -h "${PG_HOST}" -p "${PG_PORT}" -U "${PG_USER}" -d postgres \ + -c "DROP DATABASE IF EXISTS ${SCRATCH_DB};" >/dev/null 2>&1 || true +} +trap cleanup EXIT…and then you can drop the explicit drop block at lines 91–93.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/migrate/generate-sqlc-testdata.sh` around lines 48 - 93, Add a shell EXIT trap to always drop the scratch DB (using SCRATCH_DB with psql) so failures or Ctrl-C won't leave sqlc_gen_<VERSION> behind; implement the trap near the top of the script (after SCRATCH_DB is set) and call psql -c "DROP DATABASE IF EXISTS ${SCRATCH_DB};" in the trap, then remove the explicit final DROP block after sqlc generate; ensure the trap runs regardless of where migrate, pg_dump, or sqlc generate fail and that any necessary environment variables (PG_HOST, PG_PORT, PG_USER) are available to the trap.AGENTS.md (1)
7-14: Trim the duplicate Makefile/justfile note.Lines 7 and 14 now say the same thing. I’d keep the short Quick Reference entry and remove or shorten the second mention so this guidance doesn’t drift later.
Based on learnings, AGENTS.md should fold guidance into the most relevant section and remove stale or duplicate notes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 7 - 14, The AGENTS.md contains duplicated notes about using the Makefile and justfile (the same guidance appears in the Quick Reference and again later); edit AGENTS.md to keep the short Quick Reference entry that states "Use the `Makefile` for all common tasks. A `justfile` also exists but is seldom used." and remove or condense the later paragraph that repeats this guidance so only one clear reference to `Makefile`/`justfile` remains; ensure the remaining text is folded into the most relevant section and eliminate the second mention to prevent drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yaml:
- Around line 250-263: The sdk-javascript-release job can be manually triggered
from any branch; add a guard so that when the event is workflow_dispatch it only
runs for main branch or tag refs. Update the sdk-javascript-release job (the
block with name "JavaScript SDK Release" and uses:
./.github/workflows/npm-release.yaml) to include an if condition that permits
execution when github.event_name != 'workflow_dispatch' OR (github.ref ==
'refs/heads/main' OR startsWith(github.ref, 'refs/tags/')), thereby preventing
beta releases from arbitrary branches.
In `@tools/migrate/generate-sqlc-testdata.sh`:
- Around line 53-56: The DB_URL construction embeds PG_PASSWORD into the command
line used by migrate, exposing secrets; update the script to avoid including
PG_PASSWORD in DB_URL and rely on the already-exported PGPASSWORD environment
variable (or build a URL without the password) before calling migrate -database
"${DB_URL}" so the password is not visible in process args; adjust references to
DB_URL and the migrate invocation (the lines that set DB_URL and call migrate
-path "${MIGRATIONS_DIR}" -database "${DB_URL}" goto "${VERSION}") accordingly
and ensure any downstream consumers still receive a valid connection string
without the embedded password.
---
Duplicate comments:
In @.github/workflows/release.yaml:
- Around line 236-248: Update the "Determine version and npm dist-tag" step (id:
meta) so prerelease tags don't get dist-tag=latest and non-tag beta versions are
unique per workflow run: when REF_TYPE=="tag" check GITHUB_REF_NAME for a
prerelease marker (e.g., contains '-') and only set dist-tag=latest for normal
semver tags, otherwise set dist-tag=beta; when not a tag, append a run-unique
value (use GITHUB_RUN_ID or GITHUB_RUN_NUMBER) to the generated beta version
(instead of only the short SHA) so reruns produce a new npm version; keep
references to GITHUB_REF_NAME and GITHUB_SHA usage in the same step.
---
Nitpick comments:
In `@AGENTS.md`:
- Around line 7-14: The AGENTS.md contains duplicated notes about using the
Makefile and justfile (the same guidance appears in the Quick Reference and
again later); edit AGENTS.md to keep the short Quick Reference entry that states
"Use the `Makefile` for all common tasks. A `justfile` also exists but is seldom
used." and remove or condense the later paragraph that repeats this guidance so
only one clear reference to `Makefile`/`justfile` remains; ensure the remaining
text is folded into the most relevant section and eliminate the second mention
to prevent drift.
In `@tools/migrate/generate-sqlc-testdata.sh`:
- Around line 48-93: Add a shell EXIT trap to always drop the scratch DB (using
SCRATCH_DB with psql) so failures or Ctrl-C won't leave sqlc_gen_<VERSION>
behind; implement the trap near the top of the script (after SCRATCH_DB is set)
and call psql -c "DROP DATABASE IF EXISTS ${SCRATCH_DB};" in the trap, then
remove the explicit final DROP block after sqlc generate; ensure the trap runs
regardless of where migrate, pg_dump, or sqlc generate fail and that any
necessary environment variables (PG_HOST, PG_PORT, PG_USER) are available to the
trap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7ec5df1c-a3d5-436d-9474-e7743b7531e8
⛔ Files ignored due to path filters (4)
.dagger/go.modis excluded by!**/*.mod.dagger/go.sumis excluded by!**/*.sum,!**/*.sumapi/client/javascript/Makefileis excluded by!api/client/**flake.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (25)
.dagger/.gitattributes.dagger/.gitignore.dagger/build.go.dagger/generate.go.dagger/main.go.dagger/migrate.go.dagger/release.go.dagger/utils.go.dagger/versions.go.dagger/versions_pinned.go.fossa.yaml.github/workflows/ci.yaml.github/workflows/npm-release.yaml.github/workflows/release.yaml.github/workflows/sdk-javascript-dev-release.yaml.golangci-fast.yaml.golangci.yaml.vscode/default.code-workspaceAGENTS.mdMakefiledagger.jsonflake.nixjustfileopenmeter/testutils/pg_driver.gotools/migrate/generate-sqlc-testdata.sh
💤 Files with no reviewable changes (17)
- .fossa.yaml
- .golangci-fast.yaml
- .vscode/default.code-workspace
- .dagger/.gitignore
- .golangci.yaml
- dagger.json
- .dagger/main.go
- .dagger/versions.go
- .dagger/generate.go
- .github/workflows/sdk-javascript-dev-release.yaml
- .dagger/versions_pinned.go
- .dagger/.gitattributes
- flake.nix
- .dagger/utils.go
- .dagger/migrate.go
- .dagger/build.go
- .dagger/release.go
✅ Files skipped from review due to trivial changes (2)
- openmeter/testutils/pg_driver.go
- .github/workflows/ci.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- justfile
- Makefile
Comment on lines
+250
to
+263
| sdk-javascript-release: | ||
| name: JavaScript SDK Release | ||
| # Runs on both tag pushes (stable/pre-release) and main pushes (per-commit beta). | ||
| # npm's trusted publisher entry is keyed on caller workflow file + environment, | ||
| # so this single caller must serve both channels. | ||
| needs: [sdk-javascript-meta] | ||
| uses: ./.github/workflows/npm-release.yaml | ||
| with: | ||
| version: ${{ needs.sdk-javascript-meta.outputs.version }} | ||
| dist-tag: ${{ needs.sdk-javascript-meta.outputs.dist-tag }} | ||
| permissions: | ||
| contents: read | ||
| id-token: write | ||
| secrets: inherit |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find .github/workflows -name "release.yaml" -o -name "release.yml"Repository: openmeterio/openmeter
Length of output: 95
🏁 Script executed:
cat -n .github/workflows/release.yamlRepository: openmeterio/openmeter
Length of output: 13443
Add a guard to prevent beta JS SDK releases from arbitrary branches.
The workflow_dispatch trigger lets anyone manually kick off this workflow from any branch, which would publish a beta release to npm for that branch. Since beta packages should only come from main or release tags, add the guard to keep things tidy.
Suggested fix
sdk-javascript-release:
name: JavaScript SDK Release
# Runs on both tag pushes (stable/pre-release) and main pushes (per-commit beta).
# npm's trusted publisher entry is keyed on caller workflow file + environment,
# so this single caller must serve both channels.
+ if: github.ref_type == 'tag' || github.ref_name == 'main'
needs: [sdk-javascript-meta]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yaml around lines 250 - 263, The
sdk-javascript-release job can be manually triggered from any branch; add a
guard so that when the event is workflow_dispatch it only runs for main branch
or tag refs. Update the sdk-javascript-release job (the block with name
"JavaScript SDK Release" and uses: ./.github/workflows/npm-release.yaml) to
include an if condition that permits execution when github.event_name !=
'workflow_dispatch' OR (github.ref == 'refs/heads/main' OR
startsWith(github.ref, 'refs/tags/')), thereby preventing beta releases from
arbitrary branches.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
.github/workflows/release.yaml (2)
243-255:⚠️ Potential issue | 🟠 MajorKeep prerelease tags off npm
latest.Since this workflow also triggers on
v*-beta.*andv*-dev.*, Line 250 still sends those prerelease tag builds to the npmlatestdist-tag. That can promote beta/dev SDKs to default installs.🏷️ Suggested fix
run: | if [[ "$REF_TYPE" == "tag" ]]; then echo "version=${GITHUB_REF_NAME}" >> "$GITHUB_OUTPUT" - echo "dist-tag=latest" >> "$GITHUB_OUTPUT" + case "$GITHUB_REF_NAME" in + *-beta.*) echo "dist-tag=beta" >> "$GITHUB_OUTPUT" ;; + *-dev.*) echo "dist-tag=dev" >> "$GITHUB_OUTPUT" ;; + *) echo "dist-tag=latest" >> "$GITHUB_OUTPUT" ;; + esac else short_sha="${GITHUB_SHA:0:12}" echo "version=1.0.0-beta-${short_sha}" >> "$GITHUB_OUTPUT"
257-263:⚠️ Potential issue | 🟠 MajorGuard JS SDK publishing to tags and
main.
workflow_dispatchcan still run this from a non-mainbranch, and this job would publish that branch SHA as a beta package. A small job-level guard keeps ad-hoc beta publishes scoped to the intended refs.🛡️ Suggested fix
sdk-javascript-release: name: JavaScript SDK Release # Runs on both tag pushes (stable/pre-release) and main pushes (per-commit beta). # npm's trusted publisher entry is keyed on caller workflow file + environment, # so this single caller must serve both channels. + if: github.ref_type == 'tag' || github.ref_name == 'main' needs: [ sdk-javascript-meta ] uses: ./.github/workflows/npm-release.yaml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yaml around lines 257 - 263, The sdk-javascript-release job can run via workflow_dispatch from non-main branches; add a job-level guard to only allow tag pushes or main branch runs by adding an if condition on the job (reference job name: sdk-javascript-release) such as checking startsWith(github.ref, 'refs/tags/') || github.ref == 'refs/heads/main' so ad-hoc dispatches from other branches won’t publish beta packages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yaml:
- Around line 273-274: The current tag-only guard (if: github.ref_type == 'tag')
still allows dev/beta tags through; update the condition to exclude dev and beta
tags by changing it to something like: if: github.ref_type == 'tag' &&
!contains(github.ref, '-dev.') && !contains(github.ref, '-beta.') so the stable
Python release job (the if in .github/workflows/release.yaml) will skip tags
with '-dev.' and '-beta.' in their names.
- Around line 220-230: The release step "Create GitHub release" currently runs
the gh release create command without marking dev/beta tags as prereleases;
update the logic around the gh release create invocation (the step named "Create
GitHub release" that calls gh release create) to include the --prerelease flag
when the tag name (GITHUB_REF_NAME) matches the dev/beta patterns (e.g.,
v*-dev.* or v*-beta.*), otherwise call gh release create as before; implement
this by conditionally adding --prerelease based on a shell test of
GITHUB_REF_NAME before running gh release create so dev/beta tags create
prereleases.
---
Duplicate comments:
In @.github/workflows/release.yaml:
- Around line 257-263: The sdk-javascript-release job can run via
workflow_dispatch from non-main branches; add a job-level guard to only allow
tag pushes or main branch runs by adding an if condition on the job (reference
job name: sdk-javascript-release) such as checking startsWith(github.ref,
'refs/tags/') || github.ref == 'refs/heads/main' so ad-hoc dispatches from other
branches won’t publish beta packages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33e88ebe-bdea-4f54-bb4d-a5ea2f6df453
📒 Files selected for processing (1)
.github/workflows/release.yaml
Comment on lines
+220
to
+230
| - name: Create GitHub release | ||
| if: github.ref_type == 'tag' | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| gh release create "${GITHUB_REF_NAME}" \ | ||
| --title "${GITHUB_REF_NAME}" \ | ||
| --generate-notes \ | ||
| --verify-tag \ | ||
| dist/benthos-collector_*.tar.gz \ | ||
| dist/checksums.txt |
Contributor
There was a problem hiding this comment.
Mark dev/beta GitHub releases as prereleases.
The workflow creates releases for v*-dev.* and v*-beta.* tags, but gh release create currently omits --prerelease, so those tags show up as normal releases.
🚦 Suggested fix
- name: Create GitHub release
if: github.ref_type == 'tag'
env:
GH_TOKEN: ${{ github.token }}
run: |
+ prerelease_args=()
+ if [[ "$GITHUB_REF_NAME" == *-dev.* || "$GITHUB_REF_NAME" == *-beta.* ]]; then
+ prerelease_args+=(--prerelease)
+ fi
+
gh release create "${GITHUB_REF_NAME}" \
--title "${GITHUB_REF_NAME}" \
--generate-notes \
--verify-tag \
+ "${prerelease_args[@]}" \
dist/benthos-collector_*.tar.gz \
dist/checksums.txt📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Create GitHub release | |
| if: github.ref_type == 'tag' | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| run: | | |
| gh release create "${GITHUB_REF_NAME}" \ | |
| --title "${GITHUB_REF_NAME}" \ | |
| --generate-notes \ | |
| --verify-tag \ | |
| dist/benthos-collector_*.tar.gz \ | |
| dist/checksums.txt | |
| - name: Create GitHub release | |
| if: github.ref_type == 'tag' | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| run: | | |
| prerelease_args=() | |
| if [[ "$GITHUB_REF_NAME" == *-dev.* || "$GITHUB_REF_NAME" == *-beta.* ]]; then | |
| prerelease_args+=(--prerelease) | |
| fi | |
| gh release create "${GITHUB_REF_NAME}" \ | |
| --title "${GITHUB_REF_NAME}" \ | |
| --generate-notes \ | |
| --verify-tag \ | |
| "${prerelease_args[@]}" \ | |
| dist/benthos-collector_*.tar.gz \ | |
| dist/checksums.txt |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yaml around lines 220 - 230, The release step
"Create GitHub release" currently runs the gh release create command without
marking dev/beta tags as prereleases; update the logic around the gh release
create invocation (the step named "Create GitHub release" that calls gh release
create) to include the --prerelease flag when the tag name (GITHUB_REF_NAME)
matches the dev/beta patterns (e.g., v*-dev.* or v*-beta.*), otherwise call gh
release create as before; implement this by conditionally adding --prerelease
based on a shell test of GITHUB_REF_NAME before running gh release create so
dev/beta tags create prereleases.
Comment on lines
+273
to
+274
| # Python SDK releases are tag-only (dev Python releases live in sdk-python-dev-release.yaml). | ||
| if: github.ref_type == 'tag' |
Contributor
There was a problem hiding this comment.
Skip Python dev tags in this release job.
The comment says dev Python releases live in sdk-python-dev-release.yaml, but this if still runs for v*-dev.* tags because the workflow trigger includes them. That can route dev publishes through the stable Python release path.
🐍 Suggested fix
name: Python SDK Release
# Python SDK releases are tag-only (dev Python releases live in sdk-python-dev-release.yaml).
- if: github.ref_type == 'tag'
+ if: github.ref_type == 'tag' && !contains(github.ref_name, '-dev.')If beta Python releases also have a separate path, exclude -beta. here too.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yaml around lines 273 - 274, The current tag-only
guard (if: github.ref_type == 'tag') still allows dev/beta tags through; update
the condition to exclude dev and beta tags by changing it to something like: if:
github.ref_type == 'tag' && !contains(github.ref, '-dev.') &&
!contains(github.ref, '-beta.') so the stable Python release job (the if in
.github/workflows/release.yaml) will skip tags with '-dev.' and '-beta.' in
their names.
turip
previously approved these changes
Apr 20, 2026
turip
force-pushed
the
chore/remove-dagger
branch
from
6ebcbea to
bb223a2
Compare
turip
approved these changes
Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit