Skip to content

feat(agent): proxify ws executor call#1522

Open
arnaud-moncel wants to merge 4 commits intofeat/prd-214-setup-workflow-executor-packagefrom
feat/wf-executor-proxy
Open

feat(agent): proxify ws executor call#1522
arnaud-moncel wants to merge 4 commits intofeat/prd-214-setup-workflow-executor-packagefrom
feat/wf-executor-proxy

Conversation

@arnaud-moncel
Copy link
Copy Markdown
Member

@arnaud-moncel arnaud-moncel commented Apr 1, 2026

Definition of Done

General

  • Write an explicit title for the Pull Request, following Conventional Commits specification
  • Test manually the implemented changes
  • Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

  • Consider the security impact of the changes made

Note

Add proxy routes for workflow executor under /_internal/workflow-executions/*

  • Adds a new WorkflowExecutorProxyRoute that registers three private routes (GET, POST, PATCH) under /_internal/workflow-executions/:runId and proxies them to the workflow executor's /runs/* endpoints.
  • A new workflowExecutorUrl option on AgentOptions controls whether the proxy is active; when null or unset, no routes are registered.
  • Request bodies are forwarded as JSON for non-GET methods; responses are parsed as JSON when possible, otherwise returned as raw text.
  • Also adds mock mode infrastructure (MockWorkflowPort, MockAgentPort, MockAiModelPort) and an expanded README to the workflow-executor example package.

Macroscope summarized ba56cb7.

Comment on lines +36 to +39
const executorPath = context.path.replace(
WorkflowExecutorProxyRoute.AGENT_PREFIX,
WorkflowExecutorProxyRoute.EXECUTOR_PREFIX,
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium workflow/workflow-executor-proxy.ts:36

handleProxy uses context.path to construct the target URL, but path contains only the pathname without the query string. Requests with query parameters (e.g., ?foo=bar) lose those parameters when forwarded to the executor. Consider using context.url instead, which includes both the pathname and query string.

-    const executorPath = context.path.replace(
+    const executorPath = context.url.replace(
       WorkflowExecutorProxyRoute.AGENT_PREFIX,
       WorkflowExecutorProxyRoute.EXECUTOR_PREFIX,
     );
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/agent/src/routes/workflow/workflow-executor-proxy.ts around lines 36-39:

`handleProxy` uses `context.path` to construct the target URL, but `path` contains only the pathname without the query string. Requests with query parameters (e.g., `?foo=bar`) lose those parameters when forwarded to the executor. Consider using `context.url` instead, which includes both the pathname and query string.

Evidence trail:
1. packages/agent/src/routes/workflow/workflow-executor-proxy.ts lines 36-40 (REVIEWED_COMMIT): Code uses `context.path.replace()` to construct `executorPath`, then `new URL(executorPath, this.executorUrl)` to create the target URL.
2. Koa documentation at https://koajs.com confirms: `request.path` - "Get request pathname" and `request.querystring` - "Get raw query string void of ?" - these are separate properties, confirming `path` does not include query parameters.

@arnaud-moncel arnaud-moncel force-pushed the feat/wf-executor-proxy branch from c938768 to 8891bf0 Compare April 1, 2026 07:59
@arnaud-moncel arnaud-moncel changed the base branch from main to feat/prd-214-setup-workflow-executor-package April 1, 2026 08:00
@qltysh
Copy link
Copy Markdown

qltysh bot commented Apr 1, 2026

1 new issue

Tool Category Rule Count
qlty Structure Function with many returns (count = 4): createMockModel 1

@arnaud-moncel arnaud-moncel force-pushed the feat/wf-executor-proxy branch from 8891bf0 to 1f28b96 Compare April 1, 2026 08:02
Comment on lines +40 to +41
const httpPort = Number(process.env.EXECUTOR_HTTP_PORT ?? '4001');
const pollingIntervalMs = Number(process.env.EXECUTOR_POLLING_INTERVAL_MS ?? '5000');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low src/main.ts:40

Number(process.env.EXECUTOR_HTTP_PORT) and Number(process.env.EXECUTOR_POLLING_INTERVAL_MS) return NaN for invalid numeric strings (e.g., abc), silently propagating NaN to downstream code that expects a valid number. Unlike required env vars which throw on missing values, these misconfigurations fail silently and cause confusing downstream failures. Consider validating and throwing a clear error when the environment variable is set but not parsable as a number.

-  const httpPort = Number(process.env.EXECUTOR_HTTP_PORT ?? '4001');
-  const pollingIntervalMs = Number(process.env.EXECUTOR_POLLING_INTERVAL_MS ?? '5000');
+  const httpPort = parseIntEnv('EXECUTOR_HTTP_PORT', 4001);
+  const pollingIntervalMs = parseIntEnv('EXECUTOR_POLLING_INTERVAL_MS', 5000);
Also found in 1 other location(s)

packages/agent/src/routes/workflow/workflow-executor-proxy.ts:19

Calling .replace() on options.workflowExecutorUrl at line 19 will throw a TypeError if the value is null. The type AgentOptions.workflowExecutorUrl is string | null, and the test factory in code object 1 defaults it to null. If WorkflowExecutorProxyRoute is instantiated when workflowExecutorUrl is null, the constructor crashes with "Cannot read properties of null (reading 'replace')".

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/workflow-executor/src/main.ts around lines 40-41:

`Number(process.env.EXECUTOR_HTTP_PORT)` and `Number(process.env.EXECUTOR_POLLING_INTERVAL_MS)` return `NaN` for invalid numeric strings (e.g., `abc`), silently propagating `NaN` to downstream code that expects a valid number. Unlike required env vars which throw on missing values, these misconfigurations fail silently and cause confusing downstream failures. Consider validating and throwing a clear error when the environment variable is set but not parsable as a number.

Evidence trail:
packages/workflow-executor/src/main.ts:40 - `const httpPort = Number(process.env.EXECUTOR_HTTP_PORT ?? '4001')` shows Number() parsing without validation
packages/workflow-executor/src/main.ts:41 - `const pollingIntervalMs = Number(process.env.EXECUTOR_POLLING_INTERVAL_MS ?? '5000')` same pattern
packages/workflow-executor/src/main.ts:70 - httpPort passed to Runner constructor
packages/workflow-executor/src/http/executor-http-server.ts:93 - `this.server.listen(this.options.port, resolve)` uses port directly
packages/workflow-executor/src/runner.ts:198 - `setTimeout(() => this.runPollCycle(), this.config.pollingIntervalMs)` uses interval directly
packages/workflow-executor/src/main.ts:24-30 - requireEnv() validates required vars but no equivalent for numeric validation

Also found in 1 other location(s):
- packages/agent/src/routes/workflow/workflow-executor-proxy.ts:19 -- Calling `.replace()` on `options.workflowExecutorUrl` at line 19 will throw a `TypeError` if the value is `null`. The type `AgentOptions.workflowExecutorUrl` is `string | null`, and the test factory in code object 1 defaults it to `null`. If `WorkflowExecutorProxyRoute` is instantiated when `workflowExecutorUrl` is `null`, the constructor crashes with "Cannot read properties of null (reading 'replace')".

@Scra3 Scra3 force-pushed the feat/wf-executor-proxy branch from 1f28b96 to d520398 Compare April 7, 2026 14:54
alban bertolini and others added 2 commits April 7, 2026 21:06
Add a mock executor that simulates a complete "Customer Onboarding"
workflow with all 7 step types (condition, read-record, update-record,
trigger-action, load-related-record, mcp, guidance).

No external dependencies needed — mock ports replace the orchestrator,
agent, and AI. The front can connect via the standard HTTP endpoints.

Also upgrade @langchain/core from 1.1.15 to 1.1.39 to fix the
ERR_PACKAGE_PATH_NOT_EXPORTED error when running TS files with tsx.

Run: npx tsx packages/workflow-executor/example/mock.ts

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Scra3 Scra3 force-pushed the feat/wf-executor-proxy branch from d520398 to 25be9f0 Compare April 8, 2026 07:22
alban bertolini and others added 2 commits April 8, 2026 09:33
Remove Dockerfile, .dockerignore, and main.ts entry point.
These duplicate the example/ setup and are premature — Docker
deployment will be added when we're ready to ship.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@qltysh
Copy link
Copy Markdown

qltysh bot commented Apr 8, 2026

Qlty

Coverage Impact

⬇️ Merging this pull request will decrease total coverage on feat/prd-214-setup-workflow-executor-package by 0.01%.

Modified Files with Diff Coverage (3)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
packages/agent/src/utils/options-validator.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent/src/routes/index.ts66.7%179
New file Coverage rating: A
packages/agent/src/routes/workflow/workflow-executor-proxy.ts97.1%69
Total94.7%
🤖 Increase coverage with AI coding...

In the `feat/wf-executor-proxy` branch, add test coverage for this new code:

- `packages/agent/src/routes/index.ts` -- Line 179
- `packages/agent/src/routes/workflow/workflow-executor-proxy.ts` -- Line 69

🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant