small fixes and changes#3
Merged
Merged
Conversation
lukaskollmer
added
bug
|
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:
📝 WalkthroughWalkthroughUpdates coverage report merging to handle empty glob matches and adds a reusable macOS workflow that can run xcodebuild or fastlane with optional signing, CodeQL, Firebase Emulator, artifact upload, and cleanup steps. ChangesCoverage report parsing
Reusable xcodebuild workflow
Sequence Diagram(s)sequenceDiagram
participant workflow_call
participant build_and_test
participant xcodebuild
participant fastlane
workflow_call->>build_and_test: pass inputs and secrets
alt inputs.scheme is set
build_and_test->>xcodebuild: resolve dependencies and run build or tests
else inputs.fastlanelane is set
build_and_test->>fastlane: run lane
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
.github/workflows/xcodebuild-or-fastlane.yml (2)
429-429: 💤 Low valueConsider breaking long line for readability.
This line exceeds 150 characters. Consider breaking it into multiple lines for better readability.
📏 Suggested formatting
if [ -n "${{ inputs.firebaseemulatorimport }}" ]; then echo "Importing firebase emulator data from ${{ inputs.firebaseemulatorimport }}" - firebase emulators:exec -c ${{ inputs.firebasejsonpath }} --import=${{ inputs.firebaseemulatorimport }} 'fastlane ${{ inputs.fastlanelane }}' + firebase emulators:exec \ + -c ${{ inputs.firebasejsonpath }} \ + --import=${{ inputs.firebaseemulatorimport }} \ + 'fastlane ${{ inputs.fastlanelane }}' else🤖 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 @.github/workflows/xcodebuild-or-fastlane.yml at line 429, The firebase emulators:exec command at line 429 exceeds 150 characters and should be broken into multiple lines for readability. Break the long command into multiple lines by separating the firebase emulators:exec command, the -c parameter with firebasejsonpath, the --import parameter with firebaseemulatorimport, and the fastlane command portion into distinct lines using appropriate line continuation syntax for YAML shell commands, ensuring the command remains functionally equivalent.Source: Linters/SAST tools
452-455: ⚖️ Poor tradeoffClarify keychain cleanup condition logic.
The cleanup condition
(inputs.setupsigning && runner.environment != 'github-hosted') || failure()doesn't match the setup condition at line 284, which is simplyinputs.setupsigning.This means on github-hosted runners with signing enabled, signing materials are set up but only cleaned up on failure. While github-hosted runners are ephemeral (making this low-risk), the asymmetry is confusing and could be problematic if the runner type assumptions change.
Consider either:
- Always cleaning up when
inputs.setupsigningis true, OR- Adding a comment explaining why github-hosted success cases skip cleanup
🤖 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 @.github/workflows/xcodebuild-or-fastlane.yml around lines 452 - 455, The cleanup condition for the signing keychain deletion step is asymmetrical with the setup condition from line 284. The setup uses just `inputs.setupsigning`, but the cleanup uses `(inputs.setupsigning && runner.environment != 'github-hosted') || failure()`, which means on github-hosted runners with signing enabled, the signing materials are set up but only cleaned up on failure. Simplify the cleanup condition to match the setup condition by changing it to just `inputs.setupsigning` to ensure consistent behavior across all runner types, or alternatively, add a comment explaining why github-hosted success cases deliberately skip cleanup despite setting up signing materials..github/workflows/firebase-emulators-exec.yml (2)
149-154: 💤 Low valuePath construction could be more robust.
The path
${{ inputs.path }}/${{ inputs.artifact }}uses simple string concatenation. While this works for typical cases (e.g.,./myfile.txt), it may produce unexpected results if:
inputs.pathhas a trailing slash:.//myfile.txt(benign but inelegant)inputs.artifacthas a leading slash (unlikely but possible)GitHub Actions expressions don't offer built-in path joining, so this approach is pragmatic. However, consider documenting the expected format or adding an example in the input description.
Note: The static analysis tool flagged the unpinned
actions/upload-artifact@v7reference. This is consistent with the project's style throughout this file (all actions use version tags rather than commit SHAs), so it's a project-wide policy rather than a localized concern.🤖 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 @.github/workflows/firebase-emulators-exec.yml around lines 149 - 154, The path construction in the upload-artifact step uses simple string concatenation that could produce inelegant or problematic results if inputs.path has a trailing slash or inputs.artifact has a leading slash. While GitHub Actions lacks built-in path joining utilities, add documentation or examples to the input descriptions for inputs.path and inputs.artifact to clarify the expected format (e.g., inputs.path should not have a trailing slash, inputs.artifact should not have a leading slash) to help users provide correctly formatted values and prevent edge cases like .//myfile.txt.
70-75: ⚡ Quick winClarify the
artifactinput description and consider separating concerns.The description states "name and path" but
artifactis a single string used both as the artifact name (line 153) and concatenated into the upload path (line 154). This dual-purpose design may confuse callers:
- If a user provides
test-results.xml, it works cleanly (name:test-results.xml, path:./<file>)- If a user provides
build/output/results.xml, the artifact name becomesbuild/output/results.xml(slashes are flattened by GitHub Actions, but this is non-intuitive)Additionally, "at the end of the build" is inaccurate—this workflow executes commands, not builds.
Consider either:
- Clarifying the description to specify that
artifactshould be a relative path frominputs.pathand will also be used as the artifact name, or- Splitting into
artifact_nameandartifact_pathinputs for clearer semantics🤖 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 @.github/workflows/firebase-emulators-exec.yml around lines 70 - 75, Update the description for the `artifact` input parameter in the reusable workflow to clarify its dual-purpose usage. The current description states "name and path" but `artifact` is a single string that serves both as the artifact name (used at line 153) and is concatenated into the upload path (used at line 154). Revise the description to explicitly state that `artifact` should be a relative file path from inputs.path and will also be used as the artifact name, noting that directory separators will be flattened by GitHub Actions. Additionally, correct the inaccurate phrase "at the end of the build" since this workflow executes commands rather than performing a build. If appropriate, consider whether splitting the input into separate `artifact_name` and `artifact_path` parameters would provide clearer semantics for workflow callers.
🤖 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.
Inline comments:
In @.github/workflows/coverage.yml:
- Line 84: The SchmiedmayerLab/xccov2lcov action is referenced with a mutable
tag `@v1` which weakens supply-chain integrity in the CI workflow. Replace the `@v1`
tag with a full immutable commit SHA to pin the action to a specific version and
prevent unexpected changes from tag reassignment.
In @.github/workflows/xcodebuild-or-fastlane.yml:
- Line 225: Fix the typo in the description comment on line 225 of the
xcodebuild-or-fastlane.yml workflow file by replacing the misspelled word
"arguemnt" with the correct spelling "argument" in the phrase "Replace the file
found at googleserviceinfoplistpath if the argument is set."
- Line 264: Fix the typo in the deprecation warning message by changing "Plase"
to "Please" in the echo command that outputs the warning about the removal of
.derivedData folder caching.
- Line 63: There is a spelling error in the comment description for the workflow
input. In the line describing the iOS simulators flag, change the misspelled
word "shoud" to "should" so the text reads "Flag indicating if all iOS
simulators matching the `destination` input should be setup."
- Around line 242-247: Add the `persist-credentials: false` parameter to the
checkout action step to prevent the GitHub token from being persisted in the
workspace. This parameter should be added alongside the existing `token`,
`submodules`, and `lfs` parameters in the `with:` section of the
`actions/checkout@v4` action to enhance security by limiting credential exposure
in subsequent steps.
- Line 347: The scheme parameter in the xcrun xcodebuild command is not quoted,
which will cause issues if the scheme contains spaces or special characters. Add
double quotes around the ${{ inputs.scheme }} variable in the xcodebuild command
so that the parameter is properly escaped and can handle any characters safely.
- Around line 358-365: The scheme parameter in the xcodebuild command is not
quoted, which will cause failures if the scheme contains spaces or special
characters. Add double quotes around the ${{ inputs.scheme }} variable reference
in the -scheme parameter of the xcodebuild command to safely handle scheme names
with special characters.
- Around line 457-459: The cleanup condition at the Google credentials removal
step checks if inputs.fastlanelane is not empty, but the credentials are only
created when inputs.setupfirebaseemulator is true (as shown at line 423). Update
the if condition on the cleanup step to match the credential creation condition
by checking inputs.setupfirebaseemulator instead of inputs.fastlanelane, while
keeping the || failure() part to ensure cleanup happens even when the workflow
fails.
---
Nitpick comments:
In @.github/workflows/firebase-emulators-exec.yml:
- Around line 149-154: The path construction in the upload-artifact step uses
simple string concatenation that could produce inelegant or problematic results
if inputs.path has a trailing slash or inputs.artifact has a leading slash.
While GitHub Actions lacks built-in path joining utilities, add documentation or
examples to the input descriptions for inputs.path and inputs.artifact to
clarify the expected format (e.g., inputs.path should not have a trailing slash,
inputs.artifact should not have a leading slash) to help users provide correctly
formatted values and prevent edge cases like .//myfile.txt.
- Around line 70-75: Update the description for the `artifact` input parameter
in the reusable workflow to clarify its dual-purpose usage. The current
description states "name and path" but `artifact` is a single string that serves
both as the artifact name (used at line 153) and is concatenated into the upload
path (used at line 154). Revise the description to explicitly state that
`artifact` should be a relative file path from inputs.path and will also be used
as the artifact name, noting that directory separators will be flattened by
GitHub Actions. Additionally, correct the inaccurate phrase "at the end of the
build" since this workflow executes commands rather than performing a build. If
appropriate, consider whether splitting the input into separate `artifact_name`
and `artifact_path` parameters would provide clearer semantics for workflow
callers.
In @.github/workflows/xcodebuild-or-fastlane.yml:
- Line 429: The firebase emulators:exec command at line 429 exceeds 150
characters and should be broken into multiple lines for readability. Break the
long command into multiple lines by separating the firebase emulators:exec
command, the -c parameter with firebasejsonpath, the --import parameter with
firebaseemulatorimport, and the fastlane command portion into distinct lines
using appropriate line continuation syntax for YAML shell commands, ensuring the
command remains functionally equivalent.
- Around line 452-455: The cleanup condition for the signing keychain deletion
step is asymmetrical with the setup condition from line 284. The setup uses just
`inputs.setupsigning`, but the cleanup uses `(inputs.setupsigning &&
runner.environment != 'github-hosted') || failure()`, which means on
github-hosted runners with signing enabled, the signing materials are set up but
only cleaned up on failure. Simplify the cleanup condition to match the setup
condition by changing it to just `inputs.setupsigning` to ensure consistent
behavior across all runner types, or alternatively, add a comment explaining why
github-hosted success cases deliberately skip cleanup despite setting up signing
materials.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 06cd2cda-e2e4-4f65-b006-9f74f99a76a5
📒 Files selected for processing (4)
.github/workflows/coverage.yml.github/workflows/firebase-emulators-exec.yml.github/workflows/swiftlint.yml.github/workflows/xcodebuild-or-fastlane.yml
PSchmiedmayer
approved these changes
Jun 16, 2026
PSchmiedmayer
left a comment
PSchmiedmayer
left a comment
Contributor
There was a problem hiding this comment.
Thanks @lukaskollmer; happy to merge once the lint issues are resolved.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/coverage.yml (1)
63-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: mirror the xcresult zero-match guard in the lcov steps.
With
shopt -s nullglob, a non-emptyCOVERAGE_REPORTS_LCOVpattern that matches no files yields an emptyFILES, so the loop is skipped andlcov "${args[@]}" -o info.lcovruns with no-ainputs, failing with an opaque lcov error rather than the clear::error::message you just added for xcresult (Line 86). Consider emitting an explicit error here (and in the Line 98-106 step) for consistency and easier debugging of misconfigured globs.♻️ Suggested consistency tweak
shopt -s nullglob FILES=( $COVERAGE_REPORTS_LCOV ) + if [ "${`#FILES`[@]}" -eq 0 ]; then + echo "::error::No lcov reports matched pattern: $COVERAGE_REPORTS_LCOV" + exit 1 + fi args=()🤖 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 @.github/workflows/coverage.yml around lines 63 - 64, The lcov coverage steps currently rely on nullglob and can fall through to an opaque lcov failure when COVERAGE_REPORTS_LCOV matches no files. Update the lcov collection logic in the coverage workflow to mirror the xcresult guard by checking whether FILES is empty before building args or calling lcov, and emit an explicit ::error:: message with the same style used in the xcresult step. Apply this in both lcov steps so the behavior is consistent and easier to debug, using the existing FILES, args, and lcov invocation blocks as the reference points.
🤖 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.
Nitpick comments:
In @.github/workflows/coverage.yml:
- Around line 63-64: The lcov coverage steps currently rely on nullglob and can
fall through to an opaque lcov failure when COVERAGE_REPORTS_LCOV matches no
files. Update the lcov collection logic in the coverage workflow to mirror the
xcresult guard by checking whether FILES is empty before building args or
calling lcov, and emit an explicit ::error:: message with the same style used in
the xcresult step. Apply this in both lcov steps so the behavior is consistent
and easier to debug, using the existing FILES, args, and lcov invocation blocks
as the reference points.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2bffdbaf-bea2-4620-bb2e-02aa867d881d
📒 Files selected for processing (1)
.github/workflows/coverage.yml
…o lukas/fix-artifacts # Conflicts: # .github/workflows/coverage.yml # .github/workflows/firebase-emulators-exec.yml # .github/workflows/swiftlint.yml
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/coverage.yml (2)
98-107: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd an explicit empty-input guard for LCOV in the combined merge path.
With
nullglob, unmatchedCOVERAGE_REPORTS_LCOVbecomes an emptyFILESarray. This path can still succeed (lcov -a info.lcov ...) and silently omit Linux coverage, producing incomplete merged coverage.Suggested fix
- name: Merge xcresult and lcov if: ${{ inputs.coveragereports_lcov != '' && inputs.coveragereports != '' }} env: COVERAGE_REPORTS_LCOV: ${{ inputs.coveragereports_lcov }} run: | shopt -s nullglob FILES=( $COVERAGE_REPORTS_LCOV ) + if [ "${`#FILES`[@]}" -eq 0 ]; then + echo "::error::No lcov reports matched pattern: $COVERAGE_REPORTS_LCOV" + exit 1 + fi args=() for file in "${FILES[@]}"; do # each x.lcov artifact gets downloaded to `x.lcov/x.lcov` (ie, into a folder with the same name); # we need to take this into account when constructing the paths. args+=(-a "$file/$(basename "$file")") done🤖 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 @.github/workflows/coverage.yml around lines 98 - 107, The LCOV merge step in the coverage workflow needs an explicit empty-input guard because `shopt -s nullglob` can leave `FILES` empty and still let `lcov -a info.lcov ...` run without Linux coverage. Update the merge path around the `FILES`, `args`, and `lcov -a` block to detect when no LCOV artifacts were matched and fail fast or skip the merge with a clear error, so `coverage_combined.lcov` is never produced from incomplete inputs.
63-71: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMirror the xcresult no-input check in the lcov-only merge step.
If the glob matches nothing, this step falls through to
lcovwith no inputs and fails with a less clear error than the xcresult branch.Suggested fix
- name: Merge lcov coverage reports if: ${{ inputs.coveragereports_lcov != '' && inputs.coveragereports == '' }} env: COVERAGE_REPORTS_LCOV: ${{ inputs.coveragereports_lcov }} run: | shopt -s nullglob FILES=( $COVERAGE_REPORTS_LCOV ) + if [ "${`#FILES`[@]}" -eq 0 ]; then + echo "::error::No lcov reports matched pattern: $COVERAGE_REPORTS_LCOV" + exit 1 + fi args=() for file in "${FILES[@]}"; do # each x.lcov artifact gets downloaded to `x.lcov/x.lcov` (ie, into a folder with the same name); # we need to take this into account when constructing the paths. args+=(-a "$file/$(basename "$file")") done🤖 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 @.github/workflows/coverage.yml around lines 63 - 71, The lcov merge step should mirror the existing xcresult no-input guard so it does not invoke lcov with an empty input set. Update the workflow logic around the coverage merge loop and the lcov command to detect when the FILES array is empty after the nullglob expansion, and skip or short-circuit with a clear message before building args or calling lcov. Use the same pattern already used in the xcresult branch to keep the behavior consistent and avoid the less clear failure.
🤖 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.
Inline comments:
In @.github/workflows/firebase-emulators-exec.yml:
- Line 170: The workflow file is missing a trailing newline at the end, which is
causing YAML lint to fail. Update the end of the firebase-emulators-exec
workflow so it finishes with a newline after the final cleanup command, keeping
the change localized to the file’s trailing content.
- Around line 92-95: The checkout step is using an unpinned action version and
is leaving git credentials persisted after checkout. Update the actions/checkout
usage in the workflow to reference a fixed SHA instead of the version tag, and
add persist-credentials: false to the checkout configuration so the token is not
stored in .git/config.
- Around line 161-166: The Upload artifact step is using the mutable
actions/upload-artifact@v7 tag, which should be pinned to an immutable commit
SHA. Update the uses reference in the upload-artifact step to a specific SHA
instead of the v7 tag, keeping the rest of the step unchanged and ensuring the
workflow still refers to the same artifact name and path inputs.
---
Outside diff comments:
In @.github/workflows/coverage.yml:
- Around line 98-107: The LCOV merge step in the coverage workflow needs an
explicit empty-input guard because `shopt -s nullglob` can leave `FILES` empty
and still let `lcov -a info.lcov ...` run without Linux coverage. Update the
merge path around the `FILES`, `args`, and `lcov -a` block to detect when no
LCOV artifacts were matched and fail fast or skip the merge with a clear error,
so `coverage_combined.lcov` is never produced from incomplete inputs.
- Around line 63-71: The lcov merge step should mirror the existing xcresult
no-input guard so it does not invoke lcov with an empty input set. Update the
workflow logic around the coverage merge loop and the lcov command to detect
when the FILES array is empty after the nullglob expansion, and skip or
short-circuit with a clear message before building args or calling lcov. Use the
same pattern already used in the xcresult branch to keep the behavior consistent
and avoid the less clear failure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 141b4061-a60d-4958-a800-19f7fa0d1191
📒 Files selected for processing (2)
.github/workflows/coverage.yml.github/workflows/firebase-emulators-exec.yml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/coverage.yml (2)
98-107: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd an explicit empty-input guard for LCOV in the combined merge path.
With
nullglob, unmatchedCOVERAGE_REPORTS_LCOVbecomes an emptyFILESarray. This path can still succeed (lcov -a info.lcov ...) and silently omit Linux coverage, producing incomplete merged coverage.Suggested fix
- name: Merge xcresult and lcov if: ${{ inputs.coveragereports_lcov != '' && inputs.coveragereports != '' }} env: COVERAGE_REPORTS_LCOV: ${{ inputs.coveragereports_lcov }} run: | shopt -s nullglob FILES=( $COVERAGE_REPORTS_LCOV ) + if [ "${`#FILES`[@]}" -eq 0 ]; then + echo "::error::No lcov reports matched pattern: $COVERAGE_REPORTS_LCOV" + exit 1 + fi args=() for file in "${FILES[@]}"; do # each x.lcov artifact gets downloaded to `x.lcov/x.lcov` (ie, into a folder with the same name); # we need to take this into account when constructing the paths. args+=(-a "$file/$(basename "$file")") done🤖 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 @.github/workflows/coverage.yml around lines 98 - 107, The LCOV merge step in the coverage workflow needs an explicit empty-input guard because `shopt -s nullglob` can leave `FILES` empty and still let `lcov -a info.lcov ...` run without Linux coverage. Update the merge path around the `FILES`, `args`, and `lcov -a` block to detect when no LCOV artifacts were matched and fail fast or skip the merge with a clear error, so `coverage_combined.lcov` is never produced from incomplete inputs.
63-71: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMirror the xcresult no-input check in the lcov-only merge step.
If the glob matches nothing, this step falls through to
lcovwith no inputs and fails with a less clear error than the xcresult branch.Suggested fix
- name: Merge lcov coverage reports if: ${{ inputs.coveragereports_lcov != '' && inputs.coveragereports == '' }} env: COVERAGE_REPORTS_LCOV: ${{ inputs.coveragereports_lcov }} run: | shopt -s nullglob FILES=( $COVERAGE_REPORTS_LCOV ) + if [ "${`#FILES`[@]}" -eq 0 ]; then + echo "::error::No lcov reports matched pattern: $COVERAGE_REPORTS_LCOV" + exit 1 + fi args=() for file in "${FILES[@]}"; do # each x.lcov artifact gets downloaded to `x.lcov/x.lcov` (ie, into a folder with the same name); # we need to take this into account when constructing the paths. args+=(-a "$file/$(basename "$file")") done🤖 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 @.github/workflows/coverage.yml around lines 63 - 71, The lcov merge step should mirror the existing xcresult no-input guard so it does not invoke lcov with an empty input set. Update the workflow logic around the coverage merge loop and the lcov command to detect when the FILES array is empty after the nullglob expansion, and skip or short-circuit with a clear message before building args or calling lcov. Use the same pattern already used in the xcresult branch to keep the behavior consistent and avoid the less clear failure.
🤖 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.
Inline comments:
In @.github/workflows/firebase-emulators-exec.yml:
- Line 170: The workflow file is missing a trailing newline at the end, which is
causing YAML lint to fail. Update the end of the firebase-emulators-exec
workflow so it finishes with a newline after the final cleanup command, keeping
the change localized to the file’s trailing content.
- Around line 92-95: The checkout step is using an unpinned action version and
is leaving git credentials persisted after checkout. Update the actions/checkout
usage in the workflow to reference a fixed SHA instead of the version tag, and
add persist-credentials: false to the checkout configuration so the token is not
stored in .git/config.
- Around line 161-166: The Upload artifact step is using the mutable
actions/upload-artifact@v7 tag, which should be pinned to an immutable commit
SHA. Update the uses reference in the upload-artifact step to a specific SHA
instead of the v7 tag, keeping the rest of the step unchanged and ensuring the
workflow still refers to the same artifact name and path inputs.
---
Outside diff comments:
In @.github/workflows/coverage.yml:
- Around line 98-107: The LCOV merge step in the coverage workflow needs an
explicit empty-input guard because `shopt -s nullglob` can leave `FILES` empty
and still let `lcov -a info.lcov ...` run without Linux coverage. Update the
merge path around the `FILES`, `args`, and `lcov -a` block to detect when no
LCOV artifacts were matched and fail fast or skip the merge with a clear error,
so `coverage_combined.lcov` is never produced from incomplete inputs.
- Around line 63-71: The lcov merge step should mirror the existing xcresult
no-input guard so it does not invoke lcov with an empty input set. Update the
workflow logic around the coverage merge loop and the lcov command to detect
when the FILES array is empty after the nullglob expansion, and skip or
short-circuit with a clear message before building args or calling lcov. Use the
same pattern already used in the xcresult branch to keep the behavior consistent
and avoid the less clear failure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 141b4061-a60d-4958-a800-19f7fa0d1191
📒 Files selected for processing (2)
.github/workflows/coverage.yml.github/workflows/firebase-emulators-exec.yml
🛑 Comments failed to post (3)
.github/workflows/firebase-emulators-exec.yml (3)
92-95: 🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
sed -n '1,240p' .github/workflows/firebase-emulators-exec.ymlRepository: SchmiedmayerLab/.github
Length of output: 6708
🏁 Script executed:
sed -n '1,240p' .github/workflows/firebase-emulators-exec.ymlRepository: SchmiedmayerLab/.github
Length of output: 6708
Pin checkout and disable persisted git credentials.
actions/checkoutshould be pinned to a SHA, and this job doesn’t need git auth after checkout, so setpersist-credentials: falseto keep the token out of.git/config.🧰 Tools
🪛 zizmor (1.26.1)
[warning] 92-95: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 92-92: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/firebase-emulators-exec.yml around lines 92 - 95, The checkout step is using an unpinned action version and is leaving git credentials persisted after checkout. Update the actions/checkout usage in the workflow to reference a fixed SHA instead of the version tag, and add persist-credentials: false to the checkout configuration so the token is not stored in .git/config.Source: Linters/SAST tools
161-166: 🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== workflow excerpt ==\n' nl -ba .github/workflows/firebase-emulators-exec.yml | sed -n '145,175p' printf '\n== action references ==\n' rg -n "actions/upload-artifact|uses: .*`@v`[0-9]+|uses: .*@[0-9a-f]{7,}" .github/workflows .github -g '*.yml' -g '*.yaml' || true printf '\n== pinning policy search ==\n' rg -n "pin.*action|SHA|actions/upload-artifact|supply-chain|floating tag" .github README.md docs . || trueRepository: SchmiedmayerLab/.github
Length of output: 225
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== workflow excerpt ==\n' nl -ba .github/workflows/firebase-emulators-exec.yml | sed -n '145,175p' printf '\n== action references ==\n' rg -n "actions/upload-artifact|uses: .*`@v`[0-9]+|uses: .*@[0-9a-f]{7,}" .github/workflows .github -g '*.yml' -g '*.yaml' || true printf '\n== pinning policy search ==\n' rg -n "pin.*action|SHA|actions/upload-artifact|supply-chain|floating tag" .github README.md docs . || trueRepository: SchmiedmayerLab/.github
Length of output: 225
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== workflow excerpt ==\n' awk 'NR>=145 && NR<=175 { printf "%6d %s\n", NR, $0 }' .github/workflows/firebase-emulators-exec.yml printf '\n== action references ==\n' rg -n "actions/upload-artifact|uses: .*`@v`[0-9]+|uses: .*@[0-9a-f]{7,}" .github/workflows .github -g '*.yml' -g '*.yaml' || true printf '\n== pinning policy search ==\n' rg -n "pin.*action|SHA|supply-chain|floating tag|upload-artifact" . .github README.md docs -g '*.md' -g '*.yml' -g '*.yaml' || trueRepository: SchmiedmayerLab/.github
Length of output: 20288
Pin
actions/upload-artifactto a commit SHA.
actions/upload-artifact@v7is still a mutable tag, so it can change behavior over time. Use a SHA to avoid silent drift and reduce supply-chain risk.🧰 Tools
🪛 zizmor (1.26.1)
[error] 163-163: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/firebase-emulators-exec.yml around lines 161 - 166, The Upload artifact step is using the mutable actions/upload-artifact@v7 tag, which should be pinned to an immutable commit SHA. Update the uses reference in the upload-artifact step to a specific SHA instead of the v7 tag, keeping the rest of the step unchanged and ensuring the workflow still refers to the same artifact name and path inputs.Source: Linters/SAST tools
170-170: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the missing trailing newline.
YAML lint is already failing on the file ending without a newline.
🧰 Tools
🪛 GitHub Actions: Validate Repository / 4_Lint YAML.txt
[error] 170-170: Linting/formatting error: new-line-at-end-of-file. No newline character at the end of the file.
🪛 GitHub Check: Lint YAML
[failure] 170-170:
170:76 [new-line-at-end-of-file] no new line character at the end of file🤖 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 @.github/workflows/firebase-emulators-exec.yml at line 170, The workflow file is missing a trailing newline at the end, which is causing YAML lint to fail. Update the end of the firebase-emulators-exec workflow so it finishes with a newline after the final cleanup command, keeping the change localized to the file’s trailing content.Source: Linters/SAST tools
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
2 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.
small fixes and changes
⚙️ Release Notes
xcodebuild-or-fastlaneworkflow as an alternative to the new firebase-emulators-exec workflow (which currently doesn't have artifact uploading support)📚 Documentation
n/a
✅ Testing
n/a
Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:
Summary by CodeRabbit