feat: connect orchestrator and frontend

[Scra3]

MAIN BRANCH TO INTRODUCE WORKFLOW EXECUTOR.

Note

Connect workflow orchestrator to frontend by adding a workflow executor service and agent proxy routes

• Introduces the @forestadmin/workflow-executor package: a standalone Koa HTTP service and CLI that polls the Forest server for pending workflow steps, executes them via dedicated step executors (condition, read-record, update-record, trigger-action, load-related-record, MCP, guidance), and reports results back.
• Adds WorkflowExecutorProxyRoute to the agent, which forwards /_internal/workflow-executions/:runId GET and POST requests to a configured workflowExecutorUrl, preserving auth headers.
• Extends @forestadmin/ai-proxy with AiClient, createBaseChatModel, getAiConfiguration, and validateAiConfigurations; propagates mcpServerId through RemoteTool subclasses and Forest integration tool factories.
• Provides in-memory and Sequelize-backed RunStore implementations, a TTL SchemaCache, and a Runner with lifecycle management, deduplication via InFlightRunRegistry, and graceful shutdown.
• Risk: adding workflowExecutorUrl to agent options and mounting proxy routes changes the agent's route surface; the new package exposes port 3400 by default and requires FOREST_ENV_SECRET, FOREST_AUTH_SECRET, and AGENT_URL environment variables to start.

^{Macroscope summarized 018ee05.}

[macroscopeapp]

🟢 Low adapters/run-to-available-step-mapper.ts:64

The guidance branch converts 'awaiting-input' status to 'success' because it uses ctx.status === 'error' ? 'error' : 'success' instead of toRecordStatus. Since GuidanceStepOutcomeSchema explicitly allows 'awaiting-input' via RecordStepStatusSchema, this valid state is incorrectly overwritten. Consider using toRecordStatus(ctx.status) to preserve all valid status values.

Suggested change

	const status: GuidanceStepOutcome['status'] = ctx.status === 'error' ? 'error' : 'success';
	return { type: 'guidance', ...baseFromCtx, status };
	}
	if (outcomeType === 'guidance') {
	const status: GuidanceStepOutcome['status'] = toRecordStatus(ctx.status);
	return { type: 'guidance', ...baseFromCtx, status };
	}

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts around lines 64-67:

The `guidance` branch converts `'awaiting-input'` status to `'success'` because it uses `ctx.status === 'error' ? 'error' : 'success'` instead of `toRecordStatus`. Since `GuidanceStepOutcomeSchema` explicitly allows `'awaiting-input'` via `RecordStepStatusSchema`, this valid state is incorrectly overwritten. Consider using `toRecordStatus(ctx.status)` to preserve all valid status values.

Evidence trail:
packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts lines 30-35 (toRecordStatus helper), line 64 (guidance branch using ternary instead of toRecordStatus); packages/workflow-executor/src/types/validated/step-outcome.ts lines 11 (RecordStepStatusSchema allows 'awaiting-input'), lines 57-63 (GuidanceStepOutcomeSchema uses RecordStepStatusSchema); packages/workflow-executor/src/executors/guidance-step-executor.ts line 14 (executor produces 'awaiting-input' for guidance steps)

[macroscopeapp]

🟡 Medium executors/trigger-record-action-step-executor.ts:185

saveFrontendResult spreads existingExecution without setting idempotencyPhase: 'done', so after Branch C completes the step data lacks that field. If the executor restarts, checkIdempotency() (line 53) fails to recognize completion because idempotencyPhase is undefined, causing the step to incorrectly re-enter the confirmation flow.

    await this.context.runStore.saveStepExecution(this.context.runId, {
      ...existingExecution,
      type: 'trigger-action',
      stepIndex: this.context.stepIndex,
      executionParams: { displayName, name },
      executionResult: { success: true, actionResult },
      selectedRecordRef,
+      idempotencyPhase: 'done',
    });

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/executors/trigger-record-action-step-executor.ts around lines 185-192:

`saveFrontendResult` spreads `existingExecution` without setting `idempotencyPhase: 'done'`, so after Branch C completes the step data lacks that field. If the executor restarts, `checkIdempotency()` (line 53) fails to recognize completion because `idempotencyPhase` is undefined, causing the step to incorrectly re-enter the confirmation flow.

Evidence trail:
packages/workflow-executor/src/executors/trigger-record-action-step-executor.ts lines 48-62 (checkIdempotency), 134-142 (Branch C save without idempotencyPhase), 178-195 (saveFrontendResult missing idempotencyPhase:'done'), 146-174 (executeOnExecutor correctly sets idempotencyPhase). packages/workflow-executor/src/executors/update-record-step-executor.ts lines 205-239 (resolveAndUpdate sets idempotencyPhase:'done' even in confirmation flow). packages/workflow-executor/src/types/step-execution-data.ts lines 13-14 (MutatingStepExecutionData.idempotencyPhase definition), lines 76-83 (TriggerRecordActionStepExecutionData extends MutatingStepExecutionData). packages/workflow-executor/src/executors/base-step-executor.ts lines 194-234 (findPendingExecution, patchAndReloadPendingData), lines 238-262 (handleConfirmationFlow).

[macroscopeapp]

🟢 Low adapters/console-logger.ts:5

In error, warn, and info, the spread order { message, timestamp, ...context } allows context to overwrite message and timestamp when the caller passes those keys. This corrupts the log output by replacing the actual message or timestamp with values from context. Reorder the spread to { ...context, message, timestamp } to protect the primary log fields.

-      console.error(JSON.stringify({ message, timestamp: new Date().toISOString(), ...context }));
+      console.error(JSON.stringify({ ...context, message, timestamp: new Date().toISOString() }));

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/adapters/console-logger.ts around line 5:

In `error`, `warn`, and `info`, the spread order `{ message, timestamp, ...context }` allows `context` to overwrite `message` and `timestamp` when the caller passes those keys. This corrupts the log output by replacing the actual message or timestamp with values from context. Reorder the spread to `{ ...context, message, timestamp }` to protect the primary log fields.

Evidence trail:
packages/workflow-executor/src/adapters/console-logger.ts lines 4-14 at REVIEWED_COMMIT — `...context` is spread last in the object literal on lines 5, 9, and 13, meaning any `message` or `timestamp` key in the `context: Record<string, unknown>` parameter overwrites the primary fields. JavaScript spec (ES2018+) defines that later spread properties overwrite earlier ones with the same key.

[macroscopeapp]

🟢 Low adapters/with-retry.ts:14

When fn throws null or undefined, the destructuring const { status } = err as { status?: number } throws a TypeError before line 17 can check the status. This crashes the retry mechanism instead of treating the error as non-retryable. Consider adding a null guard before destructuring, or use optional chaining to safely access status.

 function isRetryable(err: unknown): boolean {
-  const { status } = err as { status?: number };
+  if (err == null) return false;
+  const status = (err as { status?: number }).status;
 
   return typeof status === 'number' && RETRYABLE_STATUS.has(status);
 }

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/adapters/with-retry.ts around lines 14-18:

When `fn` throws `null` or `undefined`, the destructuring `const { status } = err as { status?: number }` throws a `TypeError` before line 17 can check the status. This crashes the retry mechanism instead of treating the error as non-retryable. Consider adding a null guard before destructuring, or use optional chaining to safely access `status`.

Evidence trail:
packages/workflow-executor/src/adapters/with-retry.ts lines 14-18 at REVIEWED_COMMIT: `isRetryable` destructures `err` without null guard. JavaScript behavior: `const { x } = null` throws TypeError. Callers at packages/workflow-executor/src/adapters/forest-server-workflow-port.ts:231, packages/workflow-executor/src/adapters/forestadmin-client-activity-log-port.ts:26,58,80.

[macroscopeapp]

🟡 Medium src/ai-client.ts:39

