From 18ca40cbd0ce294f8449f8573f1059ae33fc7891 Mon Sep 17 00:00:00 2001 From: feruzm Date: Sat, 15 Aug 2026 12:32:51 +0000 Subject: [PATCH] Build packages before testing them, and report PR build on every PR Two changes, both prerequisites for making `build (24.x)` a required check on develop. It is not required today, which is how #1493 merged with six consecutive red builds behind it. Build order. apps/web resolves `@ecency/sdk` to the COMMITTED packages/sdk/dist, so running the tests first tests a stale build whenever a PR changes SDK source. #1493 spent six runs red on 36 tests whose source and mocks were both correct: `isAuthorMuted` existed in src, but not yet in the committed dist, and only landed there when the changeset commit rebuilt it. A required check that fails for a reason its author cannot act on is exactly what teaches people to merge past CI, so consumers now test what the PR actually builds. staging.yml and master.yml had the same ordering, and get the same move. Trigger. PR build opted in by branch prefix, which leaves a branch named anything else with no build at all, and would leave such a PR unable ever to satisfy the check once it is required: Dependabot, Renovate, forks and hotfix/* would be blocked with no way to produce it. Keying on the pull request closes both holes, and matches what lint.yml and typecheck.yml already do for the same reason. Making the check required is a repo setting and cannot ride along in a PR. It should be flipped once this merges. --- .github/workflows/PR-branch.yml | 31 ++++++++++++++++--------------- .github/workflows/master.yml | 6 ++++-- .github/workflows/staging.yml | 6 ++++-- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/.github/workflows/PR-branch.yml b/.github/workflows/PR-branch.yml index 3ab6a1742b..a13c4b0983 100644 --- a/.github/workflows/PR-branch.yml +++ b/.github/workflows/PR-branch.yml @@ -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: # Build/test only — no writes back to GitHub. # Superseded runs are cancelled: only the newest commit on the branch is worth building. concurrency: @@ -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 diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index 1e527b3087..846a3ede0d 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -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: diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 63aa9892cd..d3296246d1 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -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: