Skip to content

ci(metadata): Add merge method labels and description-based auto-labeling - #2433

Closed
tintinhamans wants to merge 1 commit into
TheSuperHackers:mainfrom
tintinhamans:arctic/ci-titles-commits
Closed

ci(metadata): Add merge method labels and description-based auto-labeling#2433
tintinhamans wants to merge 1 commit into
TheSuperHackers:mainfrom
tintinhamans:arctic/ci-titles-commits

Conversation

@tintinhamans

@tintinhamans tintinhamans commented Mar 9, 2026

Copy link
Copy Markdown

This pull request updates the validate-pull-request.yml workflow to recognise labels "Squash" and "Rebase". It also detects and applies labels from the PR description.

  • Added labeled and unlabeled events workflow triggers.
  • Added steps to detect and apply "Squash" or "Rebase" labels based on PR description content, and to check for conflicting labels (workflow fails if both labels are present).
  • Skips commit validation if the "Squash" label is present

@greptile-apps

greptile-apps Bot commented Mar 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR enhances the validate-pull-request.yml workflow to support Squash and Rebase merge-method labels: it adds a description-scanning step that automatically applies the appropriate label from PR body keywords, a live label-detection step (via the GitHub API) that gates commit validation for Squash PRs, and a central result aggregation step that replaces the previous ad-hoc always() && … chain with structured infra/validation failure separation.

Key changes and observations:

  • New apply-merge-labels step: Uses an ERR trap + exit 0 to guarantee the step never fails hard; label-edit failures are surfaced via autolabel-warning=true output and propagated to the PR comment as an infra warning. This design choice is intentional and documented inline.
  • New labels step: Fetches labels via the GitHub API with an explicit fallback (squash=false/rebase=false/conflict=false + exit 1) on API failure, correctly propagating an infra failure through the rest of the pipeline.
  • result step: Centralises failure detection logic including unexpected step skips, infra failures, and validation errors; downstream steps (post-comment, delete-stale, fail) all key off result.outputs.failed and result.outcome, giving clean single-source-of-truth semantics.
  • Comment ordering corrected: The new code posts the failure comment first (post-comment step) and then deletes stale ones using IDs captured before posting, eliminating the "newly posted comment immediately deleted" race that existed in any implementation where the delete ran before the post.
  • Dead env var: LABELS_OUTCOME is declared in the post-comment step's env: block but is never referenced in the run script — minor cleanup opportunity (see inline comment).
  • Timeout reduction: The timeout was reduced from 3 to 2 minutes while the workflow gained two new API-calling steps (apply-merge-labels and labels). This is likely fine on ubuntu-slim but worth monitoring on busy queues.

Confidence Score: 3/5

  • The core logic is sound and well-defended, but the PR has accumulated many edge-case interactions (infra-failure propagation, cancellation windows, stale-comment cleanup races) that have been flagged and partly addressed across a long review thread — careful regression testing on the full event matrix is advisable before merging.
  • The structural design (ERR-trap guaranteeing exit 0 on apply-merge-labels, explicit API fallback on labels, result aggregation keyed off by all downstream steps) is solid and the ordering fix (post before delete) resolves a real prior bug. However, the number of outstanding edge-case concerns raised in previous threads — label-conflict infra message conflation, stale-comment deletion under skipped result, unexpected skip detection gaps, timeout reduction with added API calls — leaves meaningful residual risk. The one new finding (unused LABELS_OUTCOME env var) is minor.
  • .github/workflows/validate-pull-request.yml — specifically the result step (lines 315–389) and post-comment / delete-stale interaction (lines 391–487) deserve the most scrutiny given the complexity of their combined conditional logic.

Important Files Changed

Filename Overview
.github/workflows/validate-pull-request.yml Significant rework adding description-based auto-labeling, per-label merge method detection (Squash/Rebase), and a central result-aggregation step; overall logic is sound but contains a dead LABELS_OUTCOME env var in the post-comment step and has several edge-case interactions documented across previous review threads.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[PR Event\nopened/edited/synchronize/reopened/labeled/unlabeled] --> B[apply-merge-labels]
    B -->|labeled/unlabeled event| C[Skip – manual label event]
    B -->|synchronize + label already present| D[Skip – respect manual override]
    B -->|Description matches squash/rebase pattern| E[gh pr edit: add/swap label]
    B -->|No matching pattern| F[No label change]
    C & D & E & F --> G[Detect merge method labels\nvia GitHub API]
    G -->|API failure| H[squash=false, rebase=false, conflict=false\nexit 1 → INFRA_FAILED]
    G -->|Both Squash + Rebase| I[conflict=true\nexit 1 → validation failure]
    G -->|Squash only| J[squash=true]
    G -->|Rebase or no label| K[squash=false]
    J & K --> L[Checkout base branch]
    I & H --> L
    L --> M[Fetch PR head\nSkipped if squash=true or conflict]
    L --> N[Load valid tags]
    N --> O[Validate PR title\nAlways runs when tags loaded]
    M --> P[Validate PR commits\nSkipped if squash=true, conflict, or labels failed]
    O & P --> Q[Determine validation result\naggregates infra + validation failures]
    Q -->|failed=true| R[Post failure comment]
    Q -->|failed=false| S[Skip comment]
    R --> T[Delete stale bot comments\nusing OLD_COMMENT_IDS captured before posting]
    S --> T
    T --> U{failed?}
    U -->|true| V[exit 1 – job fails]
    U -->|false| W[Job passes]
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/validate-pull-request.yml
Line: 405

