Skip to content
Open
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
21 changes: 19 additions & 2 deletions packages/ai-proxy/src/forest-integration-client.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { McpServerLoadFailure } from './mcp-client';
import type RemoteTool from './remote-tool';
import type { ToolProvider } from './tool-provider';
import type { Logger } from '@forestadmin/datasource-toolkit';
Expand Down Expand Up @@ -38,8 +39,12 @@ export default class ForestIntegrationClient implements ToolProvider {
this.configs = configs;
}

async loadTools(): Promise<RemoteTool[]> {
async loadToolsWithFailures(): Promise<{
tools: RemoteTool[];
failures: McpServerLoadFailure[];
}> {
const tools: RemoteTool[] = [];
const failures: McpServerLoadFailure[] = [];

this.configs.forEach(({ id: mcpServerId, integrationName, config }) => {
switch (integrationName) {
Expand All @@ -54,10 +59,22 @@ export default class ForestIntegrationClient implements ToolProvider {
break;
default:
this.logger?.('Warn', `Unsupported integration: ${integrationName}`);
// Reporting it is what stops a caller reading an integration this build doesn't know as
// a healthy connector that publishes nothing.
failures.push({
server: integrationName,
mcpServerId,
kind: 'unknown',
error: new Error(`Unsupported integration: ${integrationName}`),
});
}
});

return tools;
return { tools, failures };
}

async loadTools(): Promise<RemoteTool[]> {
return (await this.loadToolsWithFailures()).tools;
}

async checkConnection(): Promise<true> {
Expand Down
57 changes: 57 additions & 0 deletions packages/ai-proxy/test/forest-integration-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,63 @@ describe('ForestIntegrationClient', () => {
});
});

describe('loadToolsWithFailures', () => {
it('reports an unsupported integration as a failure carrying its name and id', async () => {
const client = new ForestIntegrationClient(
// @ts-expect-error Testing unsupported integration
[{ id: '7', integrationName: 'unknown', config: {} as any, isForestConnector: true }],
);

const { tools, failures } = await client.loadToolsWithFailures();

expect(tools).toEqual([]);
expect(failures).toEqual([
{
server: 'unknown',
mcpServerId: '7',
kind: 'unknown',
error: new Error('Unsupported integration: unknown'),
},
]);
});

it('reports no failure when every integration is supported', async () => {
const client = new ForestIntegrationClient([
{
id: '1',
integrationName: 'Zendesk',
config: { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' },
isForestConnector: true,
},
]);

const { tools, failures } = await client.loadToolsWithFailures();

expect(tools).toEqual(mockZendeskTools);
expect(failures).toEqual([]);
});

// A supported connector alongside a broken one must still contribute its tools.
it('keeps a supported integration tools when another one is unsupported', async () => {
const client = new ForestIntegrationClient([
{
id: '1',
integrationName: 'Zendesk',
config: { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' },
isForestConnector: true,
},
// @ts-expect-error Testing unsupported integration
{ id: '2', integrationName: 'unknown', config: {} as any, isForestConnector: true },
]);

const { tools, failures } = await client.loadToolsWithFailures();

expect(tools).toEqual(mockZendeskTools);
expect(failures).toHaveLength(1);
expect(failures[0].mcpServerId).toBe('2');
});
});

describe('checkConnection', () => {
it('should call validateZendeskConfig for Zendesk integration', async () => {
const zendeskConfig = { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' };
Expand Down
3 changes: 2 additions & 1 deletion packages/workflow-executor/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p
- **Boundary validation** — wire/mapper types live in `types/validated/` as zod. Strictness by origin: executor-produced + frontend bodies use `.strict()`; the orchestrator collection schema **strips** unknowns and asserts step-specific props at use-time (resilient to orchestrator drift). Parse failure → `DomainValidationError`/`InvalidStepDefinitionError`. `StepOutcome` is validated only when it arrives via `previousSteps`; executor outputs are trusted by construction.
- **DatabaseStore** — table `workflow_step_executions` + migration registry namespaced under a schema (default `forest`, override via `DATABASE_SCHEMA`), so a DB shared with the agent/server is safe. The schema is created idempotently at `init()`, but gated on a `pg_namespace` existence probe (not `CREATE SCHEMA IF NOT EXISTS` alone): Postgres checks database-level `CREATE` even for `IF NOT EXISTS`, so probing lets a pre-created schema boot with only schema-level `CREATE`. SQLite (tests) skips schemas. Migrations run behind a **transaction-scoped Postgres advisory lock** (`pg_advisory_xact_lock`, safe behind RDS Proxy / PgBouncer) so HA cold-starts migrate once; migrations are transactional + idempotent. Postgres-only; the lock key is a fixed constant — never change it.
- **Graceful shutdown** — `stop()` drains in-flight steps (`idle → running → draining → stopped`), `stopTimeoutMs` default 30s, HTTP stays up during drain. Signal handling is the consumer's job.
- **Logging** — `Logger = (level, message, context?) => void`. `BaseStepExecutor` stamps `logCtx` (runId/stepId/stepIndex/stepType); type-specific ids via `getExtraLogContext()`. `createConsoleLogger`/`createPrettyLogger(minLevel)` factories; CLI level from `LOG_LEVEL` (default `Info`).
- **Logging** — `Logger = (level, message, context?) => void`. `BaseStepExecutor` stamps `logCtx` (runId/stepId/stepIndex/stepType); type-specific ids via `getExtraLogContext()`. `createConsoleLogger`/`createPrettyLogger(minLevel)` factories; CLI level from `LOG_LEVEL` (default `Info`). ai-proxy's logger takes the cause as a third `Error` argument instead of a context object, so both AI adapters bridge it with `toAiProxyLogger` (flattens to `{ error, cause, stack }` — an `Error`'s own properties are non-enumerable and would vanish from the emitted line — and swallows a throwing host logger, which ai-proxy calls from inside its catch blocks).
- **MCP load failures come from the `failures` channel** — `RemoteToolFetcher` loads through `loadRemoteToolsWithFailures` and reports what the providers classified (`server`/`kind`/`error`); never infer failure from absent tools, which flags a healthy server exposing none. `loadFailed` drives the 503 on `GET /list-mcp-tools`, so a wrong inference is user-visible.
- **Config comes from the boundary, never `process.env`** — no executor *config* is read from `process.env` outside `cli-core`: every knob is parsed there (standalone) or injected as an option (`ExecutorOptions` / the agent's `addWorkflowExecutor` options), and the check for a value is `Boolean(options.x)`, not `process.env`. Runtime-mode flags — `NODE_ENV` (forceAiError prod-guard, token-endpoint dev check) and the `OTEL_*` observability vars in `tracing.ts` — are the deliberate exception. This keeps the executor identically configurable standalone and embedded, and testable without mutating env. (Regression fixed once: `FOREST_EXECUTOR_ENCRYPTION_KEY` was read in `crypto/` — now injected via `executorEncryptionKey`.)
- **AI** — import every AI type (`BaseChatModel`, `DynamicStructuredTool`, `SystemMessage`/`HumanMessage`, `RemoteTool`/`ToolConfig`) from `@forestadmin/ai-proxy`, **not** `@langchain/core` (which is not a dependency). `ExecutionContext.model` is a `BaseChatModel`. The only langchain mention in src is a comment in `cli.ts` about transitively loading `@langchain/openai`.

Expand Down
28 changes: 28 additions & 0 deletions packages/workflow-executor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,34 @@ When your workflows use OAuth-protected MCP connectors, the executor stores each

---

## When an MCP step fails to load its tools

The executor names the reason at `Error`, so it is in your logs at the default level:

```json
{
"level": "Error",
"message": "MCP servers failed to load tools",
"requestedMcpServerId": "39",
"mcpServerName": "acme-crm",
"failures": [
{ "server": "acme-crm", "kind": "connection", "error": "connect ECONNREFUSED 10.0.4.12:8080" }
]
}
```

`kind` tells you where to look:

- `auth` — the server rejected the credential (HTTP 401). On an OAuth2 connector the executor refreshes the token and retries once on its own, so this line concerns static credentials; an OAuth2 connector that recovered logs `MCP tools loaded after refreshing the credential` at `Info`, and one that cannot pauses the run for re-authentication instead.
- `connection` — unreachable, refused, or slower than the 15s per-server load timeout.
- `unknown` — the server answered but the load failed anyway, including HTTP 403 (the credential is valid but lacks the permission, which no refresh can fix) and an integration this build does not support; the `error` text carries the reason.

A server that answers but exposes no tools is not a failure: you get an empty tool list and no error.

Set `LOG_LEVEL=Debug` to add one line per server with its tool count and load time, which is how you find the connector that is slowing a step down.

---

## Testing only

The following modes skip the database requirement but are **not suitable for production** — state is lost on restart.
Expand Down
9 changes: 7 additions & 2 deletions packages/workflow-executor/src/adapters/ai-client-adapter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { AiModelPort, GetModelOptions } from '../ports/ai-model-port';
import type { Logger } from '../ports/logger-port';
import type {
AiConfiguration,
BaseChatModel,
Expand All @@ -10,13 +11,17 @@ import type {
import { AiClient } from '@forestadmin/ai-proxy';

import { AiModelPortError, WorkflowExecutorError } from '../errors';
import toAiProxyLogger from './to-ai-proxy-logger';

export default class AiClientAdapter implements AiModelPort {
private readonly aiClient: AiClient;

constructor(aiConfigurations: AiConfiguration[]) {
constructor(aiConfigurations: AiConfiguration[], logger?: Logger) {
const withRetries = aiConfigurations.map(c => ({ maxRetries: 2, ...c }));
this.aiClient = new AiClient({ aiConfigurations: withRetries as AiConfiguration[] });
this.aiClient = new AiClient({
aiConfigurations: withRetries as AiConfiguration[],
logger: logger ? toAiProxyLogger(logger) : undefined,
});
}

getModel({ aiConfigName }: GetModelOptions = {}): BaseChatModel {
Expand Down
6 changes: 5 additions & 1 deletion packages/workflow-executor/src/adapters/pretty-logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ const LABEL: Record<LoggerLevel, string> = {
};

function formatContext(context: Record<string, unknown>): string {
const parts = Object.entries(context).map(([key, value]) => `${key}=${JSON.stringify(value)}`);
// Callers build a fixed context shape and leave the fields they have nothing for undefined,
// which JSON.stringify would render as the literal `undefined`.
const parts = Object.entries(context)
.filter(([, value]) => value !== undefined)
.map(([key, value]) => `${key}=${JSON.stringify(value)}`);
if (parts.length === 0) return '';

return pc.dim(parts.join(' '));
Expand Down
8 changes: 8 additions & 0 deletions packages/workflow-executor/src/adapters/server-ai-adapter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import type { AiProxyLogger } from './to-ai-proxy-logger';
import type { AiModelPort, GetModelOptions } from '../ports/ai-model-port';
import type { Logger } from '../ports/logger-port';
import type {
AiConfiguration,
BaseChatModel,
Expand All @@ -10,27 +12,33 @@ import type {
import { AiClient } from '@forestadmin/ai-proxy';

import { AiModelPortError, WorkflowExecutorError } from '../errors';
import toAiProxyLogger from './to-ai-proxy-logger';

export interface ServerAiAdapterOptions {
forestServerUrl: string;
envSecret: string;
logger?: Logger;
}

export default class ServerAiAdapter implements AiModelPort {
private readonly options: ServerAiAdapterOptions;
private readonly aiProxyLogger?: AiProxyLogger;
private readonly aiClient: AiClient;

constructor(options: ServerAiAdapterOptions) {
this.options = options;
this.aiProxyLogger = options.logger ? toAiProxyLogger(options.logger) : undefined;
this.aiClient = new AiClient({
aiConfigurations: [ServerAiAdapter.buildProxyConfiguration(options)],
logger: this.aiProxyLogger,
});
}

getModel({ userId }: GetModelOptions = {}): BaseChatModel {
try {
const client = new AiClient({
aiConfigurations: [ServerAiAdapter.buildProxyConfiguration(this.options, userId)],
logger: this.aiProxyLogger,
});

return client.getModel();
Expand Down
34 changes: 34 additions & 0 deletions packages/workflow-executor/src/adapters/to-ai-proxy-logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import type { Logger, LoggerLevel } from '../ports/logger-port';

import { extractErrorMessage } from '../errors';

// ai-proxy hands the cause as an Error, the executor's logger expects a context object.
export type AiProxyLogger = (level: LoggerLevel, message: string, error?: Error) => void;

// An Error's own properties are non-enumerable, so forwarding it as the context would emit the line
// with the cause silently stripped — flatten it the way the rest of the executor logs causes.
export default function toAiProxyLogger(logger: Logger): AiProxyLogger {
return (level, message, error) => {
Comment thread
hercemer42 marked this conversation as resolved.
// ai-proxy logs from inside its catch blocks before recording the failure it caught, so a host
// logger that throws here would abort a whole tool load instead of one server's.
try {
if (error === undefined || error === null) {
logger(level, message);

return;
}

const { cause } = error as { cause?: unknown };

logger(level, message, {
error: extractErrorMessage(error),
// `extractErrorMessage` only short-circuits on undefined, so a null cause would print
// the string "null" — the artifact the pretty logger drops undefined keys to avoid.
cause: cause == null ? undefined : extractErrorMessage(cause),
stack: error instanceof Error ? error.stack : undefined,
});
} catch {
// A broken logger must not become control flow.
}
};
}
4 changes: 2 additions & 2 deletions packages/workflow-executor/src/build-workflow-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,9 @@ function buildCommonDependencies(options: ExecutorOptions) {
if (forceAiError) {
aiModelPort = new AlwaysErrorAiModelPort();
} else if (options.aiConfigurations?.length) {
aiModelPort = new AiClientAdapter(options.aiConfigurations);
aiModelPort = new AiClientAdapter(options.aiConfigurations, logger);
Comment thread
hercemer42 marked this conversation as resolved.
} else {
aiModelPort = new ServerAiAdapter({ forestServerUrl, envSecret: options.envSecret });
aiModelPort = new ServerAiAdapter({ forestServerUrl, envSecret: options.envSecret, logger });
Comment thread
hercemer42 marked this conversation as resolved.
}

// A TTL of 0/negative/non-finite would silently make the cache always-stale, so fall back.
Expand Down
4 changes: 2 additions & 2 deletions packages/workflow-executor/src/ports/ai-model-port.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ export interface GetModelOptions {
export interface AiModelPort {
getModel(options?: GetModelOptions): BaseChatModel;
loadRemoteTools(configs: Record<string, ToolConfig>): Promise<RemoteTool[]>;
// Loads tools and exposes per-server failures classified by cause (auth vs connection), so the
// OAuth path can tell a revoked token from an unreachable server. Default consumers use loadRemoteTools.
// Loads tools and exposes per-server failures classified by cause (auth vs connection), so a
// caller can tell a revoked token from an unreachable server and name it in its logs.
loadRemoteToolsWithFailures(
configs: Record<string, ToolConfig>,
): Promise<{ tools: RemoteTool[]; failures: McpServerLoadFailure[] }>;
Expand Down
Loading
Loading