Lines 52-57: Using ${{ github.actor }} as the Docker username is likely to cause authentication failures. Most container registries require a specific username (like a service account or token identifier), not the dynamic GitHub actor name.
Recommendation: Use a fixed service account username or the previous token identifier:

- name: Login to Forgejo Container Registry
  run: |
    echo "${{ secrets.FORGEJO_PAT_PACKAGES }}" | docker login forgejo.to-wer.de \
      -u token \
      --password-stdin

🟡 Important Issues

Branch Trigger Logic Gap

Line 4: Removing feature/** from push triggers but keeping the conditional !startsWith(github.ref_name, 'feature/') in build/push steps creates inconsistency.
Impact: Push events to feature/* branches will no longer trigger this workflow, potentially hiding the conditional's purpose.
Recommendation: Either restore feature/** to push triggers or remove the conditional checks that become unreachable.

🔵 Optional Improvements

Secret Naming Clarity

Line 55: FORGEJO_PAT_PACKAGES suggests a Personal Access Token for packages. Ensure this secret has the minimum required permissions (packages: write).
Consider adding a comment about the secret's purpose if not obvious.

✅ Positives

Using --password-stdin is more secure than passing passwords as arguments
Simplified login action reduces dependency on external actions
Consistent use of forgejo.to-wer.de registry URL

💡 Recommendations

Fix the username issue (🔴 Critical) - Use token or a dedicated service account username
Align branch triggers with conditionals (🟡 Important) - Either add feature/** back to push triggers or remove the dead conditional logic
Add error handling for the login step:

- name: Login to Forgejo Container Registry
  run: |
    echo "${{ secrets.FORGEJO_PAT_PACKAGES }}" | docker login forgejo.to-wer.de \
      -u token \
      --password-stdin || exit 1

Generated by [poolside/laguna-xs.2:free] via AI Code Review Action

## 🤖 AI Code Review ## Code Review: Docker Registry Login Method Update ### 🔴 Critical Issues **Authentication Username May Be Incorrect** - **Lines 52-57**: Using `${{ github.actor }}` as the Docker username is likely to cause authentication failures. Most container registries require a specific username (like a service account or token identifier), not the dynamic GitHub actor name. - **Recommendation**: Use a fixed service account username or the previous `token` identifier: ```yaml - name: Login to Forgejo Container Registry run: | echo "${{ secrets.FORGEJO_PAT_PACKAGES }}" | docker login forgejo.to-wer.de \ -u token \ --password-stdin ``` ### 🟡 Important Issues **Branch Trigger Logic Gap** - **Line 4**: Removing `feature/**` from push triggers but keeping the conditional `!startsWith(github.ref_name, 'feature/')` in build/push steps creates inconsistency. - **Impact**: Push events to `feature/*` branches will no longer trigger this workflow, potentially hiding the conditional's purpose. - **Recommendation**: Either restore `feature/**` to push triggers or remove the conditional checks that become unreachable. ### 🔵 Optional Improvements **Secret Naming Clarity** - **Line 55**: `FORGEJO_PAT_PACKAGES` suggests a Personal Access Token for packages. Ensure this secret has the minimum required permissions (packages: write). - Consider adding a comment about the secret's purpose if not obvious. --- ### ✅ Positives - Using `--password-stdin` is more secure than passing passwords as arguments - Simplified login action reduces dependency on external actions - Consistent use of `forgejo.to-wer.de` registry URL ### 💡 Recommendations 1. **Fix the username issue** (🔴 Critical) - Use `token` or a dedicated service account username 2. **Align branch triggers with conditionals** (🟡 Important) - Either add `feature/**` back to push triggers or remove the dead conditional logic 3. **Add error handling** for the login step: ```yaml - name: Login to Forgejo Container Registry run: | echo "${{ secrets.FORGEJO_PAT_PACKAGES }}" | docker login forgejo.to-wer.de \ -u token \ --password-stdin || exit 1 ``` --- *Generated by [poolside/laguna-xs.2:free] via AI Code Review Action*

AI Code Review / ai-review (pull_request) Successful in 23s

Details

CI / test (pull_request) Successful in 20s

Details

CI / docker (pull_request) Failing after 12s

Details

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feature/fix-ci:feature/fix-ci

git switch feature/fix-ci

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch dev

git merge --no-ff feature/fix-ci

git switch feature/fix-ci

git rebase dev

git switch dev

git merge --ff-only feature/fix-ci

git switch feature/fix-ci

git rebase dev

git switch dev

git merge --no-ff feature/fix-ci

git switch dev

git merge --squash feature/fix-ci

git switch dev

git merge --ff-only feature/fix-ci

git switch dev

git merge feature/fix-ci

git push origin dev

No reviewers

No labels

No milestone

No project

No assignees

3 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

twerner/coder-bot!10

No description provided.

Rows
Columns