refactor(agent): forward workflow executor routes generically [PRD-567]#319
Merged
Conversation
a9e0562 to
0cff271
Compare
The agent proxied the workflow executor with one explicitly declared route per executor route, so every new executor route required a matching agent route and release. Replace the two hand-written routes with a single catch-all under /_internal/workflow-executions/* that forwards any sub-path and any verb to the executor's /runs/* namespace. - request forwarded verbatim: all client headers (minus hop-by-hop / host / content-length), raw body (no reshaping), query string and method as-is - security boundary: the wildcard can only map into /runs (path-traversal, encoded-dot and absolute-path inputs are rejected), so executor routes outside that namespace stay unreachable through the proxy - ForestController passes the raw request context (method/query/body) to the route closure; other routes ignore the extra keys fixes PRD-567 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0cff271 to
5375e20
Compare
…oxy [PRD-567] The generic proxy only relayed Content-Type, so response headers set by the executor (e.g. X-Forest-Executor-Version) were dropped — breaking the version gate. Forward every executor response header except hop-by-hop ones, symmetric to the request-header policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…[PRD-567] Generalize the response-header forwarding rationale: the agent forwards every executor header transparently so new executor headers never need an agent change. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eaders [PRD-567] Review follow-up on the workflow executor proxy: - ForestController now honors a root data[:headers] so executor response headers (e.g. X-Forest-Executor-Version) reach the HTTP response instead of being silently dropped — the proxy returns headers at the root, but the controller only read content[:headers] (always nil for the proxy). Additive; other routes are unaffected. Covered by a new controller-level spec. - skip accept-encoding (request) and content-encoding (response): the body is re-serialized, so upstream encoding/length no longer match, and forwarding accept-encoding disabled Faraday's transparent gzip decompression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 new issue
|
| next if value.nil? || value.to_s.empty? | ||
|
|
||
| acc[name.to_s] = value.to_s | ||
| end |
…le [PRD-567] Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oxy comments [PRD-567]
Review follow-up:
- forward now rescues Faraday::Error (SSL, etc.) as ServiceUnavailableError, so a
transport failure surfaces as 503 like ConnectionFailed instead of a generic 500.
Faraday never raises on executor 4xx/5xx (no raise_error middleware), so this only
catches reachability failures.
- fix a self-contradictory SKIPPED_HEADERS comment ("re-serialized" vs raw body) and
two minor comment imprecisions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
christophebrun-forest
approved these changes
Jun 23, 2026
christophebrun-forest
left a comment
Member
There was a problem hiding this comment.
LGTM after last commits.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The agent proxies the workflow executor with one explicitly declared route per executor route (
GET /_internal/workflow-executions/:run_id,POST .../trigger). Each new executor route therefore requires a matching agent route and an agent release — and the agent is customer-deployed, so a new executor capability is unreachable until the parc is upgraded.This replaces the two hand-written routes with a single catch-all that forwards any sub-path and any verb to the executor's
/runs/*namespace. Adding an executor route no longer requires any agent change.fixes PRD-567
Design (decisions agreed with the ticket author)
ALL /_internal/workflow-executions/*path→/runs/<path>, all verbs.Host/Content-Length), raw body (no reshaping), query string and method as-is./runs— path-traversal (..), encoded dots (%2e), absolute paths and null bytes are rejected, so executor routes outside/runs(e.g.mcp-oauth-credentials) stay unreachable through the proxy.ForestControllerpasses the raw request context (method/query_string/body) to the route closure; this is additive — other routes ignore the extra keys.Tests
rspec workflow_executor_proxy_spec→ 22/22, including 7 path-traversal security cases (the "internal route not exposed" guard). Rubocop clean. The one pre-existingforest_controller_specfailure (exception_handlerprod logging) is unrelated (fails onmaintoo).Scope
Phase 1 of 3 — this is the Ruby v2 agent. The Node
@forestadmin/agentand the Ruby v1 liana (forest-rails) carry the identical coupling and will get the same generic proxy in follow-up PRs.🤖 Generated with Claude Code
Note
Replace explicit workflow executor routes with a generic catch-all proxy in
WorkflowExecutorProxy/_internal/workflow-executions/*path) that forwards any HTTP verb and sub-path to the executor's/runs/*endpoint.unsafe_path?to reject traversal, encoded-dot, backslash, and null-byte paths with aNotFoundError.ForestControllerto passmethod,query_string, andbodyto route closures, and to merge top-leveldata[:headers]into the HTTP response.Macroscope summarized 42f1bc9.
Known limitation (by design)
This runtime is JSON-only: responses are rendered as JSON (
render json: ...viaForestController). A hypothetical non-JSON executor response (e.g.text/plain) would not pass through verbatim. The executor only returns JSON today, so this does not affect the PRD-567 contract in practice; the@forestadmin/agentNode core (agent-nodejs#1666) handles raw passthrough.