Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/setup-environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: Setup Environment

on:
pull_request:
types: [opened, reopened, synchronize, edited, closed, labeled]
types: [opened, reopened, synchronize, edited, labeled]
workflow_dispatch:

concurrency:
Expand Down
6 changes: 3 additions & 3 deletions Dockerfile.worker
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ RUN npx turbo run build --filter=@activepieces/engine && \
esbuild packages/server/worker/src/bootstrap.ts \
--bundle --platform=node --format=cjs --target=node20 \
--tsconfig=tsconfig.base.json \
--external:isolated-vm --external:bufferutil --external:utf-8-validate \
--external:isolated-vm --external:bufferutil --external:utf-8-validate --external:esbuild \
--outfile=/out/worker.js

# Prebuilt isolated-vm (the engine child resolves it at runtime via NODE_PATH).
Expand All @@ -49,8 +49,8 @@ RUN apt-get update && \
apt-get install -y --no-install-recommends ca-certificates procps && \
rm -rf /var/lib/apt/lists/*

# esbuild compiles CODE steps; the code-builder spawns it by name from PATH.
RUN npm install -g --no-fund --no-audit esbuild@0.25.0 && npm cache clean --force
# esbuild (JS API) compiles CODE steps; at /node_modules so --external:esbuild worker.js resolves it, like isolated-vm.
RUN npm install --prefix / --no-fund --no-audit --no-save esbuild@0.25.0 && npm cache clean --force

WORKDIR /app

Expand Down
2 changes: 2 additions & 0 deletions brain/knowledge/engineering/ci-pr-review-hygiene.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,7 @@ Enforcement is the **`Codeowners review` repository ruleset** (active on the def
- **`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`.
- **Retargeting a stacked PR to `main` does not drop its base branch — it merges the whole thing.** A PR opened against a long-lived feature branch shows a small diff *relative to that base*, but `gh pr edit --base main` only moves the target; the branch still contains every commit of its old base. [#14593](https://github.com/activepieces/activepieces/pull/14593) read as 2 docs files against `feat/autumn-billing-integration` and as 198 commits / 211 files / +12k lines against `main`. Check with `git diff --stat origin/main...<branch>` **before** retargeting, and if it disagrees with the PR page, cherry-pick that PR's own commits onto `main` and force-push instead. A "conflict" on such a PR is often against the feature base only — those same commits can apply to `main` cleanly.
- **A decision authored on a long-lived branch will collide on its number.** `brain/decisions/` numbers are assigned once and never reused, but the next free number is only knowable against `main` — two branches in flight both grab it. #14593 carried a `000024` that `main` had since filled, and `000025` too, so it landed as `000026`. Renumber against `main` at merge time and update every referring link; nothing in CI catches a duplicate number or a dead decision link.
- **Preview environments resurrect on PR close because `setup-environment.yml` also triggers on `closed`.** Both workflows fire on the same close event; Remove Environment tears the env down correctly (compose down, nginx, repo), then Setup Environment sees the `preview` label (labels survive merge) and re-provisions the whole thing minutes later — verified on #14832: remove finished 11:20, setup rebuilt it by 11:30. This is why merged PRs kept live zombie environments on the preview box. Both workflows are thin SSH wrappers; the real setup/remove logic lives in `/root/environments` on the preview server (`secrets.PREVIEW_HOST`), not in this repo. Fixed by dropping `closed` from setup's trigger list.
- **The preview-server remove tool can't clean containers once the repo dir is gone.** Its `stop()` skips `docker compose down` when `repos/<subdomain>/docker-compose.yml` doesn't exist, so an env whose repo folder was deleted first leaves containers running forever — re-running `remove` is a no-op for them. Clean those manually via compose labels: `docker ps -aq --filter "label=com.docker.compose.project=<subdomain>"` (same filter works for `docker volume ls`). When auditing envs against PR state: read the real branch from the clone's HEAD (`git -C repos/<subdomain> symbolic-ref --short HEAD`) since subdomains flatten `/` to `-`; a clone sitting on `main` means the branch was deleted after merge; and an env with **no PR at all** is a manual `workflow_dispatch` preview — don't auto-delete those (bulk cleanup 2026-08-20 removed 27 closed-PR envs, reclaimed 32.5GB).
- **`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.
- **A branch that predates the `brain/` → `brain/knowledge/` move cannot edit a brain page in place — GitHub will call the PR conflicting even when `git merge` is clean locally.** Git follows the rename and merges the modification into the new path; GitHub's mergeability check does not, so it reports `modify/delete` on the old path and the PR goes `dirty`. Local `git merge-tree --write-tree` exits 0 and hides the problem; reproduce what GitHub sees with `git merge -X no-renames origin/main`. Fix: merge `origin/main` into the branch first, which lands the edit at the new path, then push.
1 change: 1 addition & 0 deletions brain/knowledge/engineering/server-module-anatomy.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ Verify with `npm run lint-dev` and `npm run test-api`.
- **`npm run check-migrations` can pass while your database is untouched.** It sources `.env.tests` (not `.env.dev`) and pipes `migration:run` to `/dev/null`, so it reported "No changes in database schema were found" against a database still holding the pre-migration table. Treat a green run as "the entity and *some* database agree", not as proof your migration executed. To actually verify, run `migration:run` against the dev DB with an explicit `AP_POSTGRES_HOST` and then inspect `information_schema.columns`, `pg_indexes` and `pg_constraint`.
- **Migration timestamps collide across unmerged branches.** `migrations` is keyed by class name, so two branches can both claim `1824000000000` and only conflict at merge. Before picking a timestamp, check the applied ledger (`select name from migrations order by id desc limit 5`) as well as the files on `main` — a timestamp can already be in use by a branch you cannot see.
- **PGlite has one connection, so `CONCURRENTLY` breaks it.** Guard on `system.get(AppSystemProp.DB_TYPE) === DatabaseType.PGLITE` and issue a plain `CREATE INDEX` on that branch. When you do use `CONCURRENTLY`, set `transaction = false` on the migration class — PostgreSQL requires it outside a transaction.
- **`EntitySchema` supports partial-index `where`, but not expression columns.** For a partial index on a bare column (e.g. `ON file(platformId) WHERE projectId IS NULL`), pass `where: '"projectId" IS NULL'` alongside `columns: ['platformId']` — TypeORM 0.3.x's `EntitySchemaIndexOptions.where` is honored by the Postgres driver (`PostgresQueryRunner` line 2442: `${where ? "WHERE " + where : ""}`), so `synchronize` can stay on and `migration:generate` tracks the index correctly. Reserve `synchronize: false` for **expression indexes** — `columns` is `string[]` of bare column names with no expression syntax, so an index like `ON file(type, (metadata->>'flowId'))` (see `idx_file_sample_data_flow_id`) genuinely can't be expressed and needs the opt-out. Blindly using `synchronize: false` for every hand-written index (which I did once and got called on) leaves TypeORM blind to the index — future `migration:generate` won't drop it if you remove it from the entity, and drift can silently accumulate.
- **`UpdateResult.affected` is `undefined` on PGlite — never branch on it.** TypeORM's Postgres driver sets `affected` from `raw.rowCount`, and `typeorm-pglite` returns PGlite's `Results` (`{ rows, fields, affectedRows }`) with no `rowCount`. So the compare-and-set idiom `if (result.affected === 0) return null` is *always false* on PGlite and every predicate in the `WHERE` becomes decorative — the guard silently passes. This is not test-only: `AP_DB_TYPE=PGLITE` is the documented one-line Docker install (`docs/install/options/docker.mdx`). It hit MCP OAuth (`mcpOAuthCodeService.consume`), where it made authorization codes replayable, unbound to their client and redirect_uri, and immune to expiry. Use `.returning('*')` and test `updateResult.raw` for emptiness instead — that works on both drivers. Confirmed against the pinned `@electric-sql/pglite` 0.3.14: a plain `UPDATE` answers `{ rows, fields, affectedRows }` with **`rowCount: undefined`**, while the same statement with `RETURNING *` fills `rows` correctly (0 on no match, 1 on match). Note PGlite *does* report `affectedRows` — it is only `rowCount`, the field TypeORM reads, that is missing, so "PGlite loses the count" is the wrong mental model. The remaining call sites were converted in 2026-08 (`ee/agent/agent-rpc-handlers.ts`, `ee/projects/platform-project-service.ts`); a `.affected` that only feeds a log line was left alone. **Integration tests here run on Postgres** (`.env.tests` points at a real server), so they cannot catch this class at all — run the suite with `AP_DB_TYPE=PGLITE` prefixed to exercise it, which works today and is how the fix was proven red-to-green. Prefer `.returning('id')` over `.returning('*')`: on a table like `agent_conversation` the star form hauls the whole `messages` jsonb back on every write, and a row only has to be counted, not read.
- **`breaking = true` is the rollback-safety flag, not the customer-facing one.** It marks destructive DDL (`DROP TABLE`/`DROP COLUMN`, `ADD ... NOT NULL` without a default) for `rollback-migrations.ts`. It does *not* by itself mean the PR needs the `⛓️‍💥 breaking-change` label — decide that from upgrade impact on self-hosters and API consumers.
- **A new `AppSystemProp` needs three edits, not one.** Add the enum entry in `system-props.ts`, a default in `systemPropDefaultValues` (`system.ts`), *and* a validator in `systemPropValidators` (`system-validator.ts`). Miss the validator and `validateEnvPropsOnStartup` throws `systemPropValidators[prop] is not a function` at boot — every API test fails on setup, not just the new one. Document the var in `docs/install/reference/environment-variables.mdx` too.
Expand Down
1 change: 1 addition & 0 deletions brain/knowledge/pieces-engine/piece-sets.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ A named, reusable piece/action/trigger visibility configuration a platform admin

### Gotchas
- EE/Cloud only, gated behind `platform.plan.managePiecesEnabled`. On CE / flag off, piece sets are inert and filtering falls back to legacy project-plan allow/block lists.
- The **whole** `/v1/piece-sets` module is behind that flag, `GET` included — so on a locked plan the web list query is `enabled: false`, the table is simply empty, and row actions never render. Only toolbar/entry points need a UI guard. The `LockedAlert` + `RequestTrial featureKey="ENTERPRISE_PIECES"` lives once on `PlatformPiecesPage`, above the tabs, since the same flag gates both the Pieces and Piece Sets tabs; the details route redirects back to the tab rather than hanging on a spinner waiting for a query that will never run.
- There is **no** install-time sync and no `onPieceCreated` hook — resolution is purely read-time. See ADR 0001 (visibility derived, not materialized).
- Embed auth: a v4 JWT carries a `pieceSet` key claim; legacy v2/v3 tokens carry `piecesTags` (only the first tag honored, resolved to `key = tag`, else Default). Enforcement (`applyProjectPieceAccess`) runs unconditionally, not gated by the flag.
- Migration is three ordered steps: create table + backfill (`1807...`), then `CREATE INDEX CONCURRENTLY` (`1808...`, non-transactional), then the breaking drop of legacy platform piece-filter columns (`1809...`). Legacy `tag`/`piece_tag` tables are kept only because the backfill reads them once via raw SQL.
Expand Down
Loading
Loading