Skip to content

Build packages before testing them, and report PR build on every PR #1500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
feruzm merged 1 commit into from
Aug 15, 2026
Merged

Build packages before testing them, and report PR build on every PR #1500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 16 additions & 15 deletions .github/workflows/PR-branch.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,12 @@
name: PR build
# Mirrors lint.yml and typecheck.yml: pull_request rather than a branch-filtered push,
# so this check reports on EVERY pull request. It used to opt in by branch prefix, which
# left two holes. A branch named anything outside the list was never built at all, and
# once this becomes a required check, such a PR could never satisfy it: Dependabot,
# Renovate, forks and hotfix/* would be blocked forever with no way to produce the check.
# Both holes close by keying on the pull request itself instead of the branch name.
on:
push:
branches:
# Every prefix in real use, not just the two most common. This list is an
# opt-IN, so a branch named anything else silently gets no build at all --
# which is how a change to next.config.js (a build-time file that neither
# tsc nor eslint evaluates) reached review with nothing having compiled it.
# Add new prefixes here, or they inherit that same silent gap.
- 'bugfix/*'
- 'feature/*'
- 'fix/*'
- 'chore/*'
- 'refactor/*'
- 'docs/*'
# Bot-authored PRs were previously merged without ever being built.
- 'seer/*'
pull_request:
Comment on lines 8 to +9

@qodo-code-review qodo-code-review Bot Aug 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Cross-pr build cancellations 🐞 Bug ☼ Reliability

After switching PR build to run on pull_request, the workflow-level concurrency group now keys
primarily on github.head_ref (source branch name), so different PRs with the same branch name
(common with forks/bots) can cancel each other’s runs. This can intermittently prevent a PR’s latest
commit from ever getting a completed PR build status, making a required check unreliable.
Agent Prompt 
## Issue description
`PR build` now triggers on `pull_request`, but its concurrency group uses `${{ github.head_ref || github.ref }}`. On PR events, `github.head_ref` is just the source branch name, which can collide across unrelated PRs (especially forks), causing runs to cancel each other.

## Issue Context
When `cancel-in-progress: true`, any workflow run in the same concurrency group cancels the in-flight run. If two PRs share the same `head_ref`, their runs can interfere.

## Fix Focus Areas
- .github/workflows/PR-branch.yml[12-14]

## Proposed change
Update the concurrency group to be PR-unique, e.g.:

```yaml
concurrency:
  group: ${{ github.workflow }}-pr-${{ github.event.pull_request.number }}
  cancel-in-progress: true
```

(If this workflow might later run on non-PR events, add a fallback using `github.ref`.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

# Build/test only — no writes back to GitHub.
# Superseded runs are cancelled: only the newest commit on the branch is worth building.
concurrency:
Expand Down Expand Up @@ -48,6 +40,15 @@ jobs:
run: pnpm install --frozen-lockfile
- name: Validate DMCA patterns
run: pnpm validate:dmca
# Ahead of the tests, not after them. apps/web resolves `@ecency/sdk` to the
# COMMITTED packages/sdk/dist, so a PR changing SDK source was tested against a
# stale build until some later commit happened to rebuild the dist. #1493 spent
# six runs red on 36 tests whose source and mocks were both correct, purely
# because `isAuthorMuted` existed in src but not yet in the committed dist. A
# required check that goes red for reasons the author cannot act on is what
# teaches everyone to merge past it, so the consumers test what this PR builds.
- name: Build packages
run: pnpm build:packages
- name: Run Tests
run: pnpm -r test
- name: Build
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/master.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,12 @@ jobs:
run: pnpm install --frozen-lockfile
env:
CI: true
- name: Run Tests
run: pnpm -r test
# Packages first: apps/web resolves `@ecency/sdk` to the COMMITTED
# packages/sdk/dist, so testing before this builds against a stale dist.
- name: Build packages
run: pnpm build:packages
- name: Run Tests
run: pnpm -r test
- name: Build web app
run: pnpm --filter @ecency/web build
env:
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/staging.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,12 @@ jobs:
run: pnpm install --frozen-lockfile
env:
CI: true
- name: Run Tests
run: pnpm -r test
# Packages first: apps/web resolves `@ecency/sdk` to the COMMITTED
# packages/sdk/dist, so testing before this builds against a stale dist.
- name: Build packages
run: pnpm build:packages
- name: Run Tests
run: pnpm -r test
- name: Build web app
run: pnpm --filter @ecency/web build
env:
Expand Down
Loading