Skip to content

feat(ci): auto-approve and auto-merge weekly Crowdin translation PR (ENG-3672)#9072

Open
jianwei1 wants to merge 5 commits intomainfrom
jianweichong/eng-3672-auto-approve-and-auto-merge-weekly-crowdin-translation-pr
Open

feat(ci): auto-approve and auto-merge weekly Crowdin translation PR (ENG-3672)#9072
jianwei1 wants to merge 5 commits intomainfrom
jianweichong/eng-3672-auto-approve-and-auto-merge-weekly-crowdin-translation-pr

Conversation

@jianwei1
Copy link
Copy Markdown
Contributor

@jianwei1 jianwei1 commented Apr 23, 2026

Summary

  • Adds .github/workflows/crowdin-auto-merge.yml — a cron (Monday 09:00 UTC) + workflow_dispatch workflow that finds the open Crowdin translation PR on 00-00-CI-chore-i10n-updates, validates only libs/locales/ files changed, assigns + approves via a CI Bot App token, and enables auto-merge via the merge queue.
  • Adds docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md documenting problem frame, decisions, and open-question trail.

Context

The weekly "chore: new crowdin translations" PR has historically needed a human to assign + review before Danger will let it merge (PR #8888 was blocked 24+ days for this reason). Automating the approval + merge removes that toil.

Key safety guards:

  • Only merges PRs where every changed file is under libs/locales/.
  • Uses gh pr merge --auto --squash so the PR still goes through the merge queue and all required status checks run.
  • If no open Crowdin PR is found, the workflow exits gracefully.
  • Concurrency group prevents overlapping runs.

Open follow-ups (not blocking this PR)

  • Cron timing: considering Friday afternoon NZT (0 0 * * 5 UTC) instead of Monday, so a full week's dev-added strings have time to be translated before the merge. Happy to swap this before merging if you'd prefer.
  • Assignee: currently ${{ github.repository_owner }} (org). Crowdin's GitHub App integration authors these PRs as @tataihono — considering swapping to tataihono pending his agreement. Also open to leaving as-is.

Test plan

  • Workflow YAML lints
  • Manual workflow_dispatch trigger finds the open Crowdin PR and processes it end-to-end
  • Dry run verifies the file-path guard rejects a PR with non-locale changes
  • When no Crowdin PR is open, workflow exits cleanly with no error status

Closes ENG-3672

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores

    • Added scheduled GitHub Actions workflow that automatically processes translation pull requests, validating changes and enabling automatic merging when all requirements are satisfied.
  • Documentation

    • Added planning documentation for the translation workflow, covering trigger mechanisms, validation requirements, approval processes, and operational considerations.

jianwei1 and others added 3 commits April 13, 2026 02:15
…ation PRs

Weekly cron (Monday 9am UTC) + manual dispatch workflow that finds the
open Crowdin translation PR, validates only locale files are changed,
approves via CI Bot App token, and enables auto-merge via merge queue.

Closes ENG-3672

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…-auto-approve-and-auto-merge-weekly-crowdin-translation-pr
Captures the problem frame, key technical decisions (CI Bot App token
for approval, libs/locales/ path guard, merge-queue enqueue via
--auto --squash), and open-questions trail behind the workflow added
in the previous commit.

Closes ENG-3672
@linear
Copy link
Copy Markdown

linear Bot commented Apr 23, 2026

@jianwei1 jianwei1 requested a review from jaco-brink April 23, 2026 21:07
@jianwei1 jianwei1 self-assigned this Apr 23, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

Walkthrough

Adds a GitHub Actions workflow that automatically manages Crowdin translation pull requests by locating PRs with specific criteria, validating that all changes are within the libs/locales/ directory, approving them via a CI bot, rerunning failed checks, and enabling auto-merge with squash option. Also adds a planning document outlining the workflow requirements and test scenarios.

Changes

Cohort / File(s) Summary
Crowdin Auto-Merge Workflow
.github/workflows/crowdin-auto-merge.yml
New GitHub Actions workflow implementing automated Crowdin PR handling with PR discovery, file validation, CI bot approval, check rerun logic, and conditional auto-merge functionality.
Workflow Planning Documentation
docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md
Planning document detailing feature requirements, triggers, validation rules, test scenarios, and operational considerations for the Crowdin auto-merge workflow.

Sequence Diagram(s)

sequenceDiagram
    participant WF as GitHub Workflow
    participant GH as GitHub API
    participant CI as CI Bot
    participant MQ as Merge Queue

    WF->>GH: Find open PR on branch<br/>00-00-CI-chore-i10n-updates
    alt PR Found
        WF->>GH: Get PR diff filenames
        WF->>WF: Validate all files<br/>in libs/locales/
        alt Validation Passes
            WF->>GH: Assign PR to CI bot
            WF->>CI: Submit approval review<br/>with workflow run URL
            WF->>GH: Detect failed<br/>required checks
            alt Failed Checks Exist
                WF->>GH: Extract and rerun<br/>check runs
                GH->>WF: Checks complete
            end
            WF->>MQ: Enable auto-merge<br/>--squash
            MQ->>GH: Auto-merge PR
        else Validation Fails
            WF->>WF: Report error &<br/>skip merge
        end
    else No PR Found
        WF->>WF: Exit successfully<br/>with summary
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main feature: automating the approval and merge of weekly Crowdin translation PRs via CI workflow, which aligns with the changeset's primary objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jianweichong/eng-3672-auto-approve-and-auto-merge-weekly-crowdin-translation-pr

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Apr 23, 2026

View your CI Pipeline Execution ↗ for commit d6e4c80

Command Status Duration Result
nx affected --target=subgraph-check --base=8bf0... ✅ Succeeded <1s View ↗
nx affected --target=extract-translations --bas... ✅ Succeeded <1s View ↗
nx affected --target=lint --base=8bf0d701310e8a... ✅ Succeeded <1s View ↗
nx affected --target=type-check --base=8bf0d701... ✅ Succeeded <1s View ↗
nx run-many --target=codegen --all --parallel=3 ✅ Succeeded 2s View ↗
nx run-many --target=prisma-generate --all --pa... ✅ Succeeded 4s View ↗

☁️ Nx Cloud last updated this comment at 2026-04-24 00:23:35 UTC

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/crowdin-auto-merge.yml (1)

59-62: Exiting with exit 1 on an empty diff is questionable.

An open PR with zero changed files is anomalous but not necessarily an error condition — it could simply be a Crowdin PR where all upstream changes were already reverted. Failing the job produces a noisy red cron run every time this state persists. Consider logging a warning and exiting 0 instead, mirroring the "no open PR" branch above.

♻️ Suggested change
           if [ -z "$CHANGED_FILES" ]; then
-            echo "PR has no changed files. Exiting."
-            exit 1
+            echo "::warning::PR #$PR_NUMBER has no changed files; skipping auto-merge."
+            exit 0
           fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/crowdin-auto-merge.yml around lines 59 - 62, The script
currently treats an empty CHANGED_FILES result as a hard failure by calling exit
1; change this to treat it as a non-error by echoing a clear warning (e.g., "PR
has no changed files; nothing to merge.") and exiting with status 0 instead,
mirroring the behavior used for the "no open PR" branch; update the block that
checks the CHANGED_FILES variable (the if [ -z "$CHANGED_FILES" ] branch) to log
a warning and use exit 0 rather than exit 1.
🤖 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/crowdin-auto-merge.yml:
- Around line 6-8: The concurrency setting currently has cancel-in-progress:
true which can abort side-effecting runs (assign/approve/enable auto-merge)
mid-flight; change it to cancel-in-progress: false (or remove that key) so
overlapping runs are queued instead of cancelled, keeping the concurrency group
declaration (group: ${{ github.workflow }}) intact and ensuring the
workflow_dispatch and schedule triggers won't leave PRs partially processed.
- Around line 75-84: The CI step currently suppresses errors from the gh pr edit
command (gh pr edit --add-assignee "${{ github.repository_owner }}" || true)
which hides the fact that github.repository_owner resolves to an org and cannot
be assigned; remove the error suppression (drop the "|| true") so the job fails
loudly, or change the assignee target to a valid user (e.g. "tataihono") before
assigning; ensure the change prevents the silent failure detected by the danger
check (danger.github.pr.assignee === null in dangerfile.ts) so Danger's assignee
validation surfaces the problem instead of letting the workflow proceed.

---

Nitpick comments:
In @.github/workflows/crowdin-auto-merge.yml:
- Around line 59-62: The script currently treats an empty CHANGED_FILES result
as a hard failure by calling exit 1; change this to treat it as a non-error by
echoing a clear warning (e.g., "PR has no changed files; nothing to merge.") and
exiting with status 0 instead, mirroring the behavior used for the "no open PR"
branch; update the block that checks the CHANGED_FILES variable (the if [ -z
"$CHANGED_FILES" ] branch) to log a warning and use exit 0 rather than exit 1.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: dad0ec22-a371-49e2-962f-df8fb3f6743d

📥 Commits

Reviewing files that changed from the base of the PR and between 8bf0d70 and 9f5d484.

📒 Files selected for processing (2)
  • .github/workflows/crowdin-auto-merge.yml
  • docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md

Comment on lines +6 to +8
concurrency:
group: ${{ github.workflow }}
cancel-in-progress: true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Reconsider cancel-in-progress: true for this workflow.

This workflow performs side-effecting actions (assign → approve → enable auto-merge). If a manual workflow_dispatch run overlaps with the scheduled run — or vice versa — cancelling mid-step could leave the PR partially processed (e.g., assigned but not approved). A weekly cron with a quick job has very little reason to cancel; queuing is safer.

🛡️ Suggested change
 concurrency:
   group: ${{ github.workflow }}
-  cancel-in-progress: true
+  cancel-in-progress: false
📝 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
concurrency:
group: ${{ github.workflow }}
cancel-in-progress: true
concurrency:
group: ${{ github.workflow }}
cancel-in-progress: false
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/crowdin-auto-merge.yml around lines 6 - 8, The concurrency
setting currently has cancel-in-progress: true which can abort side-effecting
runs (assign/approve/enable auto-merge) mid-flight; change it to
cancel-in-progress: false (or remove that key) so overlapping runs are queued
instead of cancelled, keeping the concurrency group declaration (group: ${{
github.workflow }}) intact and ensuring the workflow_dispatch and schedule
triggers won't leave PRs partially processed.

Comment thread .github/workflows/crowdin-auto-merge.yml Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md (3)

98-98: Clarify the assignee in the implementation plan.

Line 98 describes assigning the PR but doesn't specify who should be assigned. While line 39 mentions the Crowdin config currently assigns to crowdin-bot, and the PR summary indicates the implementation will assign to the repository owner, this decision should be explicitly stated in the implementation approach section for clarity.

📝 Suggested clarification
-Assign the PR (using `gh pr edit --add-assignee`) to satisfy Danger's assignee check
+Assign the PR to the repository owner (using `gh pr edit --add-assignee <owner>`) to satisfy Danger's assignee check

Alternatively, if the decision is still pending per the PR summary's open follow-up, note that here explicitly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md` at line
98, Update the implementation approach text where it says "Assign the PR (using
`gh pr edit --add-assignee`)" to explicitly state who will be assigned: either
"crowdin-bot" (to match the Crowdin config) or the repository owner (per the PR
summary); if the decision is still pending, add a short note that assignment
choice is open and reference the follow-up decision required. Ensure this
clarification appears in the same implementation plan section that contains the
existing assignment instruction so readers can see the final assignee policy
alongside the `gh pr edit --add-assignee` step.

115-117: Consider adding a failure notification strategy.

The test scenarios cover various error paths (lines 115-117), but the plan doesn't specify how the team will be notified when the workflow fails (e.g., non-locale files detected, token generation fails). For a weekly cron job, relying solely on checking GitHub Actions logs may not be sufficient.

Consider adding a note about the notification approach, such as:

  • Leveraging GitHub's default workflow failure notifications
  • Adding a Slack notification step on failure
  • Creating a GitHub issue when validation fails
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md` around
lines 115 - 117, Add a concise "Failure notification strategy" section to the
plan (near the existing error-path/edge-case bullets) that describes how the
team will be notified when the workflow fails; include options to leverage
GitHub Actions' default failure emails, add a Slack notification step on
workflow failure, and/or open a GitHub issue automatically when validation
(e.g., non-locale files detected or CI Bot token generation in the error path)
fails, and note the desired primary channel and retry/escalation behavior for
the weekly cron job.

96-97: Consider documenting validation edge cases for the file-path safety guard.

The file-path validation is a critical security control (R2). Consider explicitly noting in the plan that the implementation should handle edge cases such as:

  • Path traversal attempts (e.g., libs/locales/../../sensitive-file)
  • Symlinks pointing outside the allowed directory
  • Normalized path comparison to prevent bypasses

While these details can be addressed during implementation, mentioning them in the plan ensures they're not overlooked.

📋 Suggested addition to the implementation approach

After line 97, add:

- Normalize and validate file paths to prevent traversal attacks or symlink bypasses
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md` around
lines 96 - 97, The plan's file-path safety guard (the bullet that starts
"Validate changed files using `gh pr diff` or `gh api` — check that every
changed file path starts with `libs/locales/`") lacks explicit edge-case
handling; update the plan to state that implementation must normalize and
validate file paths to prevent traversal attacks, detect and reject symlinks
that resolve outside the allowed directory, and perform a normalized path
comparison to avoid bypasses, adding a short bullet (e.g., "Normalize and
validate file paths to prevent traversal attacks or symlink bypasses")
immediately after the existing validation step so implementers know to include
these checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md`:
- Line 98: Update the implementation approach text where it says "Assign the PR
(using `gh pr edit --add-assignee`)" to explicitly state who will be assigned:
either "crowdin-bot" (to match the Crowdin config) or the repository owner (per
the PR summary); if the decision is still pending, add a short note that
assignment choice is open and reference the follow-up decision required. Ensure
this clarification appears in the same implementation plan section that contains
the existing assignment instruction so readers can see the final assignee policy
alongside the `gh pr edit --add-assignee` step.
- Around line 115-117: Add a concise "Failure notification strategy" section to
the plan (near the existing error-path/edge-case bullets) that describes how the
team will be notified when the workflow fails; include options to leverage
GitHub Actions' default failure emails, add a Slack notification step on
workflow failure, and/or open a GitHub issue automatically when validation
(e.g., non-locale files detected or CI Bot token generation in the error path)
fails, and note the desired primary channel and retry/escalation behavior for
the weekly cron job.
- Around line 96-97: The plan's file-path safety guard (the bullet that starts
"Validate changed files using `gh pr diff` or `gh api` — check that every
changed file path starts with `libs/locales/`") lacks explicit edge-case
handling; update the plan to state that implementation must normalize and
validate file paths to prevent traversal attacks, detect and reject symlinks
that resolve outside the allowed directory, and perform a normalized path
comparison to avoid bypasses, adding a short bullet (e.g., "Normalize and
validate file paths to prevent traversal attacks or symlink bypasses")
immediately after the existing validation step so implementers know to include
these checks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: cf2f7f4d-df94-4a66-b009-9473a9ad1e31

📥 Commits

Reviewing files that changed from the base of the PR and between 9f5d484 and be15129.

📒 Files selected for processing (1)
  • docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md

…auto-merge

- workflow_dispatch input `dry_run` (default true) gates only the merge-queue enqueue step; assign/approve/rerun still execute so the dry run validates them against the real PR
- Schedule commented out for now; will be re-enabled in a follow-up PR after dry-run validation
- Added `timeout-minutes: 10` and removed over-scoped `contents: write`
- Resolve CI Bot identity from `app-slug` action output (no API call)
- Tightened PR matching with jq guards: exact title, `isCrossRepository: false`, `headRepositoryOwner.login == "JesusFilm"`
- Title-mismatch case now fails hard instead of silently exiting (catches Crowdin config drift)
- New "Rerun failed required checks" step — fire-and-forget, runs in both dry and live modes, degrades gracefully on permission or API errors
- `set -euo pipefail` on every run block, with careful handling around pipes that legitimately exit non-zero
- `$GITHUB_STEP_SUMMARY` tables, `::group::` wrappers, and `::notice::` annotations for readable run pages
- Approval body now links back to the workflow run that submitted it
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
.github/workflows/crowdin-auto-merge.yml (1)

88-91: Minor: over-fetching commits from gh pr list.

--json commits returns the full commit list (with author, message, OIDs, etc.) but it's only used on line 124 as .commits | length for display. For a Crowdin PR with many commits, this is wasteful. Consider replacing with commits only where needed, or drop it from the summary — gh pr list does not expose a commit-count scalar directly, so the simplest fix is to remove the Commits row from the summary. Non-blocking.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/crowdin-auto-merge.yml around lines 88 - 91, The workflow
is over-fetching commit objects via the gh pr list --json commits call; update
the gh pr list invocation (the PR_JSON assignment) to drop commits from the
--json list and also remove or stop rendering the "Commits" summary row that
reads .commits | length (the summary rendering uses PR_JSON and the `.commits |
length` expression), so the PR_JSON fetch only requests
number,title,url,author,headRefName,isCrossRepository,headRepositoryOwner and
the summary no longer references .commits.
🤖 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/crowdin-auto-merge.yml:
- Around line 9-10: Enable and align the workflow schedule in the
crowdin-auto-merge workflow: uncomment and add a schedule cron that matches the
plan's Monday 09:00 UTC (`0 9 * * 1`) in the schedule: block (or, if you
intentionally want Friday 01:00 UTC, update the plan doc
`docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md` to that
cron instead so they match); if you intentionally keep the schedule commented
for a dry-run rollout, leave the `schedule:` block commented but add a TODO with
the follow-up ticket reference (ENG-3672) so this is not forgotten. Ensure
changes occur in the same `.github/workflows/crowdin-auto-merge.yml` schedule
block and update the PR description/doc accordingly.
- Around line 180-191: The CI job "Assign PR to CI Bot" uses BOT_LOGIN (set from
steps.bot-login.outputs.login) which yields an app bot login like
`${APP_SLUG}[bot]` that GitHub will not accept as an assignee; change the
assignment to use a real human user login (or a valid repo member) instead of
BOT_LOGIN—update the run-step that calls `gh pr edit "$PR_NUMBER" --add-assignee
"$BOT_LOGIN"` to pass a real username (e.g., `tataihono`) or lookup a valid
user, and ensure the change prevents dangerfile.ts's `pr.assignee === null`
check from blocking merges.
- Around line 215-224: The gh pr checks + jq pipeline is failing the whole
script under set -euo pipefail because gh exits non‑zero when checks are
failed/pending; change the FAILED_LINKS assignment to tolerate those expected
non‑zero exits by appending a safe fallback (e.g., wrap the pipeline with "||
true" or temporarily disable pipefail for this command). Update the line that
sets FAILED_LINKS (the `FAILED_LINKS=$(gh pr checks "$PR_NUMBER" --required
--json bucket,link | jq -r '.[] | select(.bucket == "fail") | .link')`
assignment) so the substitution never aborts the script (for example: `... | jq
-r '...' || true`) and leave the subsequent empty-check logic unchanged.

---

Nitpick comments:
In @.github/workflows/crowdin-auto-merge.yml:
- Around line 88-91: The workflow is over-fetching commit objects via the gh pr
list --json commits call; update the gh pr list invocation (the PR_JSON
assignment) to drop commits from the --json list and also remove or stop
rendering the "Commits" summary row that reads .commits | length (the summary
rendering uses PR_JSON and the `.commits | length` expression), so the PR_JSON
fetch only requests
number,title,url,author,headRefName,isCrossRepository,headRepositoryOwner and
the summary no longer references .commits.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 076ce269-c868-457c-870f-19a6aa2fbd10

📥 Commits

Reviewing files that changed from the base of the PR and between be15129 and d6e4c80.

📒 Files selected for processing (1)
  • .github/workflows/crowdin-auto-merge.yml

Comment on lines +9 to +10
# schedule:
# - cron: '0 1 * * 5' # Friday 01:00 UTC = Friday 1pm NZST / 2pm NZDT
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Schedule is commented out and doesn't match the plan.

Per the PR description and docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md (which specifies 0 9 * * 1 — Monday 9am UTC), this workflow is advertised as running weekly. In the current form:

  1. The schedule: block is commented out → the workflow only fires on workflow_dispatch. Once dry-run validation is complete, this must be re-enabled or ENG-3672 is not fully delivered.
  2. The commented cron itself (0 1 * * 5 — Friday 01:00 UTC) contradicts the plan (Monday 9am UTC) and the PR description. Pick one and align the plan doc + commit with the final choice (the PR objectives note Friday is being considered to give translators a full week — if so, update the plan doc too).

If leaving it disabled is intentional for initial rollout behind dry_run, please add a TODO referencing the follow-up ticket so this isn't silently forgotten.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/crowdin-auto-merge.yml around lines 9 - 10, Enable and
align the workflow schedule in the crowdin-auto-merge workflow: uncomment and
add a schedule cron that matches the plan's Monday 09:00 UTC (`0 9 * * 1`) in
the schedule: block (or, if you intentionally want Friday 01:00 UTC, update the
plan doc `docs/plans/2026-04-13-001-feat-crowdin-auto-merge-workflow-plan.md` to
that cron instead so they match); if you intentionally keep the schedule
commented for a dry-run rollout, leave the `schedule:` block commented but add a
TODO with the follow-up ticket reference (ENG-3672) so this is not forgotten.
Ensure changes occur in the same `.github/workflows/crowdin-auto-merge.yml`
schedule block and update the PR description/doc accordingly.

Comment on lines +180 to +191
- name: Assign PR to CI Bot
if: steps.find-pr.outputs.found == 'true'
env:
GH_TOKEN: ${{ steps.app-token.outputs.token }}
PR_NUMBER: ${{ steps.find-pr.outputs.number }}
BOT_LOGIN: ${{ steps.bot-login.outputs.login }}
run: |
set -euo pipefail
echo "Assigning PR #$PR_NUMBER to $BOT_LOGIN"
gh pr edit "$PR_NUMBER" --add-assignee "$BOT_LOGIN"
echo "Assigned successfully."
echo "| Assignee | \`$BOT_LOGIN\` |" >> "$GITHUB_STEP_SUMMARY"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how the assignee is set elsewhere in this repo and whether any workflow
# has successfully used a [bot] login as assignee.
rg -nP -C3 '(--add-assignee|addAssignees|"assignees"\s*:)' --type=yml --type=ts --type=js

Repository: JesusFilm/core

Length of output: 85


🌐 Web query:

Can a GitHub App installation (login ending in "[bot]") be assigned to a pull request via the GitHub Issues Assignees REST API, or does the API only accept human user logins?

💡 Result:

No, a GitHub App installation (login ending in "[bot]") cannot be assigned to a pull request via the GitHub Issues Assignees REST API. The API only accepts usernames of users who are eligible to be assigned, which are human users (or certain bot accounts like @copilot that appear as assignableUsers), but GitHub App [bot] accounts are not considered valid assignees. The key evidence is the "Check if a user can be assigned" endpoint GET /repos/{owner}/{repo}/assignees/{assignee}, which returns 204 if assignable or 404 otherwise. Multiple sources confirm this returns 404 for bot accounts like github-merge-queue[bot] or Copilot (in permission checks), as they are not regular GitHub users retrievable via users.getByUsername and not in the list of available assignees from GET /repos/{owner}/{repo}/assignees, which returns Simple User objects (type: "User"). The add assignees endpoint POST /repos/{owner}/{repo}/issues/{issue_number}/assignees accepts array of strings (usernames), but invalid ones are silently ignored if the caller lacks push access or the user is not assignable. Since App bots fail the check endpoint, they cannot be assigned via API. Official docs refer to "Usernames of people to assign", and assignees are Simple User objects. GitHub UI allows assigning certain bots like Copilot, but App installation bots are not supported for assignment via API. Issues like cli/cli#12614 show delays or issues assigning @copilot, but success implies special cases; general App bots fail.

Citations:


Switch from [bot] login to a real user for PR assignment.

BOT_LOGIN resolves to ${APP_SLUG}[bot] (e.g., my-app[bot]). The GitHub Issues Assignees API only accepts valid human user logins. App bot accounts are silently dropped by the API—it returns 200 but does not actually assign the bot. This causes Danger's pr.assignee === null check (dangerfile.ts line 51) to block the merge, repeating the same failure mode from the previous review.

Replace $BOT_LOGIN with a real user login (e.g., tataihono) or another assignable user from the repository.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/crowdin-auto-merge.yml around lines 180 - 191, The CI job
"Assign PR to CI Bot" uses BOT_LOGIN (set from steps.bot-login.outputs.login)
which yields an app bot login like `${APP_SLUG}[bot]` that GitHub will not
accept as an assignee; change the assignment to use a real human user login (or
a valid repo member) instead of BOT_LOGIN—update the run-step that calls `gh pr
edit "$PR_NUMBER" --add-assignee "$BOT_LOGIN"` to pass a real username (e.g.,
`tataihono`) or lookup a valid user, and ensure the change prevents
dangerfile.ts's `pr.assignee === null` check from blocking merges.

Comment on lines +215 to +224
# Fetch the failed required check links. Separate step so a real
# `gh pr checks` failure surfaces as a step failure, not as "no reruns."
FAILED_LINKS=$(gh pr checks "$PR_NUMBER" --required --json bucket,link \
| jq -r '.[] | select(.bucket == "fail") | .link')

if [ -z "$FAILED_LINKS" ]; then
echo "No failed required checks. Nothing to rerun."
echo "| Reruns | None needed |" >> "$GITHUB_STEP_SUMMARY"
exit 0
fi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

Does gh pr checks --jsonsuppress the non-zero exit codes (1 for failed, 8 for pending) thatgh pr checks normally returns?

💡 Result:

No, gh pr checks --json does not suppress the non-zero exit codes (1 for failed checks, 8 for pending checks) that gh pr checks normally returns. The exit codes are determined by the check statuses (failing or pending), independent of the output format. Official documentation lists "Additional exit codes: 8: Checks pending" under the command description, including options like --json. GitHub CLI commands using --json (added in PR #9079) follow the same exit code logic based on data aggregation in pkg/cmd/pr/checks/aggregate.go, printing JSON before exiting with the appropriate code if checks fail or pend.

Citations:


set -euo pipefail + gh pr checks will abort on the exact case it's meant to handle.

gh pr checks exits with status 1 when any required check has failed and 8 when checks are pending — and --json does not suppress these exit codes. Combined with set -euo pipefail and the pipe to jq, the command substitution propagates the non-zero status and the step fails before any rerun is attempted. The in-code comment assumes a failure here is "real," but a failing-required-check is indistinguishable from a genuine error at the exit-code level with the current invocation.

Tolerate the expected non-zero exits or drop pipefail for this single call:

🛡️ Proposed fix
-          set -euo pipefail
-
-          # Fetch the failed required check links. Separate step so a real
-          # `gh pr checks` failure surfaces as a step failure, not as "no reruns."
-          FAILED_LINKS=$(gh pr checks "$PR_NUMBER" --required --json bucket,link \
-            | jq -r '.[] | select(.bucket == "fail") | .link')
+          set -euo pipefail
+
+          # `gh pr checks` returns 1 when any check is failing and 8 when any is
+          # pending — both are expected states here, not script errors. Capture
+          # exit code explicitly and only treat unexpected codes as failures.
+          set +e
+          CHECKS_JSON=$(gh pr checks "$PR_NUMBER" --required --json bucket,link 2>&1)
+          RC=$?
+          set -e
+          if [ "$RC" -ne 0 ] && [ "$RC" -ne 1 ] && [ "$RC" -ne 8 ]; then
+            echo "::error::gh pr checks failed (exit $RC): $CHECKS_JSON"
+            exit 1
+          fi
+          FAILED_LINKS=$(echo "$CHECKS_JSON" | jq -r '.[] | select(.bucket == "fail") | .link')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/crowdin-auto-merge.yml around lines 215 - 224, The gh pr
checks + jq pipeline is failing the whole script under set -euo pipefail because
gh exits non‑zero when checks are failed/pending; change the FAILED_LINKS
assignment to tolerate those expected non‑zero exits by appending a safe
fallback (e.g., wrap the pipeline with "|| true" or temporarily disable pipefail
for this command). Update the line that sets FAILED_LINKS (the
`FAILED_LINKS=$(gh pr checks "$PR_NUMBER" --required --json bucket,link | jq -r
'.[] | select(.bucket == "fail") | .link')` assignment) so the substitution
never aborts the script (for example: `... | jq -r '...' || true`) and leave the
subsequent empty-check logic unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant