diff --git a/brain/knowledge/ai-intelligence/ai-agents.md b/brain/knowledge/ai-intelligence/ai-agents.md index aaaf2c6b2e74..4402b623bb55 100644 --- a/brain/knowledge/ai-intelligence/ai-agents.md +++ b/brain/knowledge/ai-intelligence/ai-agents.md @@ -44,6 +44,7 @@ A flow step type (the `run_agent` action of `@activepieces/piece-ai`) that runs Server-side `toolName` is log-only — `executePieceTool` / `executeFlowTool` / `executeKnowledgeBaseTool` route on `piece`, `flowId` and `knowledgeBaseFileId` — so rewriting it breaks no lookup. `stepResultFrom` must be passed the knowledge-base tools too, or its `ToolCallType.KNOWLEDGE_BASE` branch is unreachable and the card shows the rewritten key instead of the file name. - **A retired model is user config, and only a marker in the message says so.** A provider 400 stays `internal` by default; one whose *message* matches `MODEL_UNAVAILABLE_PATTERNS` is `user`. Two deliberate narrowings, both learned the hard way: the body is **never** scanned, because any 400 carrying an HTML error page that says "deprecated" in its footer would launder our own outage; and a retired model on the managed `activepieces` key is **ours**, since `resolveModelIdForProvider` substitutes `curatedModels[0]` for anything uncurated — a stale constant in our repo failing every platform at once must page, not read as "the customer picked a bad model". That substitution still classifies as `user` on a BYO key, which is the residual gap. - **The curated chat lists rot silently and nothing checks them, but "the ticket said it's deprecated" is not evidence.** `ALLOWED_CHAT_MODELS_BY_PROVIDER` (`core/piece-types/src/lib/ai-providers.ts`) is the only thing deciding what the pickers offer; the models.dev catalog is metadata keyed by id and adds or removes nothing. Check a suspected-dead id against models.dev before deleting it — of the four ENG-466 named, only `grok-4.1-fast` had actually gone; the Gemini 2.5 pair was live, current and the *cheapest* Google option. Removing a live model is not a cleanup: `resolveModelIdForProvider` falls through to `curatedModels[0]`, so a BYO customer pinned to Flash would have silently moved to a Pro preview at roughly five times the token price, on their own key, with no notice and no migration. Three things move together when editing a list: `CHAT_MODEL_LABELS` (a curated id with no label fails `ai-providers.test.ts`), `MANAGED_MODEL_WEIGHTS` in `flow-run-ai-usage-tracker.ts` (its `?? 2` default is *below* the table's floor of 6, so a forgotten managed model under-bills — every `x-ai/*` id does today), and the **order**, since `curatedModels[0]` is both the picker's first row and the fallback for every unrecognised selection. +- **An agent's tools reach the worker in two shapes, and only one of them can actually run a flow.** `ExecuteAgentRunJobData` carries `tools` (the raw `AgentTool[]`) *and* `flowTools` (resolved: `flowId`, `flowVersionId`, JSON input schema). The worker builds its flow tool set from `flowTools` alone — `data.tools` is only filtered for PIECE, MCP and KNOWLEDGE_BASE, so the `AgentToolType.FLOW` entries in it are inert. An enqueue site that fills only `tools` therefore drops every flow tool with no error and no log: the Configure panel still lists them, the chat footer still names them, and the model is simply told it has no such tool. This is how saved-agent chat shipped for three releases while the same agent worked inside a `Run Agent` step, which resolved them. Both enqueue paths now go through `agentHelpers.resolveFlowTools`, which takes the **whole** tool array and filters internally, so no caller can do half the job. Resolve against the project the run will execute in — `executeFlowTool` looks the flow up in the *conversation's* project and refuses anything else, so resolving elsewhere hands the model a `flowId` the RPC then rejects. - **Whatever enqueues an agent run must pre-check the same thing the worker resolves.** The chat route asked "is any provider enabled for chat" while the worker looked up the run's *pinned* provider, and the flow-step route checked nothing at all — so a run enqueued fine and could only fail. Both now call `agentHelpers.assertRunProviderConfigured`, which mirrors the worker's lookup. A pre-check that answers a *different* question than the worker is worse than none: it makes the failure look impossible. - **Everything the agent job does before its try/catch has no recovery.** `getAgentConfig` used to run outside it, so a config failure sent no error to the chat client and never called `releaseFlowStep` — the flow run sat PAUSED until `AP_PAUSED_FLOW_TIMEOUT_DAYS`. Anything added above that block needs its own failure path, or a paused run leaks. - **Build the unattended tool set as an allow-list.** Removing chat tools by name failed three times running — display tools, then build-plan and phase tools, then `ap_discover_action_auth` and `ap_load_guide`, which live with the local tools and so survived a filter written by tool group. Grouping tracks where a tool was constructed, not whether it assumes someone is reading. A flow step gets exactly what it is listed: its configured piece actions, the public-web readers, and the structured-output tool. Anything added to chat later stays out by default. @@ -61,6 +62,8 @@ A flow step type (the `run_agent` action of `@activepieces/piece-ai`) that runs - **A tool's *property keys* are user text too, and nothing guards them.** `mcpToolNameUtils` covers the tool NAME; the input schema's keys come straight from `property.name`, the field labels a customer typed into the MCP trigger, at `agent-run-controller.ts` (`resolveFlowTools`) and `mcp-server-builder.ts` (`registerFlowTools`), which build the shape identically. A field called `Email Sender` produces a key with a space, which Anthropic rejects on the same `^[a-zA-Z0-9_.-]{1,64}$` rule it applies to names, so that flow-as-tool step can never succeed. Sanitising is only half a fix: the payload is handed to the flow as its trigger body, so `{{trigger['Email Sender']}}` in existing flows needs a key→label map to translate back. Fix it in one shared helper, because the MCP server ships the same invalid schema to any Anthropic-backed client. +- **Do not raise `ai` past 7.0.69 — from `ai@7.0.70` the SDK stops auto-executing tools, and it fails silently.** Bisected on 2026-09-21 while trying to reach `experimental_evaluate`. On 7.0.44 (what the repo pins) and every version through 7.0.69, a tool call executes normally. From **7.0.70** onward, including 7.0.103–7.0.107, `generateText`/`streamText` still *recognise* the call — `staticToolCalls=1`, a `tool-call` part is emitted with correctly parsed input — but `execute` is never invoked and `toolResults` stays `0`. No error, no warning. The changelog entry for 7.0.70 is `a828527: Prevent automatic tool execution when a model call ends with an unsafe finish reason`, but the gate fires for every `finishReason` tried, `tool-calls` and `stop` included. Reproduced in a clean project outside the monorepo, on both `MockLanguageModelV3` and `MockLanguageModelV4`, on zod 3 and zod 4; `needsApproval: false` and `toolApproval: () => false` do not restore it, and neither does emitting the full `tool-input-start`/`delta`/`end` lifecycle before the `tool-call`. The alarm is `packages/server/worker/test/lib/agent-eval/agent-tool-call-repair.test.ts` — 4 of its 5 tests go red, and they look like a *repair* regression, which is the wrong trail: a plain valid tool call does not execute either. **Consequence: `experimental_evaluate` (first shipped in 7.0.103) cannot be reached without breaking the agent**, so anything needing typed evaluation calls the gateway's `/v4/ai/evaluation-model` endpoint over `safeHttp` instead of taking the bump. Re-test the table above before any future `ai` upgrade; if a release fixes it, that is the one to jump to. + ### Key files Entry point: `runAgent`, the createAction in the `ai` piece registered in `packages/pieces/community/ai/src/index.ts`. diff --git a/brain/knowledge/ai-intelligence/ai-providers.md b/brain/knowledge/ai-intelligence/ai-providers.md index 9e31aaf14ec4..13d7a3ba78c6 100644 --- a/brain/knowledge/ai-intelligence/ai-providers.md +++ b/brain/knowledge/ai-intelligence/ai-providers.md @@ -54,10 +54,14 @@ renders and preserves every real price; the cheapest in the set is 0.01. - **The catalog object must be published before the code that reads it ships.** There is no bundled copy, so until a `workflow_dispatch` run puts it on the CDN, every install — including local dev — shows no metadata at all. - **A fresh Cloud platform cannot connect any AI provider, and the UI says nothing about why.** The admin page at `/platform/setup/ai` sets `allowWrite = platform.plan.aiProvidersEnabled`, and that column defaults to **false** on the `free` plan (migration `1776…AddDefaultToAiProvidersEnabled`), so the connect button is simply absent rather than disabled-with-a-reason. On Cloud the flag is owned by Autumn (`autumn-utils.ts` lists it among the synced features), so there is nothing to configure locally and no `DEV_ENTERPRISE_PLAN` escape hatch on main. For local Cloud testing, flip it directly: `UPDATE platform_plan SET "aiProvidersEnabled" = true`. Then connect a **BYO key**, not the managed `ACTIVEPIECES` provider — per *Provider visibility* above, the managed row is hidden whenever `OPENROUTER_PROVISION_KEY` is unset, which is the normal local state, and a hidden provider makes `getChatProvider()` return null. Anything gated on a chat provider (chat itself, personalization research) stays silently disabled until a BYO row exists. +- **Every provider auth schema is written twice, and only one copy is the one the server reads.** `packages/core/utils/src/lib/ai-provider-credentials.ts` (plain zod) backs the API and the web app; `packages/core/piece-types/src/lib/ai-providers.ts` (`zod/mini`, so `.check(z.minLength(1))` and `z.optional(x)` instead of `.min(1)` / `.optional()`) is what `@activepieces/pieces-framework` re-exports to pieces and the engine. Adding a credential field to only one leaves the other silently stripping it — zod objects drop unknown keys, so the upsert still returns 200 and the failure surfaces later as a provider-side auth error. Change both. +- **A schema change in `packages/core/utils` is invisible to another package's tests until you build it.** Sibling packages resolve `@activepieces/core-utils` through its `main` (`dist/src/index.js`), so vitest in `core/ai-providers` keeps parsing against the stale build and a new optional field reads back as dropped. `cd packages/core/utils && npm run build` first. The API dev server is the opposite: `serve` runs under tsx with the tsconfig path alias, so it reads `src` directly — but it only watches `packages/core/shared/src/**`, not `core/utils`, so a core-utils-only edit needs a restart to take effect. The same staleness makes a **branch** look green when it is not: `dist` is gitignored, so after `git checkout` it still holds whatever the previous branch built, and a test that depends on a field added by a sibling PR passes locally and fails in CI. Rebuild `core/utils` after every branch switch before trusting a run. + - **`getEffectiveProviderAndModel` exists for one provider, and skipping it makes Cloudflare Gateway fall off the end of any per-provider switch.** It looks like generic provider resolution; it is not. It returns its input untouched for everything except `CLOUDFLARE_GATEWAY`, where it splits the model id and maps the submodel prefix (`anthropic`, `openai`, `google-ai-studio`, `google-vertex-ai`) back to the real provider. So a gateway model of `anthropic/claude-…` only reaches an Anthropic-keyed branch if you called it first. Any table or lookup keyed on `AIProviderName` — the web-search tool table, capability checks — has no `CLOUDFLARE_GATEWAY` entry and returns nothing for one that skipped the call: no tool, no error, no log. The AI piece calls it at the top of `buildWebSearchConfig`; server-side code that dispatches per provider needs the same call or the same resolution built in. - **The Agent and the Direct AI steps disagree about whether OpenAI does web search, and both are right.** `WEB_SEARCH_MODE_BY_PROVIDER` has no `OPENAI` entry, so `supportsWebSearch(OPENAI)` is `false` — the Agent asks that question before offering the tool and therefore has never had OpenAI web search (deliberately: it needs the Responses API, which breaks legacy BYOK models). Ask AI never asks it; it builds the tool straight from its own props and has had OpenAI web search all along. So `supportsWebSearch` is the *agent's* policy, not a statement of fact about the provider — reusing it as a general gate silently removes a capability Ask AI users have today. Whoever unifies the two paths has to keep the OpenAI branch reachable without routing it through that gate. The two paths also disagree about the unsupported tail: the piece's `webSearch` prop is a plain checkbox with no provider gating, and `createWebSearchTool` ends in `default: throw new Error('Provider X is not supported for web search')` — so ticking it on Azure, Mistral, xAI, or a Cloudflare Gateway model whose submodel prefix is not one of the four in `CF_GATEWAY_SUBMODEL_TO_PROVIDER` **fails the step**. The server's `buildWebSearchTools` returns `{}` for the same input, because the agent gates before calling it. The builder cannot simply be made to throw: `supportsWebSearch` is true for the `plugin` providers (`openrouter`, `activepieces`), the agent calls the builder for them, and there is no native entry for either because the plugin rides on the model settings instead — so `{}` is the correct answer there. The piece has the same shape, returning early for OpenRouter before the switch that throws. Hence two entry points: `buildWebSearchTools` stays total for callers that gate first, `buildWebSearchToolsOrThrow` reproduces the piece's error for callers that do not. +- **The AI-provider create/update endpoints reject unknown `auth` keys rather than stripping them** (`strictAuth` in `packages/core/shared/src/lib/management/ai-providers/index.ts`). This is deliberately the request boundary only: `aiProviderCredentials()` still parses *stored* rows through the loose schemas, because a strict parse of a legacy row falls through to the `auth: {}` fallback and silently empties a working key. If you tighten the shared schemas themselves, you move a loud 400 into a quiet credential loss. - **`aiProviderModule` is registered unconditionally, `agentModule` is not.** In `app.ts`, `aiProviderModule` sits outside the edition switch, while `agentModule` appears only in the `CLOUD` and `ENTERPRISE` arms — so an AI route hung off the provider module is on Community for free, and the same route hung off the agent module 404s there. That asymmetry is the cheapest way to satisfy [000036](../decisions/000036-the-worker-side-ai-path-is-community-edition-core.md): pick the module and there is no edition branch left to get wrong. - **Chat falls back to the managed Activepieces row, so "no key is flagged for chat" is no longer a wall.** `pickChatRow` takes the `enabledForChat` row if there is one and otherwise the visible `ACTIVEPIECES` row; `findAvailableChatProviderRow` and both list endpoints share it, so what the picker shows is what a turn will actually run on. Before that, a platform whose managed row sat at `enabledForChat = false` — and nothing else flagged — got `SetupRequiredState` ("Set up an AI provider to get started") a second after the chat loaded. Migration `1802…AddChatRolloutFreeCreditGrant` was meant to repair exactly that state and matched zero rows: it compared `"provider" = 'ACTIVEPIECES'` while `AIProviderName` values are stored lowercase (`'activepieces'`). Enum values in raw SQL are the lowercase string, never the TS member name — grep the enum before writing the WHERE clause. @@ -112,7 +116,13 @@ renders and preserves every real price; the cheapest in the set is 0.01. per turn for an agent** — an agent turn is many model round-trips, so charging it per call would have silently raised the price of every agent on a customer's own key. The caller says which it wants with `ownKeyCredit`, and the agent's turn credit is charged in `chatToolBilling.chargeForLatestTurn` - alongside its tool calls. + alongside its tool calls. **The worker does not know the edition, so the one own-key credit is + reported on Community too; the API is what drops it.** `aiChargeFor` returns fixed credits for any + non-`ACTIVEPIECES` provider, and only the CLOUD and ENTERPRISE arms of `app.ts` set the Autumn + `billingProvider`; the default provider's `trackFeature` is a no-op, so Community records nothing + and its `getCreditsAndAppSumoState` never blocks. Enterprise records the credit but + `shouldBlockRunOnCredits` returns false for that edition, so only Cloud can refuse an AI call for + being out of credits. Self-hosters on their own OpenRouter key pay OpenRouter directly, not us. - **OpenRouter only reports `usage.cost` when the request asks for it.** The provider sends `usage: this.settings.usage` in the request body and copies `cost` into `providerMetadata` only when the response carries one, so without `usage: { include: true }` every managed call reports no cost, @@ -160,3 +170,5 @@ renders and preserves every real price; the cheapest in the set is 0.01. - `packages/web/src/features/agents/ai-model/` — model selector used in agent step settings Paths verified 2026-07-26. +- **`OPENROUTER_PROVISION_KEY` is a management key and cannot make inference calls; only the per-platform keys it mints can.** OpenRouter states it outright: management keys are for creating, listing, updating and deleting other keys and are rejected by every completion or decisions endpoint. So "we already have an OpenRouter key" is true for provisioning and false for anything that needs to call a model at the instance level (the AI Router's Jev call, for example): that needs an ordinary inference key from the same account, created in the dashboard or via the same `POST /keys` the instance already uses, with a credit `limit`. The AI Router does exactly that since 2026-09-23: it runs on the platform's managed key where credits are on, else the admin's own OpenRouter row, as a worker `EXECUTE_AI` job billed like any AI step. +- **`z.enum(SomeTsEnum).exclude([SomeTsEnum.MEMBER])` compiles and excludes nothing at the type level; pass the key string, `.exclude(['MEMBER'])`.** zod's `exclude` is typed over `keyof T` and returns `ZodEnum>`. A TS enum member is its own literal type, not the string `'MEMBER'`, so `Omit` finds no key to drop and the inferred value type keeps every member, while the runtime check does exclude it. The symptom is one step removed from the cause: a switch over the parsed field is suddenly non-exhaustive (`switch-exhaustiveness-check` in lint, `TS2366` in the build) even though the branch you removed can never arrive. `/v1/ai/execute` hit this when `ROUTE` was carved out of `AiStepAction` for #15707. Neither vitest nor a package-local eslint run without type information reports it; only the build does. diff --git a/brain/knowledge/ai-intelligence/mcp-server.md b/brain/knowledge/ai-intelligence/mcp-server.md index d2cd7b043eea..54e9bd3be4a9 100644 --- a/brain/knowledge/ai-intelligence/mcp-server.md +++ b/brain/knowledge/ai-intelligence/mcp-server.md @@ -126,3 +126,4 @@ Paths verified 2026-07-17. list rather than hiding the rows. Note the failure shape: a disabled tool is never `registerTool`d, so the client gets an unknown-tool error from the protocol, not a permission denial from inside the tool — the copy "every call fails" is directionally right but one layer off. +- **The wording of an MCP tool's `❌ …` reply is part of a contract: `test/integration/ce/mcp/mcp-tools.test.ts` asserts substrings of it verbatim.** Rewording `resolveRouterStep`'s rejection from `not a ROUTER step` to `not a router step (ROUTER or AI_ROUTER)` failed case 62 of that suite, and only in CI, because the CE integration tests need Postgres and are not run locally. Before touching any tool's user-facing text, `grep -n "" packages/server/api/test/integration/ce/mcp/` and either keep the asserted substring or update the assertion in the same commit. diff --git a/brain/knowledge/decisions/000016-managed-ai-metering-moves-to-centralized-worker-execution.md b/brain/knowledge/decisions/000016-managed-ai-metering-moves-to-centralized-worker-execution.md index d365f2c51084..16aabade1496 100644 --- a/brain/knowledge/decisions/000016-managed-ai-metering-moves-to-centralized-worker-execution.md +++ b/brain/knowledge/decisions/000016-managed-ai-metering-moves-to-centralized-worker-execution.md @@ -54,3 +54,4 @@ managed-AI gate/metering should fold into it rather than being hardened in its c - The centralized path must preserve today's non-blocking properties (billing must not add per-token latency, per WS6): the goal is *observed* usage at one place, not a synchronous console/Autumn round-trip on every token. +- The AI Router (2026-09-23) is the first non-language-model call through the chokepoint: an `EXECUTE_AI` job with `action: ROUTE`, billed from the Decisions response's `usage.cost` via `activepiecesAiCost.reportProviderCost` on the managed key, or one fixed credit on an own key. No managed AI call is exempt. "Runs in the worker" names where the model call and the billing happen, not the request's origin: the engine still posts to the API (`/v1/engine/ai-router`, or `/v1/ai/execute` for AI pieces), because the sandbox can reach nothing but the API and must never hold a provider key — a reviewer's first reading of the architecture diagram was that the engine hop had been removed. diff --git a/brain/knowledge/decisions/000039-the-ai-router-is-its-own-action-type-and-bills-nobody.md b/brain/knowledge/decisions/000039-the-ai-router-is-its-own-action-type-and-bills-nobody.md new file mode 100644 index 000000000000..1906742ec3d5 --- /dev/null +++ b/brain/knowledge/decisions/000039-the-ai-router-is-its-own-action-type-and-bills-nobody.md @@ -0,0 +1,122 @@ +--- +title: The AI router is its own action type and runs as a worker AI step +icon: 🔀 +status: accepted +--- + +# The AI router is its own action type and runs as a worker AI step + +## Decision + +**AI Router is a real `FlowActionType.AI_ROUTER`**, not a Router mode and not a piece action. It asks +one question at the root and each branch is one possible answer, inverting the Router's one-condition- +tree-per-branch model. It calls Jev (`typesafe/jev-1.13`) through **OpenRouter's Decisions API on the +platform's own OpenRouter key**, as a `ROUTE` action of the worker's `EXECUTE_AI` job, and it **pauses +the run while it waits**: the engine creates a waitpoint, posts to `/v1/engine/ai-router` with its id, +marks the step `PAUSED` and gives its worker slot back; the API picks the provider, gates credits and +enqueues; the worker makes the call, bills it and resumes the run through `resumeAiStep`. On resume a +decided router reuses its stored evaluations and never asks again, so a pause inside a route costs +nothing. Only a single-action run, which cannot pause, still waits synchronously. The key is the managed Activepieces provider where credits are on, else the admin's own +OpenRouter row; the step has no provider or model picker and appears in the picker only when the +project's provider list contains one of the two. Billing is decision 000037 unchanged: observed +`usage.cost` on the managed key, one fixed credit on an own key. In the picker it sits in the Utility +tab under **Flow Controller** beside the Router, never in the AI tab: it is a branching step that +happens to use a model, and `isAiAndAgentPiece` says so by returning false for every core type. + +## Context + +The capability first shipped as `AI_MATCHES`, a 23rd entry in the Router's condition-operator +dropdown (PR #15666, parked unmerged). It worked and nobody could find it — there is nothing to +market from inside a ``, and a condition row is the wrong shape for the decision anyway. + +A piece action was the cheap option and is not buildable: a piece cannot own child steps, so "AI +Router as a piece" is Classify Text followed by a normal Router. + +## Why + +**Separate type, for the product.** The requirement was discoverability, and a distinct card, icon, +settings panel and docs page is what delivers it. The cost is real — `FlowActionType.ROUTER` appears +in 31 source files — but most sites widen to `flowStructureUtil.isBranchedAction` in one line, and +the engine's branch-walking loop was extracted to `executeBranches` so both routers share it. + +**One `choice` question, not one `boolean` per branch.** Measured live against Jev on 2026-09-21: +three sequential boolean calls took **1699 ms / 871 input tokens**; the same decision as one choice +question took **385 ms / 367 tokens**. The choice answer is also guaranteed to be one of the option +names, so "the model picked a branch that does not exist" is structurally impossible. + +**The fallback branch is sent to the model as a real named option.** This is the non-obvious part. +Without it, the input `"hi"` routed to `Sales` at probability **0.87** — confidently wrong, and past +any sane threshold. With an explicit `Otherwise` criterion, `"hi"` and `"what is the weather today"` +both go to Otherwise at **1.00** while a clear billing message still goes to Billing at 1.00. A +confidence floor is kept as a second, independent net, but it is not the mechanism. + +**Billed like any AI step, through the worker chokepoint.** The first cut billed nobody and called a +Vercel AI Gateway from the API process with an instance-wide key, on the reasoning that Jev costs +$0.042 per 1M input tokens and a usage pipeline would cost more than the inference. Its own +Consequences warned that metering had to land before flows depended on free routing, and the PR was +unmerged, so it did. Running the call as an `EXECUTE_AI` job reuses the pipeline decisions 000016 and +000037 already built — provider resolution, `assertCreditsAndAppSumoNotExceeded` before the job, cost +reported from the worker over RPC — instead of a second one beside it. The step still asks nothing of +the user: the provider is picked server-side from `listForProject`, which is also what the picker +reads, so server and picker agree by construction. + +## Consequences + +A routing decision costs the platform what it costs us on the managed key (about 0.02 to 0.08 +credit) and one credit on an own key; a production run through one router is therefore about one +credit plus a fraction. The pause exists because a flow job and the router's `EXECUTE_AI` job share +one worker queue and a loop holds its slot for the whole job: a router that blocked on its own answer +would starve itself at low concurrency and deadlock five flows at the default of five (see the +starvation gotcha on [[workers]]). The waitpoint's deadline is ten minutes; a deadline resume carries +no answer and fails the step with a readable message. The synchronous path (single-action runs) keeps +the old nesting: engine 30 s, API 25 s, worker HTTP 8 s. Community Edition has no OpenRouter key +unless the admin adds one, so the step is hidden there by default — the price of dropping the +instance-wide env var. OpenRouter's Decisions endpoint is alpha; its wire format lives in one worker +file (`route.ts`) with a unit test per answer shape. + +A failed or timed-out call **fails the step**. It never quietly takes the fallback, because routing a +refund down the wrong branch because a model timed out is a correctness bug that looks like normal +operation. + +The step started with **no `executionType`**, on the reasoning that a choice answer names exactly one +branch so all-match had no meaning. That held only while the step asked one `choice` question. It now +carries `matchMode`: `BEST_MATCH` asks one `choice` question and exactly one route runs, while +`ALL_MATCHES` asks one `noul` question per route — in a single request, since Jev's +`questions` field is a record — and every affirmed route runs. The fallback rule mirrors the Router's +exactly: it runs iff no other route did. + +The two modes differ in a way that is easy to get wrong: **`ALL_MATCHES` never sends the fallback to +the model.** There is no forced choice to decline from, so booleans can all be false on their own and +the fallback is computed locally. The measured "give the model an explicit `Otherwise` criterion or +`hi` routes to Sales at 0.87" finding therefore applies to `BEST_MATCH` only. `ALL_MATCHES` also +costs one question per route instead of one per step, which is the honest trade for fan-out. + +`AI_MATCHES` (PR #15666) is parked. If it ever merges, the product carries two AI branching concepts +and that needs its own call. + +**The AI router will never absorb the Router, and the reason is worth knowing before someone +proposes it.** The two differ in *evaluator*, not interface: determinism, zero cost and zero latency +are properties of not calling a model. You can get "same answer usually"; you cannot get "same answer +forever", because the model version lives outside the flow and a vendor upgrade can re-route a +historical case. Temperature, seeds and caching do not change that. So anything a human must defend +years later — compliance gates, billing tiers, approval thresholds — stays on the Router permanently. + +The gaps that *are* reachable, and how, so the design does not get re-derived: **fan-out** by a +match-mode toggle (one `choice` question → one route, N `boolean` questions → every route that +applies, which is the Router's first-match/all-match concept and the cheapest of the three); +**facts** (thresholds, exists, two-value comparisons) by an optional one-line deterministic *guard* +per route, evaluated first so guarded-out routes never reach the criteria map; **multi-dimensional +routing** by asking several questions in one call and matching routes on combinations, which Jev's +plural `questions` record already supports and which the Router cannot do on meaning at +all. The guiding line is that the model turns unbounded text into a small enumerated value and +everything after that is data. The standing risk on guards is that a nested condition builder inside +a route rebuilds the Router twice and loses the pitch — one guard, one line, no AND/OR groups. + +The MCP flow-building tools resolve both router kinds for structure, validation and delete. +`ap_add_branch` and `ap_update_branch` refuse an AI router with a message, and `ap_add_step` cannot +create one, because letting the agent build one without settings support would let it build broken +steps. Those three need a per-type body. + +**Jev is on OpenRouter, but not where a catalog scan looks.** `GET /api/v1/models` lists text-producing models only; Jev's output modality is `decisions`, so it is absent there and lives at `POST https://openrouter.ai/api/alpha/decisions` as model `typesafe/jev-1.13` (`jev-latest` 404s, so pin the version). Same price as the gateway, $0.042/M in and free out, 32k context. The wire format, confirmed against OpenRouter's own Jev tutorial: body `{ model, state, questions }` with `state` an object (we send `{ text }`), a yes/no question is `type: "noul"` and its answer is `{ type: "noul", noul }` where `noul` is P(true) — no `answer`, no `probability` field — a choice answer is `{ choice, confidence, probabilities }`, and `usage` carries `input_tokens`, `output_tokens` and `cost` in dollars by default. A first pass of this note claimed OpenRouter had no Jev, from scanning the text-model catalog only. + +**Decided 2026-09-23: run in the worker on OpenRouter, reversing the previous day's "stay on the gateway".** That call rested on a false premise: it said the only OpenRouter key the instance held was the management key, which cannot call a model. The management key is indeed inference-blind, but `enrichWithKeysIfNeeded` uses it to mint a real inference key per platform for the managed Activepieces provider, and any admin who added OpenRouter under AI providers holds one too. So the deployment already had a callable key, the gateway added a second vendor for nothing, and the worker pipeline could bill the call for free. The move also improves `.claude/rules/self-hosting.md` compliance: one env var fewer, and the key it reuses is one the deployment already manages. diff --git a/brain/knowledge/engineering/ci-pr-review-hygiene.md b/brain/knowledge/engineering/ci-pr-review-hygiene.md index bb5df7218db4..65f3e930e258 100644 --- a/brain/knowledge/engineering/ci-pr-review-hygiene.md +++ b/brain/knowledge/engineering/ci-pr-review-hygiene.md @@ -30,10 +30,13 @@ Enforcement is the **`Codeowners review` repository ruleset** (active on the def - **Repo-wide regenerators sweep `main`'s pending drift into your PR — run them, then keep only your own lines.** `npm run i18n:extract` reorders all of `en/translation.json` and rewrites nine locale files (130 moved lines for six new keys), and `bun install` after a version bump writes back every community-piece version that was bumped without a lockfile sync (103 lines for four intended bumps). Both diffs are indistinguishable from real work in review, and both bury the change you actually made. Revert the file and hand-apply your own entries instead — then prove parity by running the generator into a scratch copy and diffing just your keys against it, so you keep byte-identical output without the churn. Provider setup markdown in `features/agents/ai-providers.ts` is extracted as translation keys in **source order**, so new entries go beside their neighbours in `SUPPORTED_AI_PROVIDERS`, not at the end. - **`.env.dev` is TRACKED, so the `.env*` line in `.gitignore` does not protect it — secrets put there get committed.** `.gitignore` line 82 is `.env*`, which reads as blanket protection for every env file, but gitignore has no effect on a path already in the index, and both `.env.dev` and `.env.example` are committed on `main`. `git check-ignore .env.dev` returns nothing, which is the tell. So an SMTP password or API key dropped into `.env.dev` shows up in `git status` as a normal modification and rides the next `git add -A`. Put local secrets under `dev/` instead — that whole directory is genuinely ignored (line 27) — and reach for `git check-ignore -v ` before writing a credential anywhere, rather than trusting the pattern. - **A bare `*` 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](https://github.com/activepieces/activepieces/pull/14629)), and so did a single-page docs PR ([#14422](https://github.com/activepieces/activepieces/pull/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. +- **A long-lived branch that bumps a `packages/core/*` version will conflict with main in exactly two places, `package.json` and the workspace's `"version"` line in `bun.lock`, and the fix is the same both times.** Both sides bumped from the same base (main 0.7.0→0.8.0, the branch 0.7.0→0.7.1 on `core-utils` for #15707), so take main's number and apply the branch's own bump on top (0.8.1), write it into both files, then run `bun install`: it is a one-second no-op when `node_modules` is current and proves the lock matches the manifests. Do not take either side wholesale — main's lock alone leaves the branch's manifest unsatisfied, and the branch's alone drops main's dependency changes. Check the other core packages too: if main bumped `shared` or `execution` to the *same* number the branch chose, git auto-merges it silently and one bump then covers two releases. It happened on the very next sync of #15707: both sides took `shared` 0.176.0 → 0.177.0, the merge reported no conflict, and only a deliberate check caught it. So on every sync of a branch that touches `packages/core/*`, run `for f in packages/core/*/package.json; do echo "$f $(git show origin/main:$f | grep version) $(git show HEAD:$f | grep version)"; done` and re-bump any package where the two match but the branch changed it. +- **Answering and resolving review threads from the CLI takes two different ids.** A reply goes through REST with the inline comment's numeric id: `gh api repos///pulls//comments//replies -f body=…`. Resolving needs the *thread* id, which REST never returns: query GraphQL `pullRequest(number:) { reviewThreads(first:50) { nodes { id isResolved comments(first:1) { nodes { databaseId } } } } }`, match `databaseId` to the comment you replied to, then `mutation { resolveReviewThread(input:{threadId:"PRRT_…"}) { thread { isResolved } } }`. Greptile's summary links its findings as `discussion_r`, which is the same numeric id. Resolve only what actually changed; leave the thread for a deferred follow-up open as its tracker. +- **Reading the parallel test job's log: trust the turbo `Failed:` line at the tail, not the scary lines in the middle.** The job runs about 21 turbo tasks at once, so `gh run view --job --log-failed` interleaves all of them. `FATAL ERROR: Reached heap limit` and `Caught fatal signal 9` next to `test/lib/sandbox/*.test.ts` are the sandbox suite killing isolates on purpose; `Error: Boom inside engine trigger hook` and the `HttpError` stderr dumps are fixture output. Strip the prefixes (`sed 's/^[^\t]*\t[^\t]*\t//'`), grep ` FAIL ` for the real assertion, and read the `Tasks: … Failed: ` summary to know which package actually failed. Engine handler tests timing out at 20 s in that job are load, not hangs: `flow-skip-progress.test.ts` (28 s) and then `flow-rerun.test.ts` timed out on consecutive runs of #15707 while each finishes in 0.6 s locally. The CI step starts `turbo run test` and the API's `test-unit test-ce test-ee test-cloud` in parallel with `&` on a 4-vCPU runner, and a branch that touches `packages/core/*` invalidates about 60 downstream tasks, so the engine suites compete with four Postgres-backed API suites for the same cores; main mostly hits the turbo cache and never sees it. The engine's `testTimeout` is now 60 s, the value the API's vitest config already used for the same job; the real lever, if it recurs, is not running the two turbo invocations concurrently. - **A spurious `core` request on a pieces PR is not always the lockfile — check for a second root file.** [#14558](https://github.com/activepieces/activepieces/pull/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. -- **Greptile's Confidence Score prose is cumulative, so a low score mixes resolved findings with live ones — triage each claim separately rather than trusting or dismissing the number.** It edits one summary comment in place, and its "Files Needing Attention" list keeps naming findings that are already resolved and outdated: #14825 sat at 2/5 citing three files, two of which were a closed P1 and a duplicate view of the third. Read the *unresolved* review threads (`reviewThreads(first:60) { isResolved isOutdated }` over GraphQL — the REST comments endpoint carries no resolution state) and judge from those; re-trigger the review to refresh the score. It also re-raises the same class of finding each round with a new comment id, so a fix on one thread does not silence its sibling. **The score recomputes on a push, never on a reply** — so a finding Greptile *itself* withdraws still counts against it. On [#15243](https://github.com/activepieces/activepieces/pull/15243) the second pass read the pushed fix, filed zero new comments, and replied "this is not a valid finding… Withdrawing the comment" on its own P2; the summary comment was edited 67 seconds later, but only the footer (`Reviews (2)`, last-reviewed-commit link) changed — the headline stayed 2/5 and still justified itself with the finding just retracted. So compare the summary's `updated_at` against the review `submitted_at` from `gh api repos/:owner/:repo/pulls//reviews`: a summary newer than the last review pass has usually only had its footer bumped. But do not read "cumulative" as "stale": a 2/5 on [#14934](https://github.com/activepieces/activepieces/pull/14934) listed three files, two carrying findings already fixed and one naming a real bypass nobody had tested — the approve path published without the guard the direct path ran. The resolved-thread count is the right gate for *merging*, and a weak signal about *content*, because a finding Greptile states only in the summary never becomes a thread to resolve. Read the summary's file list even at zero unresolved threads, and check each named file against the code. -- **`api` and `worker` resolve `@activepieces/shared` and the `core-*` packages from `dist`, so a package-local `tsc --noEmit` there reports whatever was built last.** Both `packages/server/api/tsconfig.app.json` and `packages/server/worker/tsconfig.lib.json` set `"paths": {}`, clearing the inherited source mappings, so an export added to `packages/core/shared/src` shows up as `TS2305: has no exported member` until that package is rebuilt — the error names your file and is not about your file. `web` is not affected for the packages it actually imports: `vite.config.mts` aliases `@activepieces/shared`, `@activepieces/pieces-framework`, `core-utils`, `core-formula`, `core-piece-types` and `core-execution` to their `src`. Nothing else is aliased there, so check `vite.config.mts` before assuming a package is exempt — `@activepieces/ai-providers` (note the name: no `core-` prefix) has no Vite alias and resolves through `dist`, though it is mapped to source in `tsconfig.base.json` and web does not import it today. `npx turbo run build --filter=` needs no manual pre-build, since `build` declares `dependsOn: ["^build"]` and builds dependencies first; only the direct `tsc` invocation can lie, and `npx turbo run build --filter=@activepieces/shared` is the cure. -- **The `main` CI job builds web, worker, api and engine before it runs a single test, so a build error there fails the whole job with no test output to point at it.** One missing enum import in a worker file surfaced only as `worker#build` exiting 2. Before pushing a change that moves code between files, run `npx turbo run build --filter=web --filter=worker --filter=api --filter=@activepieces/engine` — the same set the job builds — rather than trusting the per-package typecheck you happened to run before the move. +- **Greptile's Confidence Score prose is cumulative, so a low score mixes resolved findings with live ones — triage each claim separately rather than trusting or dismissing the number.** It edits one summary comment in place, and its "Files Needing Attention" list keeps naming findings that are already resolved and outdated: #14825 sat at 2/5 citing three files, two of which were a closed P1 and a duplicate view of the third. Read the *unresolved* review threads (`reviewThreads(first:60) { isResolved isOutdated }` over GraphQL — the REST comments endpoint carries no resolution state) and judge from those; re-trigger the review to refresh the score. It also re-raises the same class of finding each round with a new comment id, so a fix on one thread does not silence its sibling. **The score recomputes on a push, never on a reply** — so a finding Greptile *itself* withdraws still counts against it. On [#15243](https://github.com/activepieces/activepieces/pull/15243) the second pass read the pushed fix, filed zero new comments, and replied "this is not a valid finding… Withdrawing the comment" on its own P2; the summary comment was edited 67 seconds later, but only the footer (`Reviews (2)`, last-reviewed-commit link) changed — the headline stayed 2/5 and still justified itself with the finding just retracted. So compare the summary's `updated_at` against the review `submitted_at` from `gh api repos/:owner/:repo/pulls//reviews`: a summary newer than the last review pass has usually only had its footer bumped. But do not read "cumulative" as "stale": a 2/5 on [#14934](https://github.com/activepieces/activepieces/pull/14934) listed three files, two carrying findings already fixed and one naming a real bypass nobody had tested — the approve path published without the guard the direct path ran. The resolved-thread count is the right gate for *merging*, and a weak signal about *content*, because a finding Greptile states only in the summary never becomes a thread to resolve. Read the summary's file list even at zero unresolved threads, and check each named file against the code. The same applies to a human reviewer's tool-generated summary: Amr's review on #15707 said "Blockers: 4" and named them, but only two had an inline thread; the other two never left his tool. Count the threads against the summary before triaging, and answer the missing ones in a PR comment so the record shows they were checked. +- **`api` and `worker` resolve `@activepieces/shared` and the `core-*` packages from `dist`, so a package-local `tsc --noEmit` there reports whatever was built last.** Both `packages/server/api/tsconfig.app.json` and `packages/server/worker/tsconfig.lib.json` set `"paths": {}`, clearing the inherited source mappings, so an export added to `packages/core/shared/src` shows up as `TS2305: has no exported member` until that package is rebuilt — the error names your file and is not about your file. `web` is not affected for the packages it actually imports: `vite.config.mts` aliases `@activepieces/shared`, `@activepieces/pieces-framework`, `core-utils`, `core-formula`, `core-piece-types` and `core-execution` to their `src`. Nothing else is aliased there, so check `vite.config.mts` before assuming a package is exempt — `@activepieces/ai-providers` (note the name: no `core-` prefix) has no Vite alias and resolves through `dist`, though it is mapped to source in `tsconfig.base.json` and web does not import it today. `npx turbo run build --filter=` needs no manual pre-build, since `build` declares `dependsOn: ["^build"]` and builds dependencies first; only the direct `tsc` invocation can lie, and `npx turbo run build --filter=@activepieces/shared` is the cure. **The web editor and `tsc` are affected too, one hop out.** `packages/web/tsconfig.app.json` maps only `@activepieces/shared`, `pieces-framework` and a few pieces to `src`; `shared/src` then imports `@activepieces/core-utils`, which resolves through `node_modules` to `dist`. So after a sync that changes a `core-utils` schema (a `sessionToken` field on the Bedrock auth, #15747), the editor reports `'sessionToken' does not exist in type` on main's own web code while `vite` is fine. Rebuild the core packages and restart the TS server; the code needs no change. +- **The `main` CI job builds web, worker, api and engine before it runs a single test, so a build error there fails the whole job with no test output to point at it.** One missing enum import in a worker file surfaced only as `worker#build` exiting 2. Before pushing a change that moves code between files, run `npx turbo run build --filter=web --filter=worker --filter=api --filter=@activepieces/engine` — the same set the job builds — rather than trusting the per-package typecheck you happened to run before the move. Note that the `web` half of that command needs more heap than node gives it by default: on a 14 GB machine `vite build` transforms all ~7k modules and then dies with `node::OOMErrorHandler` / `SIGABRT` during bundling, which reads as a broken change rather than a full laptop. Run it as `NODE_OPTIONS=--max-old-space-size=8192 npx turbo run build --filter=web` and it finishes in well under a minute. CI runners have the headroom, so this only ever bites locally. - **`check-migrations` runs against your own PGlite database, so a leftover index from work you abandoned fails the gate locally while CI is green.** The check runs migrations then asks TypeORM to generate one, and any difference between your database and the entity metadata counts as drift — including an index a reverted branch created and never dropped. The report names it (`DROP INDEX "public".""` in the generated `up`). CI starts from an empty database, so this class of failure is local-only. The database is `~/.activepieces/pglite` by default, which is also the dev server's, so drop the specific index rather than resetting the directory: a tiny CJS script with `PGlite.create({ dataDir })` and `DROP INDEX IF EXISTS` is enough, and it must run inside `packages/server/api` where the dependency resolves. - **A red check does not block a merge.** The gate only prevents merges once `PR size` is added as a **required status check** for `main` in branch protection. Until then it is visible but advisory. - **A workflow that opens a PR must authenticate with `secrets.CROWDIN_PRS`, not `GITHUB_TOKEN`.** Despite the name, that PAT is this repo's open-a-PR-as-a-bot token: `crowdin-pr-merger.yml`, `reusable-finalize-translations-pr.yml` and — the tell — `release-self-hosted.yml`, which has nothing to do with Crowdin and uses it for both `actions/checkout`'s `token:` and `gh pr create`'s `GH_TOKEN`. Those jobs declare only `permissions: contents: read`, because the PAT does the pushing and the PR-opening; raising `GITHUB_TOKEN` to `contents: write` / `pull-requests: write` instead is treating the symptom, since *Allow GitHub Actions to create and approve pull requests* is evidently off for the org (not readable without `admin:org`). The failure mode is nasty because it is half-done and unattended: the branch pushes fine and only `pulls.create` fails, leaving an orphan `auto/*` branch every scheduled run. Copy `release-self-hosted.yml`, and have the job delete its own branch on failure so a bad week retries clean instead of accumulating. diff --git a/brain/knowledge/engineering/engineering-handbook-playbooks.md b/brain/knowledge/engineering/engineering-handbook-playbooks.md index 081cf17d5f40..649ee54a6301 100644 --- a/brain/knowledge/engineering/engineering-handbook-playbooks.md +++ b/brain/knowledge/engineering/engineering-handbook-playbooks.md @@ -24,3 +24,4 @@ Postmortems (Redis/queue overload, infra upgrade — March 2026) and product int - **Running EE locally (outside the dev container) to preview a chat/agents-gated view hits four macOS-specific walls the [Run EE](https://www.activepieces.com/docs/handbook/engineering/playbooks/run-ee) playbook doesn't cover.** (1) The default dev DB, PGlite (`.env.dev`'s `AP_DB_TYPE=PGLITE`), hard-refuses `AP_EDITION=ee`/`cloud` with `Error: Edition ee not supported in pglite mode in dev environment` — point at a real local Postgres instead (`AP_DB_TYPE=POSTGRES` + a throwaway DB via `createdb`). (2) In a fresh git worktree, symlinking `node_modules` from another worktree/checkout works for lint/typecheck but not for actually running the server: workspace packages resolve through the symlink to whatever commit *that* checkout is on, so a genuine cross-branch source/type skew appears (e.g. `AIProviderName.VERTEX does not exist`) purely from the mismatch — do a real `bun install` scoped to the worktree instead. (3) `bun install` aborts the *entire* install (not just its own package) if `isolated-vm`'s native build fails, which it does on Darwin 25 (`node-gyp` / `undici` `webidl.util.markAsUncloneable is not a function`) — even though `isolated-vm` is only needed by the engine/worker sandbox, not by `api`+`web`. Fix: `bun install --ignore-scripts`, then `cd` into `node_modules/.bun/sqlite3@*/node_modules/sqlite3` and run `npx prebuild-install -r napi` directly to fetch its prebuilt native binding (something in the queue/api dependency tree requires it to *load* even when the configured DB isn't sqlite). (4) The repo requires Node 22.15+/24 (`bootstrap.ts` uses `zlib.zstdDecompress`, added in Node 22.15); a system default of Node 20 (common via `nvm`) crashes API boot with `ERR_INVALID_ARG_TYPE` in `file-compressor.ts` — put a `nvm`-installed v24 first on `PATH` before starting `serve`. Once booted: `AP_EDITION=ee` alone (no license key) is enough to preview chat/agents, since the unlicensed `AUTUMN_FREE_PLAN` already sets `chatEnabled`/`agentsEnabled: true` — but that plan's `aiProvidersEnabled: false` hides the "Connect" button on every provider card, so there is no UI path to satisfy chat's "Set up an AI provider" gate; insert a throwaway row into `ai_provider` directly via SQL (`enabledForChat: true`, `config: '{}'`, any `auth` JSON — it's only decrypted when a message is actually sent) to get past it for pure layout/UI verification. - **`DISCORD_ON_CALL_WEBHOOK` is the single repo secret behind every on-call Discord notification** — cloud production deploys (`continuous-delivery-cloud.yml`), self-hosted releases (`release-self-hosted.yml`), and release-pieces failure alerts all post through it. Rotating it repoints all of them at once; there is no per-workflow webhook. Self-hosted release notifications skip `-rc` tags unless `publish_rc_release` is set, mirroring the release-drafter condition so the message never links a release page that doesn't exist. - **Docs media lives on the CDN, never in the repo — and videos and images sit on two different paths.** A video embeds as `