Skip to content

fix(rpc): action with file#307

Open
arnaud-moncel wants to merge 1 commit into
fix/rpc/action-responsefrom
fix/rpc/action-file
Open

fix(rpc): action with file#307
arnaud-moncel wants to merge 1 commit into
fix/rpc/action-responsefrom
fix/rpc/action-file

Conversation

@arnaud-moncel
Copy link
Copy Markdown
Member

@arnaud-moncel arnaud-moncel commented May 20, 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 file action response support to RPC action execution

  • Collection#execute in the RPC datasource now passes with_response: true to get the raw Faraday::Response, then checks the X-Forest-Action-Type header to detect file responses and builds a structured result with type, mime type, filename, stream, and optional response headers.
  • RpcClient#call_rpc gains a with_response keyword argument; when true, returns the raw Faraday::Response instead of the parsed body.
  • ActionExecute#handle_request in the RPC agent detects file results and converts them into a proper HTTP response with Content-Disposition, Content-Type, X-Forest-Action-Type, and optionally X-Forest-Action-Response-Headers.
  • File names are URL-encoded on the agent side and decoded with CGI.unescape on the datasource side.
📊 Macroscope summarized ab5c583. 3 files reviewed, 3 issues evaluated, 0 issues filtered, 1 comment posted

🗂️ Filtered Issues

@qltysh
Copy link
Copy Markdown

qltysh Bot commented May 20, 2026

1 new issue

Tool Category Rule Count
qlty Structure Function with many parameters (count = 7): call_rpc 1


# rubocop:disable Metrics/ParameterLists
def call_rpc(endpoint, caller: nil, method: :get, payload: nil, symbolize_keys: false, if_none_match: nil)
def call_rpc(endpoint, caller: nil, method: :get, payload: nil, symbolize_keys: false,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Function with many parameters (count = 7): call_rpc [qlty:function-parameters]

name: CGI.unescape(file_name_header.to_s),
stream: response.body.to_s
}
result[:response_headers] = JSON.parse(response_headers_header) if response_headers_header
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 forest_admin_datasource_rpc/collection.rb:198

When response_headers_header is an empty string or invalid JSON, JSON.parse raises JSON::ParserError and crashes action execution. The condition if response_headers_header is truthy for empty strings, so an absent or malformed header causes a hard failure rather than graceful fallback.

-      result[:response_headers] = JSON.parse(response_headers_header) if response_headers_header
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb around line 198:

When `response_headers_header` is an empty string or invalid JSON, `JSON.parse` raises `JSON::ParserError` and crashes action execution. The condition `if response_headers_header` is truthy for empty strings, so an absent or malformed header causes a hard failure rather than graceful fallback.

Evidence trail:
packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb lines 188-201 at REVIEWED_COMMIT: `response_headers_header` set from `response.headers['x-forest-action-response-headers']` (line 189), guard on line 198 `if response_headers_header` is truthy for empty strings in Ruby, `JSON.parse("")` raises `JSON::ParserError`. Ruby semantics: only `nil` and `false` are falsy; empty string is truthy.

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