[pull] main from activepieces:main

brain/knowledge/decisions/000015-fan-in-is-an-event-driven-waitpoint-barrier.md

@@ -14,7 +14,7 @@ producer's ordinal), nullable `label` (the human name in the summary) and a smal
 Release is an **unconditional floor rule**: a sealed barrier releases once no signal is still `PENDING`. No
 configuration can produce a hang; `policy` only ever releases *sooner* — `requiredSuccesses` (K-of-N
 approvals) and `releaseOnFirstFailure` (veto). Evaluation is a coalesced `BarrierJobName.EVALUATE` job on a
-dedicated `BARRIER_JOBS` queue, deduplicated on the barrier id, owned by the waitpoints module. Counts are
+dedicated `BARRIER_EVALUATION` queue, deduplicated on the barrier id, owned by the waitpoints module. Counts are
 taken once, from a single `GROUP BY status`, and handed to a pure predicate. Signals are deleted on release,
 in the same transaction that completes the waitpoint — summary first, then complete, then delete.
 
@@ -51,7 +51,7 @@ all, purely because child rows appear later than the dispatch that created them.
   already returns the ceiling, and a shorter deadline is a `policy` input, so both are known at create.
 - **Evaluation must be coalesced, and coalescing must not swallow its own last signal.** At 10 000 signals,
   evaluating per signal is ~100M row reads. BullMQ holds a deduplication key while the job is queued *and*
-  active, so the handler's **first statement** is `clearEvaluationDeduplication`, and every producer commits
+  active, so the handler's **first statement** is `clearEvaluationDedupKey`, and every producer commits
   its signal row **before** enqueuing. Otherwise the final signal lands while the job meant to see it is
   already running, its enqueue is dropped as a duplicate, and the barrier waits out its deadline holding a
   run that was ready to resume.
@@ -61,6 +61,31 @@ all, purely because child rows appear later than the dispatch that created them.
   `shouldReleaseBarrier({ policy, sealed, counts })` takes counts rather than issuing its own queries, so one
   `GROUP BY status` replaces up to three round-trips and the policy matrix is unit-testable without a
   database.
+- **Identity is path-keyed, and that needed no new column.** The engine sends `loop_1:3/loop_2:0/approval`
+  in the existing `stepName` field, which is only ever an identity key inside `waitpoint-service`. Which
+  fixes a live bug on the way past: a delay or approval **inside a loop** used to reuse one waitpoint row
+  across every iteration, so iteration 2 found iteration 1's COMPLETED row in `createForPause`'s
+  pre-completed check and skipped its pause entirely.
+- **Two per-run waitpoint reads stopped being sound and were fixed here.** `getByFlowRunId` became
+  `findCompletedWaitpointIfRunIsIdle` — **null when any PENDING row exists for the run**, else the newest COMPLETED
+  one — because per-iteration rows mean a run can hold a COMPLETED leftover *and* an open PENDING pause at
+  once, and both callers delete the row they find and enqueue a resume. `findNonFanInByFlowRunId` became
+  `findSubflowWaitpoint` and prefers PENDING, newest first. `findPendingByVersion` is **still** unsound
+  for parallel branches; nothing here creates that case.
+- **The resume guards moved to the entry point rather than becoming a flag.** `resumeFromWaitpoint`
+  **refuses barriers, always** — by addressed waitpoint type, or, on the by-run legacy routes, by "does this
+  run hold any PENDING barrier". Every unscoped route goes through it, so there is no `false` to forget and
+  no parameter to default wrong. What the guard is *not* is structural: `resumeTrusted` and
+  `resumeTrustedWithoutLock` are ordinary public members of `resumeService`, one `if` away from the guard
+  they skip. `flow-runs-queue`'s pre-completed recovery calls the unlocked variant from outside the module,
+  so a module-private `resumeTrusted` would not have worked as written. Treated as acceptable because the
+  reachable surface — the HTTP routes — is closed; if a third caller appears, make the boundary real rather
+  than adding a second convention.
+- **`release` is what happens to a barrier; `resume` is what happens to a run.** The two verbs sit one layer
+  apart — `barrierService.release` closes the barrier, then asks `resumeService` to resume the run — so a
+  `resumeService.releaseBarrier` named the wrong layer and hid that the same method also serves plain `DELAY`
+  and `WEBHOOK` waitpoints, which is exactly what `flow-runs-queue` calls it for. It is `resumeTrusted`:
+  resume, skipping the barrier guard, because the caller is internal rather than an HTTP route.
 - **An external actor never addresses the waitpoint.** Their link carries a **signal id** on
   `/v1/flow-runs/:id/signals/:signalId/confirm` — in the path, not the query string, because `resumePayload`
   is built from `{ body, headers, queryParams }` and persisted with the run. The link must never carry
