[pull] main from activepieces:main

brain/knowledge/decisions/000030-entitlement-flags-are-resolved-from-the-customers-plans-never-from-customer-flags.md

@@ -0,0 +1,80 @@
+---
+status: proposed
+---
+
+# Entitlement flags are resolved from the customer's plans, never from `customer.flags`
+
+## Context
+
+`refreshEntitlements` projects the Autumn customer into `platform_plan`. The obvious source for the
+boolean feature flags is `customer.flags` — Autumn hands it over already flattened, keyed by feature
+id, on the same response the balances come from.
+
+That map cannot carry the meaning we need. It is keyed by feature id, so when two attached plans grant
+the same feature Autumn collapses them into one entry and reports `planId: null` — the identical shape
+it uses for a standalone customer-level grant. And two plans routinely *are* attached: `free` is
+`auto_enable`, and attaching a **purchase**-shaped plan (`appsumo`, `free_legacy`) does not replace it
+the way a subscription does. So for those customers the flag set is the union of `free` and their real
+plan, with every shared flag reporting no source plan.
+
+`toAutumnEntitlements` had already grown a special case around this — `showPoweredBy` was read as
+`!isNil(flag.planId)` — which inverted itself under the merge and handed every AppSumo and
+Free-Legacy platform free white-labelling.
+
+## Decision
+
+Resolve the flags from the plans themselves. Build an **entitlement plan set** — all active
+subscriptions (add-ons included) plus every purchase that has not expired — and union the boolean
+items on those plans.
+
+`free`, `free_legacy` and `appsumo` are **baseline** plans. They supply the flags when nothing else is
+attached, and all three are dropped from the union as soon as a non-baseline, non-add-on plan appears,
+so an AppSumo platform that later buys `plus` gets `plus`'s flags with no baseline leftovers. Add-ons
+are never dropped and never trigger the drop, so a credit top-up cannot strip a free platform's flags.
+No customer can reach zero flags.
+
+`customer.flags` is not read for the projection at all, and `billingEnforced` comes from the same set
+rather than a second source.
+
+Balances stay on `customer.balances`: that is the real balance, and the only place a one-off top-up
+grant appears.
+
+`planId` and `scheduledUsersLimit` keep using base subscriptions only. An add-on is neither the
+platform's plan name nor a seat schedule.
+
+## Why
+
+The plan set is the same thing `planId` already resolves to, so the projected flags and the projected
+plan name can no longer disagree — which is what the `showPoweredBy` special case was failing to
+paper over. It also removes `flag.planId` from the design entirely, and that field is unusable by
+construction: it cannot distinguish "granted by two plans" from "granted outside any plan".
+
+Subtracting `free`'s flags from `customer.flags` instead was rejected: it needs a catalog read to know
+what `free` grants anyway, and it leaves the merged-`planId` trap in place for the next reader.
+Fixing the Autumn catalog instead — dropping `auto_enable` from `free`, or making `appsumo` a
+subscription — was rejected because it mutates live billing data and the next purchase-shaped plan
+reintroduces the union.
+
+## Consequences
+
+AppSumo and Free-Legacy platforms get `showPoweredBy` back, so the "Powered by Activepieces" badge
+returns for them. Nothing needs a migration: `platform_plan` rows self-heal on the next
+`refreshEntitlements`.
+
+A flag granted directly onto a customer outside any plan is now invisible. That is accepted —
+entitlements come from plans.
+
+**Every `getCustomer` call that feeds an entitlement decision must pass
+`expand: ['subscriptions.plan', 'purchases.plan']`.** Without it the plans come back with no `items`,
+every flag resolves absent, and `billingEnforced` in particular fails *open* — credit gating silently
+stops for every platform. Nothing in the type system catches this, because `plan` is optional on both
+subscriptions and purchases. Two defences: `writeCustomerStateCaches` takes the resolved
+`grantedFeatureIds` as an explicit parameter rather than deriving it privately, so a new call site has
+to confront the requirement; and `toGrantedFeatureIds` logs a warning when a customer has attachments
+but none of them carry an expanded plan.
+
+The projection becomes an exhaustive `Pick<PlatformPlanLimits, ProjectedFlagId>` object literal
+instead of a hand-written id array, so adding a `FeatureFlagId` that has a `platform_plan` column
+fails the build until it is mapped. `agentsEnabled` joins the projection under that rule while no
+Autumn plan grants it yet, which switches agents off for every Cloud platform and every license-keyed
+EE self-host until the feature is attached to plans. CE is unaffected — it skips entitlement sync.