If newClient.loadTools() throws, the McpClient instance created on line 44 is never disposed. Since this.mcpClient is only assigned after loadTools() succeeds, a failed call leaves newClient dangling with open connections, causing a resource leak. Consider wrapping loadTools() in a try/finally to ensure cleanup on failure.

  async loadRemoteTools(
    mcpConfig: McpConfiguration,
  ): Promise<Awaited<ReturnType<McpClient['loadTools']>>> {
    await this.disposeMcpClient('Error closing previous MCP connection');

    const newClient = new McpClient(mcpConfig, this.logger);
+    try {
      const tools = await newClient.loadTools();
      this.mcpClient = newClient;

      return tools;
+    } catch (error) {
+      await newClient.dispose();
+      throw error;
+    }
  }

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/ai-proxy/src/ai-client.ts around lines 39-49:

If `newClient.loadTools()` throws, the `McpClient` instance created on line 44 is never disposed. Since `this.mcpClient` is only assigned after `loadTools()` succeeds, a failed call leaves `newClient` dangling with open connections, causing a resource leak. Consider wrapping `loadTools()` in a try/finally to ensure cleanup on failure.

Evidence trail:
packages/ai-proxy/src/ai-client.ts lines 39-49 (REVIEWED_COMMIT): loadRemoteTools creates newClient on line 44, calls loadTools() on line 45, assigns to this.mcpClient on line 46. No try/finally wrapping.
packages/ai-proxy/src/mcp-client.ts lines 24-37 (REVIEWED_COMMIT): McpClient constructor creates MultiServerMCPClient instances stored in this.mcpClients.
packages/ai-proxy/src/mcp-client.ts lines 95-118 (REVIEWED_COMMIT): dispose() method calls client.close() on each MultiServerMCPClient - explicit cleanup needed.
packages/ai-proxy/src/ai-client.ts lines 55-66 (REVIEWED_COMMIT): disposeMcpClient only disposes this.mcpClient, not a local newClient variable.

[macroscopeapp]

🟢 Low adapters/forestadmin-client-activity-log-port.ts:77

In markFailed, the errorMessage parameter is accepted but never passed to updateActivityLogStatus. The status is set to 'failed' without persisting the error context to the audit log, so the failure reason is lost. The message is only used for local logging when the update itself fails.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/adapters/forestadmin-client-activity-log-port.ts around line 77:

In `markFailed`, the `errorMessage` parameter is accepted but never passed to `updateActivityLogStatus`. The status is set to `'failed'` without persisting the error context to the audit log, so the failure reason is lost. The message is only used for local logging when the update itself fails.

Evidence trail:
packages/workflow-executor/src/adapters/forestadmin-client-activity-log-port.ts lines 77-98 (markFailed method), packages/workflow-executor/src/ports/activity-log-port.ts line 20 (ActivityLogPort interface), packages/forestadmin-client/src/types.ts lines 254-258 (UpdateActivityLogStatusParams - no errorMessage field), packages/forestadmin-client/src/activity-logs/index.ts lines 97-107 (updateActivityLogStatus implementation), packages/forestadmin-client/CHANGELOG.md line 43 (intentional removal of errorMessage from updateActivityLogStatus)

[macroscopeapp]

🟡 Medium adapters/agent-client-agent-port.ts:77

When getRecord is called without fields, restoreFieldNames receives undefined and returns raw camelCase keys from the agent-client. Schema field names like card_number are returned as cardNumber, breaking lookups by schema field name. Consider passing all schema field names to restoreFieldNames when fields is undefined, similar to how getRelatedData handles this on lines 133-134.

-      return {
-        collectionName: collection,
-        recordId: id,
-        values: restoreFieldNames(records[0], fields),
-      };

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/adapters/agent-client-agent-port.ts around lines 77-96:

When `getRecord` is called without `fields`, `restoreFieldNames` receives `undefined` and returns raw camelCase keys from the agent-client. Schema field names like `card_number` are returned as `cardNumber`, breaking lookups by schema field name. Consider passing all schema field names to `restoreFieldNames` when `fields` is undefined, similar to how `getRelatedData` handles this on lines 133-134.

Evidence trail:
packages/workflow-executor/src/adapters/agent-client-agent-port.ts lines 33-46 (restoreFieldNames function, returns values as-is when originalFieldNames is undefined at line 37), line 77-97 (getRecord passes `fields` directly which can be undefined at line 94), lines 120-143 (getRelatedData always passes schema field names at lines 132-134), lines 26-28 (toCamelCase conversion), lines 30-32 (comment explaining the camelCase issue)