Skip to content

ci: Adjust github workflows (actions)#21353

Merged
Jafaral merged 2 commits intoFRRouting:masterfrom
opensourcerouting:fix/github_workflows
Mar 26, 2026
Merged

ci: Adjust github workflows (actions)#21353
Jafaral merged 2 commits intoFRRouting:masterfrom
opensourcerouting:fix/github_workflows

Conversation

@ton31337
Copy link
Copy Markdown
Member

No description provided.

That causes frrbot to require an approval for running these actions.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Not used anymore.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Mar 25, 2026

Greptile Summary

This PR adjusts three GitHub Actions workflows: it removes commitlint.yml entirely, and strips labeled/unlabeled trigger events from both behind-base.yml and freeze.yml.

  • commitlint.yml deleted: The original workflow had a broken if condition — ${{ … }} && ${{ … }} placed the && operator outside the expression braces, causing it to always evaluate as truthy and run on every PR regardless of base branch. Removing the workflow is a reasonable cleanup; note that .github/commitlint.config.js remains in the tree but is no longer invoked by CI.
  • behind-base.yml: Dropping labeled/unlabeled is low-risk; this workflow only appends the rebase label and is not used as a merge gate.
  • freeze.yml: This change is problematic. The labeled trigger was the mechanism that re-ran the freeze_warning check when a maintainer applied a "freeze" or "do not merge" label to an already-open PR. Without it, if such a label is added after the last synchronize event, the workflow never re-runs, the old passing check result persists on GitHub's status API, and branch protection rules can be satisfied even though the freeze label is present — defeating the purpose of the workflow.

Confidence Score: 2/5

  • Not safe to merge as-is: the freeze.yml change silently breaks label-based freeze enforcement, allowing PRs to bypass a merge gate.
  • The freeze.yml change removes the labeled trigger, which is the only event that re-runs the freeze_warning check when a maintainer applies the label to a PR with an already-passing status. This creates a real bypass of the freeze mechanism. The other two changes (commitlint removal, behind-base.yml trigger trim) are reasonable and low-risk, but the freeze regression is significant enough to block the PR in its current form.
  • freeze.yml — the labeled/unlabeled trigger removal undermines the core purpose of the workflow.

Important Files Changed

Filename Overview
.github/workflows/freeze.yml Removes labeled/unlabeled triggers, breaking the enforcement of "freeze"/"do not merge" labels when they are applied to PRs that already have a passing status check from a prior sync event.
.github/workflows/behind-base.yml Removes labeled/unlabeled triggers; low risk since this workflow only adds the "rebase" label and is not a blocking gate.
.github/workflows/commitlint.yml Workflow entirely deleted. The original if condition (${{ … }} && ${{ … }}) was syntactically incorrect (the && operator was outside the expression braces), causing the check to run on all PRs regardless of base branch. Removal is understandable; .github/commitlint.config.js remains in the repo but is now unused by CI.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant GH as GitHub
    participant FreezeWF as freeze.yml workflow

    Note over Dev,FreezeWF: ✅ Covered: label present at last synchronize
    Dev->>GH: Push new commit (synchronize)
    GH->>FreezeWF: Trigger (synchronize)
    FreezeWF-->>GH: fail — "freeze" label detected

    Note over Dev,FreezeWF: ❌ Gap introduced by this PR: label added after last sync
    Dev->>GH: Push new commit (synchronize, no freeze label)
    GH->>FreezeWF: Trigger (synchronize)
    FreezeWF-->>GH: pass — no freeze label
    Note over Dev,GH: Time passes…
    Dev->>GH: Maintainer adds "freeze" label
    Note over GH,FreezeWF: labeled event removed → workflow NOT triggered
    GH-->>Dev: Branch protection shows ✅ (stale pass)
    Dev->>GH: PR can be merged despite freeze label
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/freeze.yml
Line: 5

Comment:
**Freeze label no longer re-triggers the check**

Removing `labeled` and `unlabeled` from the trigger list breaks the primary enforcement scenario for this workflow: a maintainer applies a "freeze" or "do not merge" label to a PR that hasn't had any new commits pushed since it last passed the `freeze_warning` check.

Without the `labeled` trigger, the workflow won't re-run when the label is added. The old **passing** status from the previous `synchronize` run remains on GitHub's check suite for that commit SHA. If `freeze_warning` is a required status check in branch protection rules, that stale green result is sufficient for GitHub to allow the merge — even though the PR now carries a "freeze" label.

The `behind-base.yml` change (also removing `labeled`/`unlabeled`) is much lower risk because that workflow only _adds_ a label and is not used as a blocking gate. But for `freeze.yml`, the `labeled` event is load-bearing: it is the only trigger that fires when a maintainer applies the freeze label to an already-open, already-passing PR.

```suggestion
    types: [synchronize, opened, reopened, labeled, unlabeled]
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "ci: Drop commitling Github actions" | Re-trigger Greptile

on:
pull_request_target:
types: [synchronize, opened, reopened, labeled, unlabeled]
types: [synchronize, opened, reopened]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Freeze label no longer re-triggers the check

Removing labeled and unlabeled from the trigger list breaks the primary enforcement scenario for this workflow: a maintainer applies a "freeze" or "do not merge" label to a PR that hasn't had any new commits pushed since it last passed the freeze_warning check.

Without the labeled trigger, the workflow won't re-run when the label is added. The old passing status from the previous synchronize run remains on GitHub's check suite for that commit SHA. If freeze_warning is a required status check in branch protection rules, that stale green result is sufficient for GitHub to allow the merge — even though the PR now carries a "freeze" label.

The behind-base.yml change (also removing labeled/unlabeled) is much lower risk because that workflow only adds a label and is not used as a blocking gate. But for freeze.yml, the labeled event is load-bearing: it is the only trigger that fires when a maintainer applies the freeze label to an already-open, already-passing PR.

Suggested change
types: [synchronize, opened, reopened]
types: [synchronize, opened, reopened, labeled, unlabeled]
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/freeze.yml
Line: 5

Comment:
**Freeze label no longer re-triggers the check**

Removing `labeled` and `unlabeled` from the trigger list breaks the primary enforcement scenario for this workflow: a maintainer applies a "freeze" or "do not merge" label to a PR that hasn't had any new commits pushed since it last passed the `freeze_warning` check.

Without the `labeled` trigger, the workflow won't re-run when the label is added. The old **passing** status from the previous `synchronize` run remains on GitHub's check suite for that commit SHA. If `freeze_warning` is a required status check in branch protection rules, that stale green result is sufficient for GitHub to allow the merge — even though the PR now carries a "freeze" label.

The `behind-base.yml` change (also removing `labeled`/`unlabeled`) is much lower risk because that workflow only _adds_ a label and is not used as a blocking gate. But for `freeze.yml`, the `labeled` event is load-bearing: it is the only trigger that fires when a maintainer applies the freeze label to an already-open, already-passing PR.

```suggestion
    types: [synchronize, opened, reopened, labeled, unlabeled]
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is intentional. We don't need to trigger these actions when frrbot labels/unlabels.

@Jafaral Jafaral merged commit 0bc6abd into FRRouting:master Mar 26, 2026
22 checks passed
@ton31337 ton31337 deleted the fix/github_workflows branch March 26, 2026 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants