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
3 changes: 2 additions & 1 deletion brain/knowledge/engineering/server-module-anatomy.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ Verify with `npm run lint-dev` and `npm run test-api`.
- **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. When the collision does surface in a merge, renumber **yours** — the one on `main` is already applied in production and cannot move — which means renaming the file, the class, and the class's `name` field, then re-registering it after the merged one in `getMigrations()`. Whoever already ran the old name locally needs no DB surgery *provided* `up()` is idempotent (`IF NOT EXISTS` / `DROP … IF EXISTS` throughout): TypeORM sees an unapplied name and re-runs it as a no-op. Without that, they have to update the `migrations` ledger row by hand.
- **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.
- **`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 PGlite** (`.env.tests` sets `AP_DB_TYPE=PGLITE`), so they do exercise this class by default; it was still missed because nothing asserted on the guard. Earlier revisions of this page claimed the suite ran against a real Postgres server, which is wrong. 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.
- **No concurrency property can be tested in the api integration suite.** `.env.tests` runs PGlite, one in-process connection, so a second session cannot exist: `SELECT … FOR UPDATE` held from the test blocks nothing, two `Promise.all` requests serialise before either transaction opens, and a lost update is unobservable. A test written for a race there passes with the lock removed, which reads as proof and is the opposite. Measured Aug 2026 while adding a row lock to the agent draft-tools edit: deleting `setLock('pessimistic_write')` left all 13 tests green. So pin the user-visible invariant, mutation-test the parts that *are* observable (a name in a denylist, a guard's SQL), and say plainly in the commit that the lock rests on Postgres row semantics rather than a reproduced race. Row locks use `.createQueryBuilder().setLock('pessimistic_write')` inside `transaction(...)` — three of the four sites in the repo take that form.
- **`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.
- **`permission: undefined` on `securityAccess.project(...)` silently allows any project member.** The argument is required in practice even though the type tolerates omitting it.
Expand Down
2 changes: 2 additions & 0 deletions packages/core/execution/src/lib/workers/worker-contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export type GetAgentConfigRequest = {
files?: Array<{ name: string, mimeType: string, data: string }>
promptOverride?: AgentPromptOverride
dryRun?: boolean
discoveryOnly?: boolean
}

export type ResolvedAiToolConfig = {
Expand Down Expand Up @@ -158,6 +159,7 @@ export type AgentConfigResponse = {
guides: Record<string, string>
aiTools: AgentAiToolsConfig
emailEnabled: boolean
agentsAvailable: boolean
userEmail: string
source: AgentRunSource
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/shared/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@activepieces/shared",
"version": "0.142.0",
"version": "0.143.0",
"type": "commonjs",
"sideEffects": false,
"main": "./dist/src/index.js",
Expand Down
1 change: 1 addition & 0 deletions packages/core/shared/src/lib/ee/agent/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const Agent = z.object({
})

const AgentSummary = Agent.omit({ draft: true, published: true }).extend({
isPublished: z.boolean(),
toolCount: z.number(),
toolPieceNames: z.array(z.string()),
projectDisplayName: z.string(),
Expand Down
6 changes: 6 additions & 0 deletions packages/core/shared/test/ee/agent-tool-phases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ describe('agentToolPhases.activeToolsForPhase', () => {
}
})

it('leaves the agent tools reachable in discovery, since nothing else flips the phase for them', () => {
const names = ['ap_list_agents', 'ap_create_agent', 'ap_update_agent', 'ap_add_agent_tool', 'ap_remove_agent_tool']
const active = agentToolPhases.activeToolsForPhase({ phase: 'discovery', allToolNames: names })
expect(active).toEqual(names)
})

it('leaves unknown tools visible during discovery (denylist, not allowlist)', () => {
const active = agentToolPhases.activeToolsForPhase({ phase: 'discovery', allToolNames: ['ap_some_new_tool'] })
expect(active).toContain('ap_some_new_tool')
Expand Down
16 changes: 16 additions & 0 deletions packages/server/api/src/app/ee/agent/agent-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ import { aiProviderService, ProviderScope } from '../../ai/ai-provider-service'
import { repoFactory } from '../../core/db/repo-factory'
import { transaction } from '../../core/db/transaction'
import { redisConnections } from '../../database/redis-connections'
import { system } from '../../helper/system/system'
import { AppSystemProp } from '../../helper/system/system-props'
import { projectService } from '../../project/project-service'
import { userService } from '../../user/user-service'
import { platformPlanService } from '../platform/platform-plan/platform-plan.service'
import { AgentConversationEntity, AgentConversationWithRelations } from './agent-conversation-entity'
import { UserMemoryEntity } from './user-memory-entity'

Expand Down Expand Up @@ -319,7 +322,20 @@ async function saveUserMemory({ platformId, userId, instructions, memories, base
})
}

async function agentsSurfaceAvailable({ platformId, log }: { platformId: string, log: FastifyBaseLogger }): Promise<boolean> {
if (system.getBoolean(AppSystemProp.AGENTS_ENABLED) !== true) {
return false
}
const { data: plan, error } = await tryCatch(() => platformPlanService(log).getOrCreateForPlatform(platformId))
if (!isNil(error) || isNil(plan)) {
log.error({ error, platform: { id: platformId } }, '[agentHelpers#agentsSurfaceAvailable] Could not read the plan, treating agents as unavailable')
return false
}
return plan.agentsEnabled
}

export const agentHelpers = {
agentsSurfaceAvailable,
getConversationOrThrow,
getUserProjects,
resolveChatProvider,
Expand Down
12 changes: 8 additions & 4 deletions packages/server/api/src/app/ee/agent/agent-rpc-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const CHAT_ONLY_TOOL_PREFIX = '__'
const OWNER_SCOPED_TOOLS = ['ap_remember']
const ATTENDED_STATE_TOOLS = ['__cancel_check', '__approval_wait', '__store_pending_gate', '__store_selected_connection']
const CONFIGURED_TOOL_SOURCES: AgentRunSource[] = [AgentRunSource.FLOW_STEP, AgentRunSource.AGENT]
const UNATTENDED_FORBIDDEN_TOOLS = ['ap_run_code', 'ap_execute_action', 'ap_explore_data', 'ap_list_across_projects']
const UNATTENDED_FORBIDDEN_TOOLS = ['ap_run_code', 'ap_execute_action', 'ap_explore_data', 'ap_list_across_projects', 'ap_list_agents', 'ap_create_agent', 'ap_update_agent', 'ap_add_agent_tool', 'ap_remove_agent_tool']
const KNOWLEDGE_BASE_SEARCH_LIMIT = 5
const KNOWLEDGE_BASE_SIMILARITY_THRESHOLD = 0.5

Expand Down Expand Up @@ -73,7 +73,7 @@ async function updateConversationForRun({ conversationId, runId, updates }: {

export const agentRpcHandlers = (log: FastifyBaseLogger) => ({
async getAgentConfig(input: GetAgentConfigRequest): Promise<AgentConfigResponse> {
const { conversationId, platformId, userId, userMessage, modelName, files, promptOverride, dryRun, source: requestedSource, projectId: requestedProjectId } = input
const { conversationId, platformId, userId, userMessage, modelName, files, promptOverride, dryRun, discoveryOnly, source: requestedSource, projectId: requestedProjectId } = input

// A flow-step run gets none of the owner's chat context, so it is not fetched. Reading it
// anyway meant an owner without an MCP token or a user record failed the run outright.
Expand Down Expand Up @@ -127,7 +127,9 @@ export const agentRpcHandlers = (log: FastifyBaseLogger) => ({
const userContent = await buildUserContentWithFiles({ text: userMessage, files, attachmentNote: buildAttachmentNote(attachmentRefs) })

const aiTools: GetEnabledAiToolsResponse = dryRun ? {} : enabledAiTools
const emailEnabled = !dryRun && carriesChatContext && smtpEmailSender(log).isSmtpConfigured()
const actingRun = !dryRun && !discoveryOnly
const emailEnabled = actingRun && carriesChatContext && smtpEmailSender(log).isSmtpConfigured()
const agentsAvailable = actingRun && carriesChatContext && await agentHelpers.agentsSurfaceAvailable({ platformId, log })
const fetchAvailable = !dryRun
// Tavily takes precedence over native LLM search; native is only the no-Tavily fallback.
const tavilySearchAvailable = !isNil(aiTools.webSearch)
Expand Down Expand Up @@ -194,8 +196,9 @@ export const agentRpcHandlers = (log: FastifyBaseLogger) => ({
searchAvailable: webSearchAvailable,
fetchAvailable,
scrapeAvailable: fetchAvailable && !isNil(aiTools.webScraping),
imageAvailable: fetchAvailable && !isNil(aiTools.imageGeneration),
imageAvailable: actingRun && !isNil(aiTools.imageGeneration),
emailAvailable: emailEnabled,
agentsAvailable,
userEmail: runUserEmail,
connections: inventoryResult && !inventoryResult.error
? { connections: inventoryResult.data.data, truncated: inventoryResult.data.data.length >= CONNECTION_INVENTORY_LIMIT }
Expand Down Expand Up @@ -288,6 +291,7 @@ export const agentRpcHandlers = (log: FastifyBaseLogger) => ({
guides,
aiTools,
emailEnabled,
agentsAvailable,
userEmail: runUserEmail,
source: conversation.source,
}
Expand Down
25 changes: 25 additions & 0 deletions packages/server/api/src/app/ee/agent/agent-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Agent, AgentConfig, AgentSummary, agentUtils, AgentVisibility, CreateAg
import { FastifyBaseLogger } from 'fastify'
import { Brackets, In, SelectQueryBuilder } from 'typeorm'
import { repoFactory } from '../../core/db/repo-factory'
import { transaction } from '../../core/db/transaction'
import { buildPaginator } from '../../helper/pagination/build-paginator'
import { paginationHelper } from '../../helper/pagination/pagination-utils'
import { projectService } from '../../project/project-service'
Expand Down Expand Up @@ -143,6 +144,25 @@ export const agentService = (log: FastifyBaseLogger) => ({
return this.getOneOrThrow({ id, projectId, userId })
},

async editDraftTools({ id, projectId, userId, edit }: EditDraftToolsParams): Promise<Agent | null> {
return transaction(async (entityManager) => {
const repo = entityManager.getRepository(AgentEntity)
const agent = await repo.createQueryBuilder('agent')
.setLock('pessimistic_write')
.where('agent.id = :id AND agent."projectId" = :projectId', { id, projectId })
.getOne()
if (isNil(agent)) {
return null
}
const tools = edit(agent.draft.tools)
if (isNil(tools)) {
return null
}
await repo.save({ ...omit(agent, ['published']), draft: sanitizeObjectForPostgresql({ ...agent.draft, tools }) })
return this.getOneOrThrow({ id, projectId, userId })
})
},

async delete({ id, projectId, userId }: GetParams): Promise<Agent> {
const agent = await this.getOneOrThrow({ id, projectId, userId })
await agentRepo().delete({ id, projectId })
Expand Down Expand Up @@ -233,6 +253,7 @@ async function resolveReadableProjects({ platformId, userId, projectId, log }: R
function toSummary(agent: Agent, project?: Project): AgentSummary {
return {
...omit(agent, ['draft', 'published']),
isPublished: !isNil(agent.published),
projectDisplayName: project?.displayName ?? '',
projectIsPrivate: project?.type === ProjectType.PERSONAL,
toolCount: agent.draft.tools.length,
Expand Down Expand Up @@ -285,6 +306,10 @@ type ListParams = {
limit?: number
}

type EditDraftToolsParams = GetParams & {
edit: (tools: AgentConfig['tools']) => AgentConfig['tools'] | null
}

type GetParams = {
id: ApId
projectId: ProjectId
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { isNil } from '@activepieces/core-utils'
import { AgentRunSource } from '@activepieces/shared'

function buildRunNotes({ source, messageSource, currentDate, searchAvailable, fetchAvailable, scrapeAvailable, imageAvailable, emailAvailable, userEmail, connections, memory }: {
function buildRunNotes({ source, messageSource, currentDate, searchAvailable, fetchAvailable, scrapeAvailable, imageAvailable, emailAvailable, agentsAvailable, userEmail, connections, memory }: {
source: AgentRunSource
messageSource?: 'onboarding'
currentDate: string
Expand All @@ -10,6 +10,7 @@ function buildRunNotes({ source, messageSource, currentDate, searchAvailable, fe
scrapeAvailable: boolean
imageAvailable: boolean
emailAvailable: boolean
agentsAvailable: boolean
userEmail: string
connections: ConnectionInventory | null
memory: RunMemory
Expand All @@ -24,6 +25,7 @@ function buildRunNotes({ source, messageSource, currentDate, searchAvailable, fe
emailAvailable: emailAvailable && isChat,
userEmail,
})
+ (isChat && agentsAvailable ? AGENTS_NOTE : '')
+ (isChat && !isNil(connections) ? buildConnectionInventoryNote(connections) : '')
+ (isChat ? buildMemoryNote(memory) : '')
+ (isChat && messageSource === 'onboarding' ? ONBOARDING_FIRST_MESSAGE_NOTE : '')
Expand Down Expand Up @@ -127,6 +129,13 @@ function buildMemoryNote({ instructions, memories }: RunMemory): string {

export const agentSurfaceNotes = { buildRunNotes }

const AGENTS_NOTE = [
'\n\n## Saved agents',
'This project can hold saved agents: named, reusable agents with their own instructions and tools, which the user can chat with and reuse.',
'Offer one when the user describes something recurring they will run again or across several flows, rather than a single automation. A one-off automation is still a flow.',
'What you edit is the draft; what runs unattended is the published version. Publish only when the user asks to make changes live, and do it with the `publish` flag on the edit rather than a separate publish call.',
].join('\n')

type ConnectionInventory = {
connections: { displayName: string, pieceName: string, status: string }[]
truncated: boolean
Expand Down
Loading
Loading