brain/knowledge/pieces-engine/building-pieces.md

@@ -27,6 +27,11 @@ Authentication, triggers (polling/webhook), properties + validation, flow contro
 - **Porting postgres `new-row.ts` to another SQL piece: the `LIMIT 5` is cold-start only — do not carry it onto the resume branch.** `constructQuery` (`postgres/src/lib/triggers/new-row.ts:41`) has two shapes, and the asymmetry between them is load-bearing: the no-checkpoint branch seeds with `ORDER BY %I DESC LIMIT 5` (`:46,48`), while the resume branch is deliberately **unbounded** — `WHERE %I >= %L ORDER BY %I DESC`, no LIMIT (`:58,60`). It has to be, because `DedupeStrategy.LAST_ITEM` (`:17`) recovers the checkpoint by scanning *the page it just fetched* (`pieces/common/src/lib/polling/index.ts:99`, `items.findIndex((f) => f.id === lastItemId)`) and emits everything ahead of it. Bound the resume page and the checkpoint row can fall off the end, where `findIndex → -1` is read as "no checkpoint" and the entire page re-emits ([triggers.md](../flows-execution/triggers.md) has the same mechanic from the republish side). So a literal `LIMIT 5` → `TOP (5)` is a behaviour change, not a dialect translation — and the moment you *do* want a bounded resume page you are off `pollingHelper` altogether and owe a keyset cursor that carries its position in the store instead of recovering it by scanning: `microsoft-sql-server/src/lib/common/cursor.ts` is the worked example (`TOP (@limit)` on every page, versioned cursor, explicit tiebreaker key). Two more sharp edges if you copy this template: the item id is `orderValue + '|' + md5(JSON.stringify(row))` (`:24-28`), so **any edit to the checkpoint row changes its id and invalidates the checkpoint**, and `lastItem.split('|')[0]` (`:42`) truncates any order value containing a literal `|` — fine for timestamps and serial ids, wrong for ordering on a text column.
 - **Streaming a file *into* a piece is `Property.File({ streaming: true })`.** It resolves to an `ApStreamingFile` with `body: Readable` (pieces-framework ≥ 0.35.0, [000014](../../decisions/000014-streaming-file-inputs-resolve-to-a-lazy-apstreamingfile.md)) and accepts a URL, a base64 data URL, the builder's file picker, or a previous step's file — a strict superset of a URL text field, with the fetch owned by the engine. `amazon-s3/upload-file.ts` and `subflows/stream-csv-to-flow.ts` are the references. Three things to know: the engine's `fileProcessor` swallows fetch failures and returns `null`, which for a `required: true` prop surfaces as the confusing `Expected file url or base64 with mimeType` validation error rather than a fetch error (so no `isNil` guard in your `run()` is needed — the action never starts); the engine's fetch has **no timeout**, so a source that connects then stalls burns `FLOW_TIMEOUT_SECONDS`; and `.pipe()` does not forward `'error'`, so you still need `file.body.on('error', ...)` or a mid-stream network drop becomes an uncaught exception in the sandbox.
 - **Streaming only removes *our* memory ceiling — check the destination's per-request cap before calling an upload action fixed.** A body that streams cleanly out of the sandbox still gets rejected whole by the API: Dropbox's `/2/files/upload` answers `409 {".tag": "payload_too_large"}` above 150 MB, Graph's simple `PUT …/content` above 250 MB. The tell that it's the service and not us is the shape — an endpoint-specific 409 with a documented error tag, and an axios/undici request echo whose `body` is just a `_readableState` blob (our stream, sent fine). The fix is a chunked upload session, not a bigger buffer: `dropbox/upload-file.ts` and `microsoft-onedrive/upload-file.ts` are the references, both chunking through the shared `streamUtils.readChunks({ readable, chunkSize })` from `@activepieces/pieces-common` — reuse it rather than writing a third stream chunker. Two rules that fall out of doing it: **route unknown-size sources through the session too** (`size` is best-effort and absent on chunked or compressed sources, so you cannot prove they fit — and the old fallback of buffering to learn the size is the OOM this streaming work exists to remove), and keep the chunk size a multiple of the service's preferred unit (4 MiB for Dropbox, 320 KiB for OneDrive). Chunk bodies are `Buffer`s, so unlike a one-shot stream body they keep `httpClient`'s retries. **Whether you can chunk an unknown-size source at all depends on how the session addresses its parts:** Dropbox's is offset-based (`cursor.offset`, no total ever declared) so it streams straight through, while Graph's wants the file's total length in every fragment's `Content-Range` — so `microsoft-sharepoint` and `microsoft-onedrive` must `readableToBuffer` once to learn the length, then re-wrap with `Readable.from` so both branches still take a stream. That buffer is the OOM this work removes, so it is a last resort, not the pattern: reach for the offset-based session whenever the API offers one. SharePoint's cap is generous enough (250 MB one-shot vs OneDrive's 4 MiB) that the buffer only ever runs for a size-less source.
+- **On Windows, a new action/trigger name (or any metadata-shape change) needs the dev server process killed, not restarted.** `clearPieceModuleCache` — the only thing that busts the CommonJS `require()` cache backing dev piece metadata — is called exclusively from the chokidar watcher's rebuild handler (`dev-piece-watcher.ts`), and that watcher does not fire reliably on Windows for tool-made edits. A "normal restart" reuses the same PID (confirm with `netstat`/`Get-Process` bound to the dev port), so the server keeps serving the stale metadata. Find the PID bound to the dev API port and `Stop-Process -Id <pid> -Force`, then start fresh — every other change (prop text, logic inside `run()`) hot-reloads fine; only new action/trigger names or output-shape changes hit this.
+- **`Property.Array`'s `properties` sub-schema never threads into its resolved `propsValue` type — confirmed in `packages/pieces/framework/src/lib/property/index.ts`.** `propsValue.someArrayProp` types as plain `unknown[]` regardless of what `properties` declares, so casting to the declared row type at the point of use is the only option; there is no framework-provided type-safe path around it. Document the cast in a comment so it doesn't read as an oversight on a later pass.
+- **`Property.Dropdown` (dynamic single-select) cannot go inside `Property.Array`** — it's excluded from `ArraySubProps` in `packages/pieces/framework/src/lib/property/input/array-property.ts`. A line-item array that needs to reference another resource by id (e.g. "which item/account does this line use") can't put a searchable dropdown per row; resolve by exact name server-side in `run()` instead (a lookup helper keyed on the row's plain text field) rather than falling back to a raw-id text field.
+- **Ungrouped props render after every declared `propertyGroups` section, not inline in prop-declaration order.** A "mode selector" prop that decides which of several sections is relevant (e.g. a payment-type toggle gating Accounts-Receivable vs Accounts-Payable fields) must get its own section declared *first* in `propertyGroups`, or it renders dead last — after the very fields it's supposed to gate. Caught via visual review, not build/lint.
+- **`HttpRequest.queryParams` (`@activepieces/pieces-common`) is `Record<string, string>` — one value per key, no array support** (confirmed in `query-params.ts`). A third-party API that wants a repeated param (`type=a&type=b`) rather than a comma-joined value can't be satisfied through `queryParams` alone. Fix: pre-encode the repeated params directly into `resourceUri`'s query string (`resourceUri: '/x?type=a&type=b'`) — `getUrl()` parses and preserves an existing query string on the URL before merging the `queryParams` object on top, so both coexist correctly.
 - **Every piece you touch in a PR needs a version bump, and CI only names the first one.** `validate-publishable-packages` runs `packagePrePublishChecks` (`tools/scripts/utils/package-pre-publish-checks.ts`) over every piece directory: if the piece's `package.json` version is already the npm `latest` **and** `git diff origin/main -- <piece>` is non-empty, it throws `package version not incremented` — unless that piece's own `package.json` also changed, which is how a bump satisfies it. Two traps. The diff is against **`origin/main`, not the PR base**, so a stacked PR inherits every piece its base touched and must bump those too. And the checks run in `Promise.all` batches of 10, so the first thrown error kills the process — the log names one piece (`azure-ad`) when 27 are equally broken. Don't fix the named one and re-push; enumerate `git diff --name-only origin/main...HEAD | grep pieces/` and bump the whole set at once. Patch bump is the convention even for behaviour changes like added OAuth scopes. `packages/pieces/framework` and `packages/pieces/common` are exempt (explicit `notPublished` list in `validate-publishable-packages.ts` — pieces inline them at build time), as is everything outside `packages/pieces/`. The script is runnable locally, and takes ~3 min: `npx ts-node -r tsconfig-paths/register -P packages/server/engine/tsconfig.lib.json tools/scripts/validate-publishable-packages.ts`.
 
 ## Sharing & misc