brain/engineering/ci-pr-review-hygiene.md
The CI gates that shape how a PR is reviewed, as opposed to whether it builds. Lives in .github/workflows/.
We open PRs as drafts so no human reviewer is auto-assigned until "Ready for review". Greptile's Review draft pull requests setting is enabled, so its first pass lands on draft open with no CI glue — first-pass AI review while it is still a draft, human review after. Unlike a once-per-PR CI nudge, the native setting also re-reviews as commits land on the draft.
pr-size.yml + tools/scripts/pr-size-check.ts count meaningful lines (additions + deletions, minus lockfiles, i18n/translation.json, locales/**, snapshots, dist) per area and fail when a gated area is over budget: engine+worker+execution combined 300, core/shared 250, server/api 600, packages/web 1200. packages/pieces and everything unmatched are measured but exempt — a line count can't tell a cohesive new piece from a codemod, and pieces are self-contained with low blast radius. Bypass with the large-pr-ok label or a revert: title. Budgets were calibrated from the distribution of recently merged PRs.
The diff comes from local git diff --numstat, not the /files API, so it is immune to GitHub's 3,000-file response cap — a mega-PR cannot under-count its way past the gate.
Which team gets asked to review comes entirely from .github/CODEOWNERS — there is no bot, no dependabot/renovate config, and no workflow that requests reviewers. @activepieces/core is the catch-all owner; @activepieces/pieces owns /packages/pieces/; @activepieces/platform owns the execution path (/packages/server/engine/, /packages/server/worker/, /packages/core/execution/). /bun.lock and /brain/ are listed with an empty owner column, which releases them from the catch-all — a PR touching only those needs no code-owner approval. Each team uses GitHub round-robin assignment, so one human per team per PR.
Enforcement is the Codeowners review repository ruleset (active on the default branch), not classic branch protection: require_code_owner_review: true plus required_approving_review_count: 1 and required_review_thread_resolution: true. Eight bypass actors are configured, which is why an owner-team request can look non-blocking on some PRs.
* in CODEOWNERS matches every file at every depth, so the catch-all owner is dragged into PRs that have nothing to do with them. Unlike docs/* (direct children only), * is fully recursive, and last-match-wins means only an explicit later rule can release a path. A lockfile-only PR requested core (#14629), and so did a single-page docs PR (#14422, one file under brain/). The release valve is a path listed with no owner after the * line, which GitHub reads as owned-by-nobody; CODEOWNERS has no !negation syntax and no brace expansion — packages/**/{A,B}.md parses clean and matches a file literally named {A,B}.md. Verify any edit with gh api repos/activepieces/activepieces/codeowners/errors — an invalid line is silently skipped, which quietly restores the catch-all owner instead of failing loudly.core request on a pieces PR is not always the lockfile — check for a second root file. #14558 looked like the lockfile case but its non-pieces files were bun.lock and tsconfig.base.json; the core request landed 6s after the commit that touched the tsconfig, not after the pieces push. Per-piece paths mappings generated into root tsconfig.base.json mean a pieces change can still reach a core-owned file, and no CODEOWNERS pattern can fix that — the file holds real compiler options and CODEOWNERS has no sub-file granularity.PR size is added as a required status check for main in branch protection. Until then it is visible but advisory.actions/checkout@v5, oven-sh/setup-bun@v2). The only SHA pins live in the CodeQL security workflow. Reviewers — human and AI — regularly suggest SHA-pinning a single new workflow; decline it. Moving to SHA pinning is a repo-wide policy call, and a half-pinned .github/ is worse than a consistent one.redis-memory-server compiles Redis from source during bun install, so its version must stay pinned. It is in trustedDependencies, and with no version configured it defaults to stable — whatever download.redis.io/redis-stable.tar.gz points at today. When that moved to Redis 8.10.0 (2026-07-29), the bundled module tree (redisearch, redistimeseries, LibMR) started failing to build on runners and took bun install down across every branch: 8.10.0 vendors the module sources into the tarball and changes the default make goal to build, which compiles every module under modules/*/src regardless of BUILD_WITH_MODULES. It reads as flakiness because ci.yml caches ~/.bun/install/cache but not the compiled binary, so each run recompiles and only sometimes survives. Root package.json pins redisMemoryServer.version to 8.8.1, the newest release that still builds core-only — treat it as a ceiling, bump it deliberately, and never go back to stable.tools/scripts/ is outside the lint and test wiring. ESLint ignores it, and npm run test-unit only covers engine/shared/web. A script there with real policy logic must run its own tests from its own workflow — pr-size.yml runs bun test tools/scripts/pr-size-check.test.ts as a step before the check itself.