Comment:
**Unused `LABELS_OUTCOME` env var**

`LABELS_OUTCOME` is declared in the step's `env:` block but is never referenced anywhere inside the `run:` script (lines 408–463). The post-comment body is constructed using `IS_CONFLICT`, `IS_SQUASH`, `IS_REBASE`, `INFRA_FAILED`, and `AUTOLABEL_WARNING``LABELS_OUTCOME` contributes nothing. Removing it avoids confusion for future maintainers who might assume it drives some conditional logic.

```suggestion
          AUTOLABEL_WARNING: ${{ steps.apply-merge-labels.outputs.autolabel-warning }}
          VALID_TAGS: ${{ steps.load-tags.outputs.valid-tags }}
          IS_SQUASH: ${{ steps.labels.outputs.squash }}
          IS_REBASE: ${{ steps.labels.outputs.rebase }}
          IS_CONFLICT: ${{ steps.labels.outputs.conflict }}
          INFRA_FAILED: ${{ steps.result.outputs.infra_failed }}
          RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
```

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

Last reviewed commit: f12a758

@xezon

xezon commented Mar 10, 2026

Copy link
Copy Markdown

Can you also add a way to disable conventional commit tags check for the commits? For example by placing a keyword into the Pull description. Then only check it on the Pull Request title.

@Mauller

Mauller commented Mar 10, 2026

Copy link
Copy Markdown

Can you also add a way to disable conventional commit tags check for the commits? For example by placing a keyword into the Pull description. Then only check it on the Pull Request title.

Maybe check to see if the PR description contains "merge by rebase"

@tintinhamans
tintinhamans marked this pull request as draft March 10, 2026 21:41
@tintinhamans
tintinhamans force-pushed the arctic/ci-titles-commits branch from 3c87c60 to 6dad9e8 Compare March 11, 2026 22:29
@tintinhamans tintinhamans changed the title ci(metadata): Added extra newline to conventional commit warning comment ci(metadata): Add merge method labels and description-based auto-labeling Mar 11, 2026
@tintinhamans
tintinhamans force-pushed the arctic/ci-titles-commits branch from 6dad9e8 to 8dc6692 Compare March 11, 2026 22:34
@tintinhamans
tintinhamans marked this pull request as ready for review March 11, 2026 22:34
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml
Comment thread .github/workflows/validate-pull-request.yml Outdated
@tintinhamans
tintinhamans force-pushed the arctic/ci-titles-commits branch from 6205cfb to 9cf3b1c Compare March 13, 2026 15:18
@tintinhamans
tintinhamans requested review from bobtista and xezon March 13, 2026 15:20
Comment thread .github/workflows/validate-pull-request.yml Outdated
@tintinhamans
tintinhamans force-pushed the arctic/ci-titles-commits branch from 9cf3b1c to d939c61 Compare March 13, 2026 17:42

@xezon xezon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The described functionality looks good to me. Does it work?

@tintinhamans

Copy link
Copy Markdown
Author

The described functionality looks good to me. Does it work?

I have merged this to main on my fork https://github.com/tintinhamans/GeneralsGameCode, feel free to submit a test PR there to test.

@xezon

xezon commented Mar 14, 2026

Copy link
Copy Markdown

Can you test it? I trust you if you say it works.

Comment thread .github/workflows/validate-pull-request.yml Outdated
@tintinhamans
tintinhamans force-pushed the arctic/ci-titles-commits branch from f8612a6 to d3d8303 Compare March 15, 2026 10:51
Comment thread .github/workflows/validate-pull-request.yml Outdated
@tintinhamans
tintinhamans force-pushed the arctic/ci-titles-commits branch from d3d8303 to b0112a1 Compare March 15, 2026 11:01
Comment thread .github/workflows/validate-pull-request.yml Outdated
@tintinhamans
tintinhamans force-pushed the arctic/ci-titles-commits branch 2 times, most recently from 3cb6871 to 794b141 Compare March 15, 2026 22:27
Comment thread .github/workflows/validate-pull-request.yml
Comment thread .github/workflows/validate-pull-request.yml Outdated
@tintinhamans
tintinhamans force-pushed the arctic/ci-titles-commits branch 2 times, most recently from 05ad495 to 7203ddc Compare March 15, 2026 22:37
@Mauller

Mauller commented Mar 15, 2026

Copy link
Copy Markdown

Had to unsubscribe to this, it was blowing up my phone too much, munkee needs his sleep! lol

Comment thread .github/workflows/validate-pull-request.yml Outdated
@tintinhamans
tintinhamans force-pushed the arctic/ci-titles-commits branch from 7203ddc to 3c8d974 Compare March 15, 2026 22:47
Comment thread .github/workflows/validate-pull-request.yml Outdated
@tintinhamans
tintinhamans force-pushed the arctic/ci-titles-commits branch 3 times, most recently from 56c82dd to 49a7058 Compare March 15, 2026 23:12
Comment thread .github/workflows/validate-pull-request.yml Outdated
@tintinhamans
tintinhamans force-pushed the arctic/ci-titles-commits branch from 49a7058 to c44c9f5 Compare March 16, 2026 08:15
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml Outdated
Comment thread .github/workflows/validate-pull-request.yml
Comment thread .github/workflows/validate-pull-request.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants