From 4a82a2fa71a0f3356865ff913947f88109494eb7 Mon Sep 17 00:00:00 2001 From: Hazem Adel Date: Wed, 26 Aug 2026 05:34:48 +0300 Subject: [PATCH 1/2] feat(agents): chat can create, change and equip saved agents (#15035) --- .../engineering/server-module-anatomy.md | 3 +- .../src/lib/workers/worker-contract.ts | 2 + packages/core/shared/package.json | 2 +- .../core/shared/src/lib/ee/agent/agent.ts | 1 + .../shared/test/ee/agent-tool-phases.test.ts | 6 + .../api/src/app/ee/agent/agent-helpers.ts | 16 + .../src/app/ee/agent/agent-rpc-handlers.ts | 12 +- .../api/src/app/ee/agent/agent-service.ts | 25 ++ .../ee/agent/prompt/agent-surface-notes.ts | 11 +- .../api/src/app/ee/agent/tools/agent-tools.ts | 208 +++++++++++- .../ee/agent/chat-agent-tools.test.ts | 321 ++++++++++++++++++ .../app/ee/agent/agent-surface-notes.test.ts | 11 +- .../jobs/ee/agent/agent-tool-policy.ts | 2 + .../jobs/ee/agent/agent-worker-tools.ts | 67 ++++ .../jobs/ee/agent/execute-agent-run.ts | 9 +- .../jobs/ee/agent/agent-tool-policy.test.ts | 14 + 16 files changed, 699 insertions(+), 11 deletions(-) create mode 100644 packages/server/api/test/integration/ee/agent/chat-agent-tools.test.ts diff --git a/brain/knowledge/engineering/server-module-anatomy.md b/brain/knowledge/engineering/server-module-anatomy.md index 461b1946adee..b2c2b1695a55 100644 --- a/brain/knowledge/engineering/server-module-anatomy.md +++ b/brain/knowledge/engineering/server-module-anatomy.md @@ -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. diff --git a/packages/core/execution/src/lib/workers/worker-contract.ts b/packages/core/execution/src/lib/workers/worker-contract.ts index f5c89e627a39..e234056a8f03 100644 --- a/packages/core/execution/src/lib/workers/worker-contract.ts +++ b/packages/core/execution/src/lib/workers/worker-contract.ts @@ -127,6 +127,7 @@ export type GetAgentConfigRequest = { files?: Array<{ name: string, mimeType: string, data: string }> promptOverride?: AgentPromptOverride dryRun?: boolean + discoveryOnly?: boolean } export type ResolvedAiToolConfig = { @@ -158,6 +159,7 @@ export type AgentConfigResponse = { guides: Record aiTools: AgentAiToolsConfig emailEnabled: boolean + agentsAvailable: boolean userEmail: string source: AgentRunSource } diff --git a/packages/core/shared/package.json b/packages/core/shared/package.json index 8300baf37148..362069b51786 100644 --- a/packages/core/shared/package.json +++ b/packages/core/shared/package.json @@ -1,6 +1,6 @@ { "name": "@activepieces/shared", - "version": "0.142.0", + "version": "0.143.0", "type": "commonjs", "sideEffects": false, "main": "./dist/src/index.js", diff --git a/packages/core/shared/src/lib/ee/agent/agent.ts b/packages/core/shared/src/lib/ee/agent/agent.ts index 6764342e8392..09d393c17f29 100644 --- a/packages/core/shared/src/lib/ee/agent/agent.ts +++ b/packages/core/shared/src/lib/ee/agent/agent.ts @@ -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(), diff --git a/packages/core/shared/test/ee/agent-tool-phases.test.ts b/packages/core/shared/test/ee/agent-tool-phases.test.ts index ae3fc1feea56..7bcad8cc2d99 100644 --- a/packages/core/shared/test/ee/agent-tool-phases.test.ts +++ b/packages/core/shared/test/ee/agent-tool-phases.test.ts @@ -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') diff --git a/packages/server/api/src/app/ee/agent/agent-helpers.ts b/packages/server/api/src/app/ee/agent/agent-helpers.ts index 50b43a9fe9e9..1d2e8ee74efe 100644 --- a/packages/server/api/src/app/ee/agent/agent-helpers.ts +++ b/packages/server/api/src/app/ee/agent/agent-helpers.ts @@ -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' @@ -319,7 +322,20 @@ async function saveUserMemory({ platformId, userId, instructions, memories, base }) } +async function agentsSurfaceAvailable({ platformId, log }: { platformId: string, log: FastifyBaseLogger }): Promise { + 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, diff --git a/packages/server/api/src/app/ee/agent/agent-rpc-handlers.ts b/packages/server/api/src/app/ee/agent/agent-rpc-handlers.ts index 66b8b50cb737..6033a60f06f3 100644 --- a/packages/server/api/src/app/ee/agent/agent-rpc-handlers.ts +++ b/packages/server/api/src/app/ee/agent/agent-rpc-handlers.ts @@ -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 @@ -73,7 +73,7 @@ async function updateConversationForRun({ conversationId, runId, updates }: { export const agentRpcHandlers = (log: FastifyBaseLogger) => ({ async getAgentConfig(input: GetAgentConfigRequest): Promise { - 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. @@ -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) @@ -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 } @@ -288,6 +291,7 @@ export const agentRpcHandlers = (log: FastifyBaseLogger) => ({ guides, aiTools, emailEnabled, + agentsAvailable, userEmail: runUserEmail, source: conversation.source, } diff --git a/packages/server/api/src/app/ee/agent/agent-service.ts b/packages/server/api/src/app/ee/agent/agent-service.ts index 3b7de68a9dc1..0e152118a262 100644 --- a/packages/server/api/src/app/ee/agent/agent-service.ts +++ b/packages/server/api/src/app/ee/agent/agent-service.ts @@ -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' @@ -143,6 +144,25 @@ export const agentService = (log: FastifyBaseLogger) => ({ return this.getOneOrThrow({ id, projectId, userId }) }, + async editDraftTools({ id, projectId, userId, edit }: EditDraftToolsParams): Promise { + 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 { const agent = await this.getOneOrThrow({ id, projectId, userId }) await agentRepo().delete({ id, projectId }) @@ -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, @@ -285,6 +306,10 @@ type ListParams = { limit?: number } +type EditDraftToolsParams = GetParams & { + edit: (tools: AgentConfig['tools']) => AgentConfig['tools'] | null +} + type GetParams = { id: ApId projectId: ProjectId diff --git a/packages/server/api/src/app/ee/agent/prompt/agent-surface-notes.ts b/packages/server/api/src/app/ee/agent/prompt/agent-surface-notes.ts index e5bcd3ea6e31..15634ec8620f 100644 --- a/packages/server/api/src/app/ee/agent/prompt/agent-surface-notes.ts +++ b/packages/server/api/src/app/ee/agent/prompt/agent-surface-notes.ts @@ -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 @@ -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 @@ -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 : '') @@ -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 diff --git a/packages/server/api/src/app/ee/agent/tools/agent-tools.ts b/packages/server/api/src/app/ee/agent/tools/agent-tools.ts index 786390543574..680c696d8d47 100644 --- a/packages/server/api/src/app/ee/agent/tools/agent-tools.ts +++ b/packages/server/api/src/app/ee/agent/tools/agent-tools.ts @@ -1,12 +1,13 @@ import { isNil, isObject, isString, parseToJsonIfPossible, Permission, spreadIfDefined, tryCatch } from '@activepieces/core-utils' import { agentAiUtils } from '@activepieces/server-utils' -import { agentToolClassification, AppConnectionStatus, AppConnectionType, FileCompression, FileType, FlowRunStatus, FlowStatus, Project, RunEnvironment } from '@activepieces/shared' +import { Agent, AgentIcon, AgentTool, agentToolClassification, AgentToolType, AppConnectionStatus, AppConnectionType, ColorName, DEFAULT_AGENT_MAX_STEPS, FileCompression, FileType, FlowRunStatus, FlowStatus, Project, RunEnvironment } from '@activepieces/shared' import { FastifyBaseLogger } from 'fastify' import { appConnectionService } from '../../../app-connection/app-connection-service/app-connection-service' import { fileService } from '../../../file/file.service' import { filesService } from '../../../file/files-service' import { flowService } from '../../../flows/flow/flow.service' import { flowRunService } from '../../../flows/flow-run/flow-run-service' +import { domainHelper } from '../../../helper/domain-helper' import { resolvePermissionChecker } from '../../../mcp/mcp-permissions' import { formatFlowLine } from '../../../mcp/tools/ap-list-flows' import { runActionInput } from '../../../mcp/tools/ap-run-action' @@ -17,8 +18,10 @@ import { tableService } from '../../../tables/table/table.service' import { agentApprovalGate } from '../agent-approval-gate' import { agentHelpers } from '../agent-helpers' import { agentMemoryAi } from '../agent-memory-ai' +import { agentService } from '../agent-service' import { agentPrompt } from '../prompt/agent-prompt' +const AGENT_LIST_LIMIT = 50 const CROSS_PROJECT_CONNECTION_LIMIT = 100 const OAUTH_TYPES: ReadonlySet = new Set([ AppConnectionType.OAUTH2, @@ -212,6 +215,168 @@ async function listResourceForProject({ resource, projectId, status, log }: { } } +function nonEmpty(value: unknown): string | undefined { + return isString(value) && value.trim().length > 0 ? value.trim() : undefined +} + +async function createAgentFromChat({ toolInput, projectId, userId, log }: { + toolInput: Record + projectId: string + userId: string + log: FastifyBaseLogger +}): Promise { + const displayName = nonEmpty(toolInput.displayName) + const instructions = nonEmpty(toolInput.instructions) + if (isNil(displayName) || isNil(instructions)) { + return { error: 'An agent needs a name and instructions.' } + } + const agent = await agentService(log).create({ + projectId, + ownerId: userId, + request: { + projectId, + displayName, + description: nonEmpty(toolInput.description) ?? null, + icon: AgentIcon.SPARKLES, + color: ColorName.PURPLE, + draft: { instructions, maxSteps: DEFAULT_AGENT_MAX_STEPS, tools: [], structuredOutput: [] }, + }, + }) + return afterDraftChange({ agent, publish: false, projectId, userId, log }) +} + +async function updateAgentFromChat({ toolInput, agent, projectId, userId, log }: { + toolInput: Record + agent: Agent + projectId: string + userId: string + log: FastifyBaseLogger +}): Promise { + const displayName = nonEmpty(toolInput.displayName) + const description = nonEmpty(toolInput.description) + const instructions = nonEmpty(toolInput.instructions) + const publish = toolInput.publish === true + if (isNil(displayName) && isNil(description) && isNil(instructions) && !publish) { + return { error: 'Nothing to change. Pass a new displayName, description or instructions, and none of them may be blank.' } + } + const updated = await agentService(log).update({ + id: agent.id, + projectId, + userId, + request: { + ...spreadIfDefined('displayName', displayName), + ...spreadIfDefined('description', description), + ...(isNil(instructions) ? {} : { draft: { ...agent.draft, instructions } }), + }, + }) + return afterDraftChange({ agent: updated, publish, projectId, userId, log }) +} + +async function addAgentToolFromChat({ toolInput, agent, projectId, platformId, userId, log }: { + toolInput: Record + agent: Agent + projectId: string + platformId: string + userId: string + log: FastifyBaseLogger +}): Promise { + const pieceName = nonEmpty(toolInput.pieceName) + const actionNames = toolNamesFrom(toolInput) + if (isNil(pieceName) || actionNames.length === 0) { + return { error: 'Adding tools needs the piece name and at least one action name.' } + } + const normalizedPiece = mcpUtils.normalizePieceName(pieceName) ?? pieceName + const piece = await pieceMetadataService(log).get({ name: normalizedPiece, projectId, platformId }) + const missing = actionNames.filter((actionName) => isNil(piece?.actions[actionName])) + if (isNil(piece) || missing.length > 0) { + return { error: `${normalizedPiece} has no action called ${missing.join(' or ')}. Look it up with ap_research_pieces before adding it.` } + } + const connectionExternalId = nonEmpty(toolInput.connectionExternalId) + if (!isNil(connectionExternalId)) { + const connection = await appConnectionService(log).getOneWithoutValue({ projectId, platformId, externalId: connectionExternalId }) + if (isNil(connection)) { + return { error: 'No connection with that externalId in this project. Call ap_list_connections and pass one of those.' } + } + if (mcpUtils.normalizePieceName(connection.pieceName) !== normalizedPiece) { + return { error: `That connection is for ${connection.pieceName}, not ${normalizedPiece}. Pass a connection for the same app.` } + } + } + const added: AgentTool[] = actionNames.map((actionName) => ({ + type: AgentToolType.PIECE, + toolName: actionName, + pieceMetadata: { + pieceName: normalizedPiece, + pieceVersion: piece.version, + actionName, + ...(isNil(connectionExternalId) ? {} : { predefinedInput: { auth: connectionExternalId, fields: {} } }), + }, + })) + const updated = await agentService(log).editDraftTools({ + id: agent.id, + projectId, + userId, + edit: (tools) => tools.some((tool) => actionNames.includes(tool.toolName)) ? null : [...tools, ...added], + }) + if (isNil(updated)) { + return { error: `${agent.displayName} already has one of those tools. List them with ap_list_agents before adding.` } + } + return afterDraftChange({ agent: updated, publish: toolInput.publish === true, projectId, userId, log }) +} + +async function removeAgentToolFromChat({ toolInput, agent, projectId, userId, log }: { + toolInput: Record + agent: Agent + projectId: string + userId: string + log: FastifyBaseLogger +}): Promise { + const actionNames = toolNamesFrom(toolInput) + if (actionNames.length === 0) { + return { error: 'Removing tools needs at least one action name.' } + } + const updated = await agentService(log).editDraftTools({ + id: agent.id, + projectId, + userId, + edit: (tools) => tools.some((tool) => actionNames.includes(tool.toolName)) + ? tools.filter((tool) => !actionNames.includes(tool.toolName)) + : null, + }) + if (isNil(updated)) { + return { error: `${agent.displayName} has none of those tools, so there is nothing to remove.` } + } + return afterDraftChange({ agent: updated, publish: toolInput.publish === true, projectId, userId, log }) +} + +function toolNamesFrom(toolInput: Record): string[] { + return Array.isArray(toolInput.actionNames) ? toolInput.actionNames.flatMap((name) => nonEmpty(name) ?? []) : [] +} + +async function afterDraftChange({ agent, publish, projectId, userId, log }: { + agent: Agent + publish: boolean + projectId: string + userId: string + log: FastifyBaseLogger +}): Promise { + const { data: published } = publish + ? await tryCatch(() => agentService(log).publish({ id: agent.id, projectId, userId })) + : { data: undefined } + return { + agentId: agent.id, + displayName: agent.displayName, + published: !isNil(published), + url: await domainHelper.getPublicUrl({ path: `/projects/${projectId}/agents/${agent.id}` }), + note: !isNil(published) + ? `"${agent.displayName}" is live: new runs use this version.` + : publish + ? 'Publishing failed, so nothing is live yet — the draft may still need instructions. Send the user to the url above.' + : isNil(agent.published) + ? 'Saved to the draft. Nothing runs this agent until it is published.' + : 'Saved to the draft, so do not tell the user the change is live. The published version keeps running until this is published.', + } +} + async function checkWriteRunPermission({ userId, projectId, toolName, log }: { userId: string projectId: string @@ -314,6 +479,47 @@ async function executeCrossProjectTool({ toolName, toolInput, platformId, userId : 'This connection is NOT working (its credentials failed). Do not build on it — show the connection picker so the user can reconnect, then retry.', } } + case 'ap_list_agents': + case 'ap_update_agent': + case 'ap_add_agent_tool': + case 'ap_remove_agent_tool': + case 'ap_create_agent': { + if (!await agentHelpers.agentsSurfaceAvailable({ platformId, log })) { + return { error: 'Agents are not available here, so there is nothing to list, create or change.' } + } + const conversation = isNil(conversationId) ? undefined : await agentHelpers.getConversationOrThrow({ id: conversationId, platformId, userId }) + const projectId = projects.find((project) => project.id === conversation?.projectId)?.id + if (isNil(projectId)) { + return { error: 'No project is selected for this conversation. Ask the user which project the agent belongs to.' } + } + const checker = await resolvePermissionChecker({ userId, projectId, log }) + const denial = checker.check(toolName === 'ap_list_agents' ? Permission.READ_AGENT : Permission.WRITE_AGENT, toolName) + if (!isNil(denial)) { + return denial + } + if (toolName === 'ap_list_agents') { + const { data } = await agentService(log).list({ platformId, userId, projectId, cursor: null, limit: AGENT_LIST_LIMIT }) + return data.map(({ id, displayName, description, isPublished, toolCount }) => ({ agentId: id, displayName, description, published: isPublished, toolCount })) + } + if (toolName === 'ap_create_agent') { + return createAgentFromChat({ toolInput, projectId, userId, log }) + } + const agentId = nonEmpty(toolInput.agentId) + if (isNil(agentId)) { + return { error: 'Which agent? Call ap_list_agents first and pass its agentId.' } + } + const { data: agent } = await tryCatch(() => agentService(log).getOneOrThrow({ id: agentId, projectId, userId })) + if (isNil(agent)) { + return { error: 'No agent with that id in this project. Call ap_list_agents to see what is there.' } + } + if (toolName === 'ap_update_agent') { + return updateAgentFromChat({ toolInput, agent, projectId, userId, log }) + } + if (toolName === 'ap_remove_agent_tool') { + return removeAgentToolFromChat({ toolInput, agent, projectId, userId, log }) + } + return addAgentToolFromChat({ toolInput, agent, projectId, platformId, userId, log }) + } case 'ap_execute_action': { return runAgentAction({ toolInput, projects, availableProjectIds, conversationId, platformId, userId, requireWritePermission: true, log }) } diff --git a/packages/server/api/test/integration/ee/agent/chat-agent-tools.test.ts b/packages/server/api/test/integration/ee/agent/chat-agent-tools.test.ts new file mode 100644 index 000000000000..8e92ccde1678 --- /dev/null +++ b/packages/server/api/test/integration/ee/agent/chat-agent-tools.test.ts @@ -0,0 +1,321 @@ +import { apId } from '@activepieces/core-utils' +import { AgentToolType, PackageType, PieceType } from '@activepieces/shared' +import { FastifyInstance } from 'fastify' +import { StatusCodes } from 'http-status-codes' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { executeCrossProjectTool } from '../../../../src/app/ee/agent/tools/agent-tools' +import { db } from '../../../helpers/db' +import { createMockConnection, createMockPieceMetadata } from '../../../helpers/mocks' +import { createTestContext, TestContext } from '../../../helpers/test-context' +import { setupTestEnvironment, teardownTestEnvironment } from '../../../helpers/test-setup' + +let app: FastifyInstance + +const TOOL_PIECE = '@activepieces/piece-test-notes' +const TOOL_PIECE_VERSION = '0.4.2' + +beforeAll(async () => { + process.env.AP_AGENTS_ENABLED = 'true' + app = await setupTestEnvironment() + await db.save('piece_metadata', createMockPieceMetadata({ + name: TOOL_PIECE, + displayName: 'Test Notes', + version: TOOL_PIECE_VERSION, + pieceType: PieceType.OFFICIAL, + packageType: PackageType.REGISTRY, + platformId: undefined, + actions: { + save_note: { + name: 'save_note', + displayName: 'Save Note', + description: 'Save a note', + requireAuth: true, + props: {}, + }, + read_note: { + name: 'read_note', + displayName: 'Read Note', + description: 'Read a note', + requireAuth: true, + props: {}, + }, + }, + })) +}) + +afterAll(async () => { + await teardownTestEnvironment() +}) + +async function context(): Promise { + return createTestContext(app, { plan: { agentsEnabled: true, chatEnabled: true } }) +} + +// A chat conversation starts with no project and picks one up when the user selects it, so the +// tools have nothing to write to until then. Selecting it is what the browser does before any of +// this is reachable. +async function startConversation(ctx: TestContext): Promise { + const response = await ctx.post('/v1/agents/conversations', {}) + expect(response.statusCode).toBe(StatusCodes.CREATED) + const conversationId = response.json().id + await db.update('agent_conversation', conversationId, { projectId: ctx.project.id }) + return conversationId +} + +async function runTool(ctx: TestContext, conversationId: string, toolName: string, toolInput: Record = {}) { + return executeCrossProjectTool({ + toolName, + toolInput, + platformId: ctx.platform.id, + userId: ctx.user.id, + conversationId, + log: app.log, + }) +} + +async function agentRow(id: string) { + return db.findOneByOrFail<{ id: string, displayName: string, draft: Record, published: Record | null }>('agent', { id }) +} + +describe('the chat tools that build agents, against the real service', () => { + it('creates an agent the API can then read back', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + + const created = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Inbox triage', + instructions: 'Flag anything needing a reply today.', + }) + + const { agentId } = created as { agentId: string } + const stored = await agentRow(agentId) + expect(stored.displayName).toBe('Inbox triage') + expect(stored.published).toBeNull() + expect(stored.draft.tools).toEqual([]) + + const read = await ctx.get(`/v1/agents/${agentId}`) + expect(read.statusCode).toBe(StatusCodes.OK) + expect(read.json().draft.instructions).toBe('Flag anything needing a reply today.') + }) + + it('publishes the edit it was asked to publish, not the version before it', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Inbox triage', + instructions: 'First brief.', + }) as { agentId: string } + + const result = await runTool(ctx, conversationId, 'ap_update_agent', { + agentId, + instructions: 'Second brief, ignore newsletters.', + publish: true, + }) + + expect(result).toEqual(expect.objectContaining({ published: true })) + const stored = await agentRow(agentId) + expect(stored.draft.instructions).toBe('Second brief, ignore newsletters.') + expect(stored.published?.instructions).toBe('Second brief, ignore newsletters.') + }) + + it('gives an agent a real piece action, pinned to the version the server resolved', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const connection = createMockConnection({ + projectIds: [ctx.project.id], + platformId: ctx.platform.id, + pieceName: TOOL_PIECE, + }, ctx.user.id) + await db.save('app_connection', connection) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Storer', + instructions: 'Keep notes.', + }) as { agentId: string } + + const result = await runTool(ctx, conversationId, 'ap_add_agent_tool', { + agentId, + pieceName: TOOL_PIECE, + actionNames: ['save_note'], + connectionExternalId: connection.externalId, + publish: true, + }) + + expect(result).toEqual(expect.objectContaining({ published: true })) + const stored = await agentRow(agentId) + const [tool] = stored.draft.tools as Array<{ type: string, toolName: string, pieceMetadata: { pieceVersion: string, predefinedInput?: { auth: string } } }> + expect(tool.type).toBe(AgentToolType.PIECE) + expect(tool.toolName).toBe('save_note') + expect(tool.pieceMetadata.pieceVersion).toBe(TOOL_PIECE_VERSION) + expect(tool.pieceMetadata.predefinedInput?.auth).toBe(connection.externalId) + expect((stored.published?.tools as unknown[])).toHaveLength(1) + }) + + it('refuses an action the piece does not have', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Storer', + instructions: 'Keep notes.', + }) as { agentId: string } + + const result = await runTool(ctx, conversationId, 'ap_add_agent_tool', { + agentId, + pieceName: TOOL_PIECE, + actionNames: ['invented_action'], + }) + + expect(result).toEqual({ error: expect.stringContaining('no action called') }) + expect((await agentRow(agentId)).draft.tools).toEqual([]) + }) + + it('keeps the tools an agent already has when its instructions change', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Storer', instructions: 'Keep notes.', + }) as { agentId: string } + await runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: TOOL_PIECE, actionNames: ['save_note'], publish: true }) + + const result = await runTool(ctx, conversationId, 'ap_update_agent', { agentId, instructions: 'Keep fewer notes.' }) + + expect(result).toEqual(expect.objectContaining({ note: expect.stringContaining('do not tell the user the change is live') })) + const stored = await agentRow(agentId) + expect(stored.draft.instructions).toBe('Keep fewer notes.') + expect(stored.draft.tools).toHaveLength(1) + }) + + it('publishes on request alone, with nothing else to change', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Storer', instructions: 'Keep notes.', + }) as { agentId: string } + + const result = await runTool(ctx, conversationId, 'ap_update_agent', { agentId, publish: true }) + + expect(result).toEqual(expect.objectContaining({ published: true })) + expect((await agentRow(agentId)).published?.instructions).toBe('Keep notes.') + }) + + it('says nothing is live when the publish after an edit fails', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Storer', instructions: 'Keep notes.', + }) as { agentId: string } + await db.update('agent', agentId, { draft: { instructions: ' ', maxSteps: 20, tools: [], structuredOutput: [] } }) + + const result = await runTool(ctx, conversationId, 'ap_update_agent', { agentId, displayName: 'Renamed', publish: true }) + + expect(result).toEqual(expect.objectContaining({ published: false, note: expect.stringContaining('nothing is live yet') })) + expect((await agentRow(agentId)).published).toBeNull() + }) + + it('hides the surface rather than failing the turn when the plan cannot be read', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + + const result = await executeCrossProjectTool({ + toolName: 'ap_list_agents', + toolInput: {}, + platformId: apId(), + userId: ctx.user.id, + conversationId, + log: app.log, + }) as { error?: string } + + expect(result.error).toContain('not available') + }) + + it('keeps both tools when the model adds two at once', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Storer', instructions: 'Keep notes.', + }) as { agentId: string } + + await Promise.all([ + runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: TOOL_PIECE, actionNames: ['save_note'] }), + runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: TOOL_PIECE, actionNames: ['read_note'] }), + ]) + + const tools = (await agentRow(agentId)).draft.tools as Array<{ toolName: string }> + expect(tools.map((tool) => tool.toolName).sort()).toEqual(['read_note', 'save_note']) + }) + + it('takes a tool away again, and says so when there is none to take', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Storer', instructions: 'Keep notes.', + }) as { agentId: string } + await runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: TOOL_PIECE, actionNames: ['save_note'] }) + + const removed = await runTool(ctx, conversationId, 'ap_remove_agent_tool', { agentId, actionNames: ['save_note'], publish: true }) + const again = await runTool(ctx, conversationId, 'ap_remove_agent_tool', { agentId, actionNames: ['save_note'] }) + + expect(removed).toEqual(expect.objectContaining({ published: true })) + expect(again).toEqual({ error: expect.stringContaining('nothing to remove') }) + const stored = await agentRow(agentId) + expect(stored.draft.tools).toEqual([]) + expect(stored.published?.tools).toEqual([]) + }) + + it('refuses until the conversation has a project to write to', async () => { + const ctx = await context() + const response = await ctx.post('/v1/agents/conversations', {}) + const conversationId = response.json().id + + const result = await runTool(ctx, conversationId, 'ap_create_agent', { displayName: 'x', instructions: 'y' }) + + expect(result).toEqual({ error: expect.stringContaining('No project is selected') }) + }) + + it('lists the project\'s agents with whether a flow could run them', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Published one', + instructions: 'Do the thing.', + }) as { agentId: string } + await runTool(ctx, conversationId, 'ap_update_agent', { agentId, publish: true }) + await runTool(ctx, conversationId, 'ap_create_agent', { displayName: 'Draft one', instructions: 'Not ready.' }) + + const listed = await runTool(ctx, conversationId, 'ap_list_agents') as Array<{ displayName: string, published: boolean }> + + expect(listed).toEqual(expect.arrayContaining([ + expect.objectContaining({ displayName: 'Published one', published: true }), + expect.objectContaining({ displayName: 'Draft one', published: false }), + ])) + }) + + it('cannot touch an agent in another project, even with its real id', async () => { + const mine = await context() + const theirs = await context() + const theirConversation = await startConversation(theirs) + const { agentId } = await runTool(theirs, theirConversation, 'ap_create_agent', { + displayName: 'Theirs', + instructions: 'Private.', + }) as { agentId: string } + const myConversation = await startConversation(mine) + + const updated = await runTool(mine, myConversation, 'ap_update_agent', { agentId, instructions: 'Mine now.', publish: true }) + const tooled = await runTool(mine, myConversation, 'ap_add_agent_tool', { agentId, pieceName: TOOL_PIECE, actionNames: ['save_note'] }) + + expect(updated).toEqual({ error: expect.stringContaining('No agent with that id') }) + expect(tooled).toEqual({ error: expect.stringContaining('No agent with that id') }) + const stored = await agentRow(agentId) + expect(stored.draft.instructions).toBe('Private.') + expect(stored.published).toBeNull() + }) + + it('refuses every agent tool once the platform loses the entitlement', async () => { + const ctx = await createTestContext(app, { plan: { agentsEnabled: false, chatEnabled: true } }) + const conversationId = await startConversation(ctx) + + for (const toolName of ['ap_list_agents', 'ap_create_agent', 'ap_update_agent', 'ap_add_agent_tool', 'ap_remove_agent_tool']) { + const result = await runTool(ctx, conversationId, toolName, { displayName: 'x', instructions: 'y', agentId: 'z' }) + expect(result, toolName).toEqual({ error: expect.stringContaining('not available here') }) + } + }) +}) diff --git a/packages/server/api/test/unit/app/ee/agent/agent-surface-notes.test.ts b/packages/server/api/test/unit/app/ee/agent/agent-surface-notes.test.ts index 64deeb512e39..a01c245ae440 100644 --- a/packages/server/api/test/unit/app/ee/agent/agent-surface-notes.test.ts +++ b/packages/server/api/test/unit/app/ee/agent/agent-surface-notes.test.ts @@ -13,7 +13,7 @@ const CHAT_ONLY_TOOLS = [ 'ap_discover_action_auth', ] -const EVERYTHING_AVAILABLE = { searchAvailable: true, fetchAvailable: true, scrapeAvailable: true, imageAvailable: true, emailAvailable: true } +const EVERYTHING_AVAILABLE = { searchAvailable: true, fetchAvailable: true, scrapeAvailable: true, imageAvailable: true, emailAvailable: true, agentsAvailable: true } function notesFor(source: AgentRunSource): string { return agentSurfaceNotes.buildRunNotes({ @@ -27,6 +27,12 @@ function notesFor(source: AgentRunSource): string { } describe('what each surface is told it can do', () => { + it('only tells a chat run about saved agents, and only where the surface exists', () => { + expect(notesFor(AgentRunSource.CHAT)).toContain('Saved agents') + expect(notesFor(AgentRunSource.FLOW_STEP)).not.toContain('Saved agents') + expect(notesFor(AgentRunSource.AGENT)).not.toContain('Saved agents') + }) + it('tells a chat run about everything it has', () => { const notes = notesFor(AgentRunSource.CHAT) @@ -75,13 +81,14 @@ describe('what each surface is told it can do', () => { const notes = agentSurfaceNotes.buildRunNotes({ source: AgentRunSource.CHAT, currentDate: 'Tuesday, August 18, 2026', - searchAvailable: false, fetchAvailable: false, scrapeAvailable: false, imageAvailable: false, emailAvailable: false, + searchAvailable: false, fetchAvailable: false, scrapeAvailable: false, imageAvailable: false, emailAvailable: false, agentsAvailable: false, userEmail: 'owner@acme.com', connections: null, memory: { instructions: null, memories: [] }, }) expect(notes).toContain('NOT available') + expect(notes).not.toContain('Saved agents') expect(notes).not.toContain('ap_web_search') expect(notes).not.toContain('ap_send_email') }) diff --git a/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-tool-policy.ts b/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-tool-policy.ts index 423da315d870..460b0e2a58d8 100644 --- a/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-tool-policy.ts +++ b/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-tool-policy.ts @@ -20,6 +20,7 @@ function selectToolsForSource({ source, groups }: { source: AgentRunSource, grou ...groups.phase, ...groups.buildPlan, ...groups.email, + ...groups.agentSurface, ...groups.mcp, } } @@ -57,6 +58,7 @@ export type AgentToolGroups = { phase: ToolSet buildPlan: ToolSet email: ToolSet + agentSurface: ToolSet mcp: ToolSet configuredPiece: ToolSet configuredFlow: ToolSet diff --git a/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-worker-tools.ts b/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-worker-tools.ts index 438b33099b6a..4019d2cebb19 100644 --- a/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-worker-tools.ts +++ b/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-worker-tools.ts @@ -702,6 +702,72 @@ function createCrossProjectTools({ executeTool, eventEmitter, waitForApproval, o } } +function createAgentSurfaceTools({ executeTool }: { + executeTool: (toolName: string, toolInput: Record) => Promise +}): ToolSet { + return { + ap_list_agents: tool({ + description: 'List the saved agents in the active project, with whether each one is published. Call it before offering to create an agent, so you build on what exists instead of adding a near-duplicate, and when the user asks what agents they have.', + inputSchema: z.object({}), + execute: async (toolInput) => { + return executeTool('ap_list_agents', toolInput) + }, + }), + + ap_add_agent_tool: tool({ + description: 'Give a saved agent piece actions it can call, so it can do the work rather than only reason about it. Look them up first (ap_research_pieces for the piece and action names, ap_list_connections for the connection). Pass every action for one piece in a single call — one call per piece, never several at once for the same agent.', + inputSchema: z.object({ + agentId: z.string().describe('The id returned by ap_list_agents, ap_create_agent or ap_update_agent'), + pieceName: z.string().describe('Full piece name, e.g. "@activepieces/piece-gmail"'), + actionNames: z.array(z.string()).describe('Action names within that piece, e.g. ["gmail_search_mail"]'), + connectionExternalId: z.string().optional().describe('externalId from ap_list_connections, for a piece that needs an account'), + publish: z.boolean().optional().describe('Make the agent live with these tools in the same step'), + }), + execute: async (toolInput) => { + return executeTool('ap_add_agent_tool', toolInput) + }, + }), + + ap_remove_agent_tool: tool({ + description: 'Take piece actions away from a saved agent, when the user no longer wants it doing that or a tool was added by mistake. Pass every action to remove in one call.', + inputSchema: z.object({ + agentId: z.string().describe('The id returned by ap_list_agents'), + actionNames: z.array(z.string()).describe('Action names to remove, e.g. ["gmail_search_mail"]'), + publish: z.boolean().optional().describe('Make the agent live without these tools in the same step'), + }), + execute: async (toolInput) => { + return executeTool('ap_remove_agent_tool', toolInput) + }, + }), + + ap_update_agent: tool({ + description: 'Change a saved agent\'s name, description or instructions, and publish it. Send the full new instructions, not a diff — they replace what is there. Pass publish: true whenever the user wants the result live, including when they only ask you to publish and change nothing else.', + inputSchema: z.object({ + agentId: z.string().describe('The id returned by ap_list_agents or ap_create_agent'), + displayName: z.string().optional(), + description: z.string().optional(), + instructions: z.string().optional().describe('The agent\'s full new standing brief, in second person'), + publish: z.boolean().optional().describe('Make the change live for flows and chats in the same step'), + }), + execute: async (toolInput) => { + return executeTool('ap_update_agent', toolInput) + }, + }), + + ap_create_agent: tool({ + description: 'Create a saved agent in the active project from a name and instructions. Write the instructions as the agent\'s own standing brief, in second person, covering what it does and what it must not do.', + inputSchema: z.object({ + displayName: z.string().describe('Short name the user will recognise, e.g. "Inbox triage"'), + instructions: z.string().describe('The agent\'s standing brief, in second person'), + description: z.string().optional().describe('One line on what it is for'), + }), + execute: async (toolInput) => { + return executeTool('ap_create_agent', toolInput) + }, + }), + } +} + function createWebTools({ taintState }: { taintState: TaintState }): ToolSet { return { ap_fetch_url: tool({ @@ -1503,6 +1569,7 @@ function schemaForOutputField(field: AgentOutputField): z.ZodType { export const agentWorkerTools = { createEventEmitter, createDisplayTools, + createAgentSurfaceTools, createLocalTools, createCrossProjectTools, createWebTools, diff --git a/packages/server/worker/src/lib/execute/jobs/ee/agent/execute-agent-run.ts b/packages/server/worker/src/lib/execute/jobs/ee/agent/execute-agent-run.ts index e0a546853d40..a22330f6ab5c 100644 --- a/packages/server/worker/src/lib/execute/jobs/ee/agent/execute-agent-run.ts +++ b/packages/server/worker/src/lib/execute/jobs/ee/agent/execute-agent-run.ts @@ -94,6 +94,7 @@ export const executeAgentRunJob: JobHandler { phaseState.phase = phase @@ -669,6 +675,7 @@ function buildToolSet({ ctx, eventEmitter, log, phaseState, taintState, mcpToolS phase: phaseTools, buildPlan: buildPlanTools, email: emailTools, + agentSurface: agentSurfaceTools, mcp: mcpTools as ToolSet, configuredPiece: configuredTools, configuredFlow: configuredFlowToolSet, diff --git a/packages/server/worker/test/lib/execute/jobs/ee/agent/agent-tool-policy.test.ts b/packages/server/worker/test/lib/execute/jobs/ee/agent/agent-tool-policy.test.ts index 9bc214f69975..386df8d72d45 100644 --- a/packages/server/worker/test/lib/execute/jobs/ee/agent/agent-tool-policy.test.ts +++ b/packages/server/worker/test/lib/execute/jobs/ee/agent/agent-tool-policy.test.ts @@ -16,6 +16,7 @@ const GROUPS: AgentToolGroups = { phase: toolSet('ap_set_phase'), buildPlan: toolSet('ap_set_build_plan'), email: toolSet('ap_send_email'), + agentSurface: toolSet('ap_list_agents', 'ap_create_agent', 'ap_update_agent', 'ap_add_agent_tool', 'ap_remove_agent_tool'), mcp: toolSet('ap_create_flow', 'ap_test_flow'), configuredPiece: toolSet('gmail_find_email'), configuredFlow: toolSet('run_my_flow'), @@ -36,6 +37,19 @@ describe('what a chat run may reach', () => { expect(names).toContain('ap_select_project') expect(names).toContain('ap_execute_action') expect(names).toContain('ap_send_email') + expect(names).toContain('ap_create_agent') + }) +}) + +describe('what may reach the tools that build saved agents', () => { + const AGENT_SURFACE_TOOLS = ['ap_list_agents', 'ap_create_agent', 'ap_update_agent', 'ap_add_agent_tool', 'ap_remove_agent_tool'] + + it('only a chat run, since the other surfaces have nobody to review what was made', () => { + for (const toolName of AGENT_SURFACE_TOOLS) { + expect(namesFor(AgentRunSource.CHAT), toolName).toContain(toolName) + expect(namesFor(AgentRunSource.AGENT), toolName).not.toContain(toolName) + expect(namesFor(AgentRunSource.FLOW_STEP), toolName).not.toContain(toolName) + } }) }) From 91afd3527ff35864a6394827061063f552988b33 Mon Sep 17 00:00:00 2001 From: Hazem Adel Date: Wed, 26 Aug 2026 06:30:39 +0300 Subject: [PATCH 2/2] fix(agents): an agent keeps both tools when two apps name their action the same (#15058) --- .../api/src/app/ee/agent/tools/agent-tools.ts | 47 ++++++-- .../ee/agent/chat-agent-tools.test.ts | 106 +++++++++++++++++- .../jobs/ee/agent/agent-worker-tools.ts | 3 +- 3 files changed, 143 insertions(+), 13 deletions(-) diff --git a/packages/server/api/src/app/ee/agent/tools/agent-tools.ts b/packages/server/api/src/app/ee/agent/tools/agent-tools.ts index 680c696d8d47..86cf18a0c729 100644 --- a/packages/server/api/src/app/ee/agent/tools/agent-tools.ts +++ b/packages/server/api/src/app/ee/agent/tools/agent-tools.ts @@ -1,6 +1,6 @@ -import { isNil, isObject, isString, parseToJsonIfPossible, Permission, spreadIfDefined, tryCatch } from '@activepieces/core-utils' +import { isNil, isObject, isString, parseToJsonIfPossible, Permission, spreadIfDefined, tryCatch, unique } from '@activepieces/core-utils' import { agentAiUtils } from '@activepieces/server-utils' -import { Agent, AgentIcon, AgentTool, agentToolClassification, AgentToolType, AppConnectionStatus, AppConnectionType, ColorName, DEFAULT_AGENT_MAX_STEPS, FileCompression, FileType, FlowRunStatus, FlowStatus, Project, RunEnvironment } from '@activepieces/shared' +import { Agent, AgentIcon, AgentTool, agentToolClassification, AgentToolType, AppConnectionStatus, AppConnectionType, ColorName, DEFAULT_AGENT_MAX_STEPS, FileCompression, FileType, FlowRunStatus, FlowStatus, mcpToolNameUtils, Project, RunEnvironment } from '@activepieces/shared' import { FastifyBaseLogger } from 'fastify' import { appConnectionService } from '../../../app-connection/app-connection-service/app-connection-service' import { fileService } from '../../../file/file.service' @@ -303,7 +303,7 @@ async function addAgentToolFromChat({ toolInput, agent, projectId, platformId, u } const added: AgentTool[] = actionNames.map((actionName) => ({ type: AgentToolType.PIECE, - toolName: actionName, + toolName: mcpToolNameUtils.createPieceToolName(normalizedPiece, actionName), pieceMetadata: { pieceName: normalizedPiece, pieceVersion: piece.version, @@ -315,7 +315,10 @@ async function addAgentToolFromChat({ toolInput, agent, projectId, platformId, u id: agent.id, projectId, userId, - edit: (tools) => tools.some((tool) => actionNames.includes(tool.toolName)) ? null : [...tools, ...added], + edit: (tools) => tools.some((tool) => { + const action = pieceActionOf(tool) + return action?.pieceName === normalizedPiece && actionNames.includes(action.actionName) + }) ? null : [...tools, ...added], }) if (isNil(updated)) { return { error: `${agent.displayName} already has one of those tools. List them with ap_list_agents before adding.` } @@ -334,13 +337,37 @@ async function removeAgentToolFromChat({ toolInput, agent, projectId, userId, lo if (actionNames.length === 0) { return { error: 'Removing tools needs at least one action name.' } } + const requestedPiece = nonEmpty(toolInput.pieceName) + const scopedPiece = isNil(requestedPiece) ? undefined : mcpUtils.normalizePieceName(requestedPiece) ?? requestedPiece + const targets = agent.draft.tools.flatMap((tool) => pieceActionOf(tool) ?? []) + .filter((action) => actionNames.includes(action.actionName)) + .filter((action) => isNil(scopedPiece) || action.pieceName === scopedPiece) + const resolved = actionNames.map((actionName) => ({ + actionName, + pieces: unique(targets.filter((target) => target.actionName === actionName).map((target) => target.pieceName)), + })) + const ambiguous = resolved.filter((entry) => entry.pieces.length > 1) + if (ambiguous.length > 0) { + return { error: `${ambiguous.map((entry) => entry.actionName).join(' and ')} is on more than one piece here: ${unique(ambiguous.flatMap((entry) => entry.pieces)).join(', ')}. Call this again with pieceName set to the one you mean.` } + } + const unmatched = resolved.filter((entry) => entry.pieces.length === 0).map((entry) => entry.actionName) + if (unmatched.length > 0) { + return { error: isNil(scopedPiece) + ? `${agent.displayName} has no tool for ${unmatched.join(' or ')}, so there is nothing to remove. List its tools with ap_list_agents.` + : `${unmatched.join(' and ')} is not on ${scopedPiece}, so there is nothing to remove. Take those away in their own call, without pieceName.` } + } + const removing = new Set(targets.map((target) => `${target.pieceName}:${target.actionName}`)) const updated = await agentService(log).editDraftTools({ id: agent.id, projectId, userId, - edit: (tools) => tools.some((tool) => actionNames.includes(tool.toolName)) - ? tools.filter((tool) => !actionNames.includes(tool.toolName)) - : null, + edit: (tools) => { + const kept = tools.filter((tool) => { + const action = pieceActionOf(tool) + return isNil(action) || !removing.has(`${action.pieceName}:${action.actionName}`) + }) + return kept.length === tools.length ? null : kept + }, }) if (isNil(updated)) { return { error: `${agent.displayName} has none of those tools, so there is nothing to remove.` } @@ -348,6 +375,12 @@ async function removeAgentToolFromChat({ toolInput, agent, projectId, userId, lo return afterDraftChange({ agent: updated, publish: toolInput.publish === true, projectId, userId, log }) } +function pieceActionOf(tool: AgentTool): { pieceName: string, actionName: string } | undefined { + return tool.type === AgentToolType.PIECE + ? { pieceName: tool.pieceMetadata.pieceName, actionName: tool.pieceMetadata.actionName } + : undefined +} + function toolNamesFrom(toolInput: Record): string[] { return Array.isArray(toolInput.actionNames) ? toolInput.actionNames.flatMap((name) => nonEmpty(name) ?? []) : [] } diff --git a/packages/server/api/test/integration/ee/agent/chat-agent-tools.test.ts b/packages/server/api/test/integration/ee/agent/chat-agent-tools.test.ts index 8e92ccde1678..92e82ec3e11e 100644 --- a/packages/server/api/test/integration/ee/agent/chat-agent-tools.test.ts +++ b/packages/server/api/test/integration/ee/agent/chat-agent-tools.test.ts @@ -1,5 +1,6 @@ import { apId } from '@activepieces/core-utils' -import { AgentToolType, PackageType, PieceType } from '@activepieces/shared' +import { unique } from '@activepieces/core-utils' +import { AgentToolType, mcpToolNameUtils, PackageType, PieceType } from '@activepieces/shared' import { FastifyInstance } from 'fastify' import { StatusCodes } from 'http-status-codes' import { afterAll, beforeAll, describe, expect, it } from 'vitest' @@ -13,6 +14,7 @@ let app: FastifyInstance const TOOL_PIECE = '@activepieces/piece-test-notes' const TOOL_PIECE_VERSION = '0.4.2' +const RIVAL_PIECE = '@activepieces/piece-test-memos' beforeAll(async () => { process.env.AP_AGENTS_ENABLED = 'true' @@ -41,6 +43,23 @@ beforeAll(async () => { }, }, })) + await db.save('piece_metadata', createMockPieceMetadata({ + name: RIVAL_PIECE, + displayName: 'Test Memos', + version: '1.0.0', + pieceType: PieceType.OFFICIAL, + packageType: PackageType.REGISTRY, + platformId: undefined, + actions: { + save_note: { + name: 'save_note', + displayName: 'Save Note', + description: 'Save a memo', + requireAuth: true, + props: {}, + }, + }, + })) }) afterAll(async () => { @@ -73,6 +92,11 @@ async function runTool(ctx: TestContext, conversationId: string, toolName: strin }) } +async function actionNamesOn(id: string): Promise { + const tools = (await agentRow(id)).draft.tools as Array<{ pieceMetadata?: { actionName: string } }> + return tools.flatMap((tool) => tool.pieceMetadata?.actionName ?? []) +} + async function agentRow(id: string) { return db.findOneByOrFail<{ id: string, displayName: string, draft: Record, published: Record | null }>('agent', { id }) } @@ -142,9 +166,10 @@ describe('the chat tools that build agents, against the real service', () => { expect(result).toEqual(expect.objectContaining({ published: true })) const stored = await agentRow(agentId) - const [tool] = stored.draft.tools as Array<{ type: string, toolName: string, pieceMetadata: { pieceVersion: string, predefinedInput?: { auth: string } } }> + const [tool] = stored.draft.tools as Array<{ type: string, toolName: string, pieceMetadata: { actionName: string, pieceVersion: string, predefinedInput?: { auth: string } } }> expect(tool.type).toBe(AgentToolType.PIECE) - expect(tool.toolName).toBe('save_note') + expect(tool.pieceMetadata.actionName).toBe('save_note') + expect(tool.toolName).toBe(mcpToolNameUtils.createPieceToolName(TOOL_PIECE, 'save_note')) expect(tool.pieceMetadata.pieceVersion).toBe(TOOL_PIECE_VERSION) expect(tool.pieceMetadata.predefinedInput?.auth).toBe(connection.externalId) expect((stored.published?.tools as unknown[])).toHaveLength(1) @@ -227,6 +252,78 @@ describe('the chat tools that build agents, against the real service', () => { expect(result.error).toContain('not available') }) + it('keeps two pieces that name their action the same, under distinct tool names', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Storer', instructions: 'Keep notes.', + }) as { agentId: string } + + await runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: TOOL_PIECE, actionNames: ['save_note'] }) + await runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: RIVAL_PIECE, actionNames: ['save_note'] }) + + const tools = (await agentRow(agentId)).draft.tools as Array<{ toolName: string, pieceMetadata: { pieceName: string } }> + expect(tools.map((tool) => tool.pieceMetadata.pieceName).sort()).toEqual([RIVAL_PIECE, TOOL_PIECE]) + expect(unique(tools.map((tool) => tool.toolName))).toHaveLength(2) + }) + + it('asks which piece when an action name is on more than one of them', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Storer', instructions: 'Keep notes.', + }) as { agentId: string } + await runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: TOOL_PIECE, actionNames: ['save_note'] }) + await runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: RIVAL_PIECE, actionNames: ['save_note'] }) + + const result = await runTool(ctx, conversationId, 'ap_remove_agent_tool', { agentId, actionNames: ['save_note'] }) as { error?: string } + + expect(result.error).toContain('more than one piece') + expect((await agentRow(agentId)).draft.tools).toHaveLength(2) + + const resolved = await runTool(ctx, conversationId, 'ap_remove_agent_tool', { + agentId, actionNames: ['save_note'], pieceName: RIVAL_PIECE, + }) as { error?: string } + + expect(resolved.error).toBeUndefined() + const left = (await agentRow(agentId)).draft.tools as Array<{ pieceMetadata: { pieceName: string } }> + expect(left.map((tool) => tool.pieceMetadata.pieceName)).toEqual([TOOL_PIECE]) + }) + + it('removes nothing when pieceName would silently skip one of the actions asked for', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Storer', instructions: 'Keep notes.', + }) as { agentId: string } + await runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: TOOL_PIECE, actionNames: ['save_note', 'read_note'] }) + await runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: RIVAL_PIECE, actionNames: ['save_note'] }) + + const result = await runTool(ctx, conversationId, 'ap_remove_agent_tool', { + agentId, actionNames: ['save_note', 'read_note'], pieceName: RIVAL_PIECE, + }) as { error?: string } + + expect(result.error).toContain('read_note is not on') + expect((await agentRow(agentId)).draft.tools).toHaveLength(3) + }) + + it('removes two actions that live on different pieces without asking anything', async () => { + const ctx = await context() + const conversationId = await startConversation(ctx) + const { agentId } = await runTool(ctx, conversationId, 'ap_create_agent', { + displayName: 'Storer', instructions: 'Keep notes.', + }) as { agentId: string } + await runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: TOOL_PIECE, actionNames: ['read_note'] }) + await runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: RIVAL_PIECE, actionNames: ['save_note'] }) + + const result = await runTool(ctx, conversationId, 'ap_remove_agent_tool', { + agentId, actionNames: ['read_note', 'save_note'], + }) as { error?: string } + + expect(result.error).toBeUndefined() + expect((await agentRow(agentId)).draft.tools).toHaveLength(0) + }) + it('keeps both tools when the model adds two at once', async () => { const ctx = await context() const conversationId = await startConversation(ctx) @@ -239,8 +336,7 @@ describe('the chat tools that build agents, against the real service', () => { runTool(ctx, conversationId, 'ap_add_agent_tool', { agentId, pieceName: TOOL_PIECE, actionNames: ['read_note'] }), ]) - const tools = (await agentRow(agentId)).draft.tools as Array<{ toolName: string }> - expect(tools.map((tool) => tool.toolName).sort()).toEqual(['read_note', 'save_note']) + expect((await actionNamesOn(agentId)).sort()).toEqual(['read_note', 'save_note']) }) it('takes a tool away again, and says so when there is none to take', async () => { diff --git a/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-worker-tools.ts b/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-worker-tools.ts index 4019d2cebb19..073b10a8eb6a 100644 --- a/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-worker-tools.ts +++ b/packages/server/worker/src/lib/execute/jobs/ee/agent/agent-worker-tools.ts @@ -729,10 +729,11 @@ function createAgentSurfaceTools({ executeTool }: { }), ap_remove_agent_tool: tool({ - description: 'Take piece actions away from a saved agent, when the user no longer wants it doing that or a tool was added by mistake. Pass every action to remove in one call.', + description: 'Take piece actions away from a saved agent, when the user no longer wants it doing that or a tool was added by mistake. Pass every action to remove in one call. If two of the agent\'s pieces share an action name, pass pieceName to say which one.', inputSchema: z.object({ agentId: z.string().describe('The id returned by ap_list_agents'), actionNames: z.array(z.string()).describe('Action names to remove, e.g. ["gmail_search_mail"]'), + pieceName: z.string().optional().describe('Full piece name, only needed when the same action name is on two of the agent\'s pieces'), publish: z.boolean().optional().describe('Make the agent live without these tools in the same step'), }), execute: async (toolInput) => {