@@ -76,7 +101,7 @@ all, purely because child rows appear later than the dispatch that created them.
 - **The machinery ships able to carry K-of-N approvals; no approval piece is rewired yet.** Per-signal links
   and `reasonRequiredOn` exist now because retrofitting them means another migration.
 - **`policy` carries no deadline override.** The plan allowed "or the policy's shorter value"; nothing
-  produces one, so `resolveDeadline()` is `now + AP_PAUSED_FLOW_TIMEOUT_DAYS` and takes no arguments. Add the
+  produces one, so `defaultBarrierDeadline()` is `now + AP_PAUSED_FLOW_TIMEOUT_DAYS` and takes no arguments. Add the
   field when a caller for it exists, not before.
 - **The 2 000-character reason bound stays a handler check, not a request schema.** The confirm route is
   `app.all` serving an HTML form: a Zod `body` schema would also apply to the GET that renders the page, and

brain/knowledge/decisions/000033-a-released-barrier-leaves-a-tombstone-until-the-run-ends.md

@@ -0,0 +1,57 @@
+---
+icon: 🪦
+status: accepted
+---
+
+# A released barrier leaves a tombstone until the run ends
+
+## Decision
+
+Delivering a resume no longer means deleting the waitpoint row. `waitpointService.consume` owns the
+rule: a `BARRIER` is tombstoned at a third status, `CONSUMED`, and every other type is still deleted.
+The tombstone lives until the run reaches a terminal status, where the existing `deleteByFlowRunId`
+sweep clears it. The legacy by-run resume guard (`hasBarrier`) therefore asks whether the run holds a
+barrier row **at all**, in any status, instead of whether one is `PENDING`.
+
+## Context
+
+A barrier's release boundary is predicate-owned: only its own predicate or its deadline may release
+it. The guard enforcing that on the deprecated V0 `/:id/requests/:requestId` routes asked
+`hasPendingBarrier`, which filters `status: PENDING`, so it stopped guarding the moment the barrier
+was released. Two windows followed — between `closeBarrier` committing `COMPLETED` and the trusted
+resume consuming it, and the much wider one after delivery, since a run stays `PAUSED` until the
+engine's next progress upload. In both, a legacy resume was accepted and enqueued a second job with
+a request-controlled payload, under a job id BullMQ would not collapse against the trusted one.
+
+The root cause was that "this resume was delivered" was encoded as *row-absence*, which makes a
+delivered barrier and a pre-waitpoint legacy pause indistinguishable — both are "no rows" — while
+the legacy path may only serve the second.
+
+## Why
+
+Making delivery explicit is what lets the two be told apart, and `waitpoint.status` is
+`character varying` with a TypeScript-only enum, so a third value costs no migration.
+
+Rejected: a `flow_run.pausedWaitpointId` column — a real migration, and `pauseMetadata` was not
+available to reuse (it is deprecated and pending removal). A Redis marker with a TTL — puts a
+security boundary on an expiry and a flush reopens the hole. Deleting the legacy path outright —
+tempting at 4.5 months past deprecation with a 30-day default pause timeout, but a self-hoster
+jumping many versions would silently lose every in-flight approval.
+
+Tombstoning is scoped to barriers rather than all waitpoints because the barrier is the only
+waitpoint whose release boundary is predicate-owned; for `DELAY` / `WEBHOOK` the unguessable link
+*is* the authorization. Widening it would leak `CONSUMED` rows into `findSubflowWaitpoint`'s
+any-status fallback and into `createForPause`'s unique `(flowRunId, stepName)` slot.
+
+## Consequences
+
+A run that has ever held a barrier can never use the legacy by-run resume path again. That is
+correct, not a side effect: that path serves only runs which never had a waitpoint row at all.
+
+`hasPendingBarrier` survives alongside `hasBarrier` and `findUndeliveredCompletedWaitpoint` must keep
+calling the *pending* one — switching it to existence would return null whenever a `CONSUMED` barrier
+exists and hang a later non-barrier delivery on the same run.
+
+The invariant that a row still sitting at `COMPLETED` was never delivered is preserved, now carried
+by an explicit status rather than by absence. Cost is one small row per barrier for the life of the
+run; `barrierService.create` already deletes any non-`PENDING` row occupying its step's slot.

brain/knowledge/decisions/000035-the-barrier-signal-cap-is-a-platform-configuration-row-not-an-env-var.md

@@ -0,0 +1,78 @@
+---
+status: accepted
+---
+
+# The barrier signal cap is a platform configuration row, not an env var
+
+## Decision
+
+`AP_MAX_BARRIER_SIGNALS` becomes `maxBarrierSignals`, an `integer NOT NULL DEFAULT 10000` column on
+`platform_configuration`, with a `Limits` section on the Configurations page to edit it. The row is
+authoritative and born from the env var, exactly as [000033](000033-platform-configuration-rows-are-authoritative-and-created-on-first-read.md)
+does it. The env var survives only as the birth default for a platform that has no row yet.
+
+Cloud is short-circuited **in the service**: `maxBarrierSignals({ platformId })` returns the system prop
+and ignores the row when `edition === CLOUD`, the same shape `isProductTelemetryEnabled` already uses.
+`platformId` is added to `CreateBarrierParams` so the caller supplies it rather than
+`barrierService.create` resolving it from `projectId`. There is no unlimited value and no nullable
+column.
+
+## Context
+
+The variable is new on `feat/barrier-service` and has never shipped: it is absent from `main`, from the
+published env var table, and from every deploy surface. So this is a choice about where a dial lives
+before release, with no upgrade path to protect, no breaking-change label and no docs migration.
+
+Its only read site is `assertSignalCountWithinLimit` in `waitpoints/barrier-service.ts`, and
+`barrierService.create` has no production caller yet, so the signature is free to change.
+
+## Why
+
+000033 draws a line that this decision deliberately walks up to: configurations hold behaviour,
+`platform_plan` holds entitlements, and a **cap** is the `MAX_RECORDS_PER_TABLE` shape that decision
+keeps off the page, because `POST /v1/platform-configurations` carries no edition guard and a limit
+living there is a self-serve quota bypass the day the page stops being Cloud-hidden.
+
+The cap goes there anyway because the bypass is closed where 000033 says invariants must be closed: in
+the server, on the value. Hiding the page was never the enforcement, and the Cloud short-circuit in the
+service makes the row unreadable as a cap on Cloud regardless of what an admin writes.
+
+**Rejected: `platform_plan`.** It is the correct home for a limit that is sold, and it is CE-readable
+via `platformService.getOneWithPlanOrThrow`. It was rejected because nobody sells barrier width, and a
+plan column would put a self-hoster's own protective cap behind an entitlement they cannot edit.
+
+**Rejected: env var as a `min(row, env)` ceiling.** It preserves an operator guarantee on a
+multi-platform EE install, where a platform admin is not the operator and could raise a cap that
+consumes shared Postgres and Redis. It was rejected as one concept too many for a case that does not
+yet exist, and because it reintroduces the two-sources-of-truth shape 000033 rejected for telemetry.
+This is the variant to revisit if multi-platform installs start mattering here.
+
+## Consequences
+
+- The error message must stop saying "raise `AP_MAX_BARRIER_SIGNALS`". It is read by a project member
+  who may not be a platform admin, so it names the limit and the page without implying the reader can
+  change it.
+- `platformId` on `CreateBarrierParams` means every future caller has to hold platform scope. The
+  waitpoints module already resolves it the other way in `resume-service.ts`
+  (`projectService.getPlatformId`), so the two paths in one module now differ on purpose: the check runs
+  before the transaction that inserts up to 10 000 signal rows, and the caller was cheaper to widen
+  while it has no production implementation.
+- **The number reaches the browser whether or not the builder uses it.** The GET response is the whole
+  `PlatformConfiguration` object, and `TelemetryProvider` wraps the router and already caches that row
+  for every signed-in user, so any builder component can read it with an existing hook and no new
+  endpoint. Nothing is built on that now: no fan-out step exists, and the real fan-out shape dispatches
+  over a runtime array, so a design-time count does not exist for the general case.
+- Because Cloud ignores the row, the raw column is **not** the effective cap. Whoever adds a builder
+  surface must serve a computed effective value rather than recompute the edition rule in the browser,
+  which is how 000033's split-brain bug happened.
+- **No `tryCatch` around the read, because every failure that could reach it is already fatal to
+  `barrierService.create`.** Postgres down kills the transaction and the signal inserts, Redis down kills
+  `enqueueEvaluation`, and the `distributedLock` is taken only on the create path, once per platform for
+  the life of the install. Fail-closed is also the do-nothing option, so there is no code to write.
+- **The `Limits` section is written but not rendered.** `limits-section.tsx` exists and is wired to the
+  same form schema, but `configurations/index.tsx` does not mount it, so no edition shows the field yet.
+  The row, the API and the cap are all live regardless: the value every platform runs on is the one
+  `AP_MAX_BARRIER_SIGNALS` seeded, and an existing platform's row can only be changed through
+  `POST /v1/platform-configurations` until the section is mounted. Rendering it is one import and one
+  line.
+- Postgres only. There is no sqlite migration for `platform_configuration`.