Conversation
📝 WalkthroughWalkthroughChanges add fork PR detection to CI workflows. The artifacts workflow now accepts an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
turip
added
the
release-note/ignore
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci.yaml (2)
536-549:⚠️ Potential issue | 🟠 MajorSame push event issue in e2e job.
The e2e job has the same conditional logic problem as quickstart. Push events will incorrectly trigger the local build step.
🛠️ Suggested fix
- name: Create override files for e2e env: DEPOT_IMAGE_URL: ${{ needs.artifacts.outputs.container-image-url-depot }} - if: github.event.pull_request.head.repo.full_name == github.repository + if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository # ... existing step content ... - name: Build as part of e2e - if: github.event.pull_request.head.repo.full_name != github.repository + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 536 - 549, The e2e job's conditional (e.g., the step using if: github.event.pull_request.head.repo.full_name == github.repository and the subsequent "Build as part of e2e" step using !=) lets push events hit the local-build branch; update the conditions to only run on pull_request events by adding an explicit check for the event type (for example combine github.event_name == 'pull_request' with the existing head.repo.full_name comparison or use github.event.pull_request != null) for both the override/write step and the "Build as part de e2e" step so push events no longer trigger the local build path.
425-448:⚠️ Potential issue | 🟠 MajorSame push event issue affects quickstart override selection.
For push events,
github.event.pull_request.head.repo.full_nameis empty, so:
- Line 425 condition (
== github.repository) →false- Line 448 condition (
!= github.repository) →trueThis means pushes to main would trigger the "Build as part of quickstart" step instead of using the depot image, which is probably not what you want.
🛠️ Suggested fix
- name: Create override files for quickstart - if: github.event.pull_request.head.repo.full_name == github.repository + if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository # ... existing step content ... - name: Build as part of quickstart - if: github.event.pull_request.head.repo.full_name != github.repository + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 425 - 448, The if-conditions that compare github.event.pull_request.head.repo.full_name to github.repository are wrong for push events (pull_request payload is absent), so update both conditionals to first ensure the event is a pull_request before comparing (e.g. add github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) for the branch that writes quickstart/docker-compose.override.yaml with DEPOT_IMAGE_URL, and invert that check (e.g. github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository) or fallback to treating non-pull_request events as same-repo as appropriate for the "Build as part of quickstart" step so pushes to main use the depot image path rather than the other branch.
🧹 Nitpick comments (1)
.github/workflows/ci.yaml (1)
464-466: Missing newline before next step.There's no blank line separating the heredoc-generating step from "Launch Docker Compose". Minor formatting nit, but the e2e job (line 559-561) does include the blank line for consistency.
✏️ Suggested fix
EOF + - name: Launch Docker Compose🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 464 - 466, The YAML step "Launch Docker Compose" is missing a blank line separating it from the preceding heredoc-generating step; add a single empty line before the "name: Launch Docker Compose" step in the .github/workflows/ci.yaml file so the steps are consistently separated (matching the e2e job formatting) to avoid formatting inconsistencies.
🤖 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/artifacts.yaml:
- Line 99: The workflow uses invalid GitHub Actions expression syntax by using
"not" for negation (e.g., the save entry currently written as "save: ${{ not
inputs.untrusted_source }}"); replace "not" with the "!" operator inside the
expression so it becomes "save: ${{ ! inputs.untrusted_source }}" and apply the
same fix to the other occurrences noted (the similar expressions at the other
save/condition locations referenced in the comment).
- Line 207: The "save" workflow input uses the wrong expression syntax for the
not operator; update the expression so the not operator wraps the operand in
parentheses (i.e., use not(inputs.untrusted_source)) so the "save" key evaluates
correctly; locate the "save" entry referencing inputs.untrusted_source and
replace the current expression with the corrected not(...) form.
- Line 111: Replace the invalid GitHub Actions conditional "if: not
inputs.untrusted_source" with a proper expression wrapper and operator; change
it to use the expression syntax and negation like: if: ${{ !
inputs.untrusted_source }} so the workflow evaluates the condition correctly
(update the conditional where "if: not inputs.untrusted_source" appears).
- Line 219: The workflow step uses raw YAML boolean syntax ("if: not
inputs.untrusted_source") which is invalid for GitHub Actions; update the step
so the if uses a GitHub Actions expression by wrapping the condition in the
expression delimiters and applying the unary not operator to
inputs.untrusted_source (i.e., convert the plain condition into a proper Actions
expression referencing inputs.untrusted_source). Ensure the line remains an if:
key and references inputs.untrusted_source so the runner evaluates it as an
expression.
- Around line 11-14: The new required workflow input "untrusted_source" causes
callers like release.yaml to fail; either make the input optional by changing
the artifacts.yaml input "untrusted_source" to include a default (e.g., default:
false) and remove required: true, or update every caller (e.g., release.yaml)
that uses the artifacts workflow to pass the input (add inputs:
untrusted_source: ${{ <appropriate expression> }} when calling the reusable
workflow); ensure you update all callers consistently so no workflow call is
left without the input.
In @.github/workflows/ci.yaml:
- Around line 373-374: The untrusted_source assignment incorrectly treats push
events as untrusted because github.event.pull_request is undefined; update the
expression for untrusted_source to only perform the pull-request comparison when
the event is a pull_request (use github.event_name or github.event_name ==
'pull_request') and otherwise mark pushes as trusted (false), referencing the
existing untrusted_source variable and the
github.event.pull_request.head.repo.full_name comparison so only PRs are
checked.
---
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 536-549: The e2e job's conditional (e.g., the step using if:
github.event.pull_request.head.repo.full_name == github.repository and the
subsequent "Build as part of e2e" step using !=) lets push events hit the
local-build branch; update the conditions to only run on pull_request events by
adding an explicit check for the event type (for example combine
github.event_name == 'pull_request' with the existing head.repo.full_name
comparison or use github.event.pull_request != null) for both the override/write
step and the "Build as part de e2e" step so push events no longer trigger the
local build path.
- Around line 425-448: The if-conditions that compare
github.event.pull_request.head.repo.full_name to github.repository are wrong for
push events (pull_request payload is absent), so update both conditionals to
first ensure the event is a pull_request before comparing (e.g. add
github.event_name == 'pull_request' &&
github.event.pull_request.head.repo.full_name == github.repository) for the
branch that writes quickstart/docker-compose.override.yaml with DEPOT_IMAGE_URL,
and invert that check (e.g. github.event_name == 'pull_request' &&
github.event.pull_request.head.repo.full_name != github.repository) or fallback
to treating non-pull_request events as same-repo as appropriate for the "Build
as part of quickstart" step so pushes to main use the depot image path rather
than the other branch.
---
Nitpick comments:
In @.github/workflows/ci.yaml:
- Around line 464-466: The YAML step "Launch Docker Compose" is missing a blank
line separating it from the preceding heredoc-generating step; add a single
empty line before the "name: Launch Docker Compose" step in the
.github/workflows/ci.yaml file so the steps are consistently separated (matching
the e2e job formatting) to avoid formatting inconsistencies.
🪄 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: 2a32a033-546a-46af-b862-2ea908088dfa
📒 Files selected for processing (2)
.github/workflows/artifacts.yaml.github/workflows/ci.yaml
| untrusted_source: | ||
| description: Whether the source is untrusted (e.g. running from a fork) | ||
| required: true | ||
| type: boolean |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all workflow files that call artifacts.yaml to verify they pass untrusted_source
rg -n 'uses:.*artifacts\.yaml' .github/workflows/ -A 10 | rg -v 'artifacts\.yaml' | head -50Repository: openmeterio/openmeter
Length of output: 1185
Update all workflow callers or make untrusted_source optional.
The release.yaml workflow calls artifacts.yaml but doesn't pass the new untrusted_source input. Since it's marked as required: true, this will break the release.yaml workflow. Either make it optional with a default value, or update release.yaml (and any other callers) to pass it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/artifacts.yaml around lines 11 - 14, The new required
workflow input "untrusted_source" causes callers like release.yaml to fail;
either make the input optional by changing the artifacts.yaml input
"untrusted_source" to include a default (e.g., default: false) and remove
required: true, or update every caller (e.g., release.yaml) that uses the
artifacts workflow to pass the input (add inputs: untrusted_source: ${{
<appropriate expression> }} when calling the reusable workflow); ensure you
update all callers consistently so no workflow call is left without the input.
| labels: ${{ steps.meta.outputs.labels }} | ||
| push: ${{ inputs.publish }} | ||
| save: true | ||
| save: ${{ not inputs.untrusted_source }} |
There was a problem hiding this comment.
Invalid expression syntax – use ! not not for negation.
GitHub Actions expressions use ! for boolean negation. The not keyword isn't valid here, which is why actionlint is reporting a parse error. Same issue appears at lines 111, 207, and 219.
🐛 Proposed fix
- save: ${{ not inputs.untrusted_source }}
+ save: ${{ ! inputs.untrusted_source }}📝 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.
| save: ${{ not inputs.untrusted_source }} | |
| save: ${{ ! inputs.untrusted_source }} |
🧰 Tools
🪛 actionlint (1.7.11)
[error] 99-99: parser did not reach end of input after parsing the expression. 3 remaining token(s) in the input: "IDENT", ".", "IDENT"
(expression)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/artifacts.yaml at line 99, The workflow uses invalid
GitHub Actions expression syntax by using "not" for negation (e.g., the save
entry currently written as "save: ${{ not inputs.untrusted_source }}"); replace
"not" with the "!" operator inside the expression so it becomes "save: ${{ !
inputs.untrusted_source }}" and apply the same fix to the other occurrences
noted (the similar expressions at the other save/condition locations referenced
in the comment).
|
|
||
| - name: Retrieve pull token | ||
| id: pull-token | ||
| if: not inputs.untrusted_source |
There was a problem hiding this comment.
Fix if condition syntax.
Two issues here: not should be !, and if: without ${{ }} wrapper for expressions with operators won't work as expected.
🐛 Proposed fix
- if: not inputs.untrusted_source
+ if: ${{ ! inputs.untrusted_source }}📝 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.
| if: not inputs.untrusted_source | |
| if: ${{ ! inputs.untrusted_source }} |
🧰 Tools
🪛 actionlint (1.7.11)
[error] 111-111: parser did not reach end of input after parsing the expression. 3 remaining token(s) in the input: "IDENT", ".", "IDENT"
(expression)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/artifacts.yaml at line 111, Replace the invalid GitHub
Actions conditional "if: not inputs.untrusted_source" with a proper expression
wrapper and operator; change it to use the expression syntax and negation like:
if: ${{ ! inputs.untrusted_source }} so the workflow evaluates the condition
correctly (update the conditional where "if: not inputs.untrusted_source"
appears).
| labels: ${{ steps.meta.outputs.labels }} | ||
| push: ${{ inputs.publish }} | ||
| save: true | ||
| save: ${{ not inputs.untrusted_source }} |
There was a problem hiding this comment.
Same syntax fix needed here.
🐛 Proposed fix
- save: ${{ not inputs.untrusted_source }}
+ save: ${{ ! inputs.untrusted_source }}📝 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.
| save: ${{ not inputs.untrusted_source }} | |
| save: ${{ ! inputs.untrusted_source }} |
🧰 Tools
🪛 actionlint (1.7.11)
[error] 207-207: parser did not reach end of input after parsing the expression. 3 remaining token(s) in the input: "IDENT", ".", "IDENT"
(expression)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/artifacts.yaml at line 207, The "save" workflow input uses
the wrong expression syntax for the not operator; update the expression so the
not operator wraps the operand in parentheses (i.e., use
not(inputs.untrusted_source)) so the "save" key evaluates correctly; locate the
"save" entry referencing inputs.untrusted_source and replace the current
expression with the corrected not(...) form.
|
|
||
| - name: Retrieve pull token | ||
| id: pull-token | ||
| if: not inputs.untrusted_source |
There was a problem hiding this comment.
Same syntax fix needed here.
🐛 Proposed fix
- if: not inputs.untrusted_source
+ if: ${{ ! inputs.untrusted_source }}📝 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.
| if: not inputs.untrusted_source | |
| if: ${{ ! inputs.untrusted_source }} |
🧰 Tools
🪛 actionlint (1.7.11)
[error] 219-219: parser did not reach end of input after parsing the expression. 3 remaining token(s) in the input: "IDENT", ".", "IDENT"
(expression)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/artifacts.yaml at line 219, The workflow step uses raw
YAML boolean syntax ("if: not inputs.untrusted_source") which is invalid for
GitHub Actions; update the step so the if uses a GitHub Actions expression by
wrapping the condition in the expression delimiters and applying the unary not
operator to inputs.untrusted_source (i.e., convert the plain condition into a
proper Actions expression referencing inputs.untrusted_source). Ensure the line
remains an if: key and references inputs.untrusted_source so the runner
evaluates it as an expression.
| # detect if the source is a fork | ||
| untrusted_source: ${{ github.event.pull_request.head.repo.full_name != github.repository }} |
There was a problem hiding this comment.
Push events will be incorrectly marked as untrusted.
On push events (like pushes to main), github.event.pull_request is undefined, so head.repo.full_name evaluates to empty. Comparing empty to github.repository results in true for !=, marking all pushes as untrusted sources.
This likely isn't intended – pushes to main should be trusted. Consider handling the push case explicitly:
🛠️ Suggested fix
# detect if the source is a fork
- untrusted_source: ${{ github.event.pull_request.head.repo.full_name != github.repository }}
+ untrusted_source: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository }}📝 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.
| # detect if the source is a fork | |
| untrusted_source: ${{ github.event.pull_request.head.repo.full_name != github.repository }} | |
| # detect if the source is a fork | |
| untrusted_source: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yaml around lines 373 - 374, The untrusted_source
assignment incorrectly treats push events as untrusted because
github.event.pull_request is undefined; update the expression for
untrusted_source to only perform the pull-request comparison when the event is a
pull_request (use github.event_name or github.event_name == 'pull_request') and
otherwise mark pushes as trusted (false), referencing the existing
untrusted_source variable and the github.event.pull_request.head.repo.full_name
comparison so only PRs are checked.
1 participant
Overview
Fixes #(issue)
Notes for reviewer
Summary by CodeRabbit
Release Notes